Skip to content

Commit fb4f518

Browse files
committed
IRGen: Fix DynamicSelfType metadata recovery in @objc convenience initializers
An @objc convenience initializer can replace 'self'. Since IRGen has no way to tell what the new 'self' value is from looking at SIL, it always refers back to the original 'self' argument when recovering DynamicSelfType metadata. This could cause a crash if the metadata was used after the 'self' value had been replaced. To fix the crash, always insert the metadata recovery into the entry block, where the 'self' value should be valid (even if uninitialized, the 'isa' pointer should be correct). Fixes <rdar://problem/50594689>.
1 parent df60557 commit fb4f518

File tree

3 files changed

+67
-6
lines changed

3 files changed

+67
-6
lines changed

lib/IRGen/GenHeap.cpp

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1684,17 +1684,43 @@ void IRGenFunction::emit##ID(llvm::Value *value, Address src) { \
16841684

16851685
llvm::Value *IRGenFunction::getLocalSelfMetadata() {
16861686
assert(LocalSelf && "no local self metadata");
1687+
1688+
// If we already have a metatype, just return it.
1689+
if (SelfKind == SwiftMetatype)
1690+
return LocalSelf;
1691+
1692+
// We need to materialize a metatype. Emit the code for that once at the
1693+
// top of the function and cache the result.
1694+
1695+
// This is a slight optimization in the case of repeated access, but also
1696+
// needed for correctness; when an @objc convenience initializer replaces
1697+
// the 'self' value, we don't keep track of what the new 'self' value is
1698+
// in IRGen, so we can't just grab the first function argument and assume
1699+
// it's a valid 'self' at the point where DynamicSelfType metadata is needed.
1700+
1701+
// Note that if DynamicSelfType was modeled properly as an opened archetype,
1702+
// none of this would be an issue since it would be always be associated
1703+
// with the correct value.
1704+
1705+
llvm::IRBuilderBase::InsertPointGuard guard(Builder);
1706+
Builder.SetInsertPoint(&CurFn->getEntryBlock(),
1707+
CurFn->getEntryBlock().begin());
1708+
16871709
switch (SelfKind) {
16881710
case SwiftMetatype:
1689-
return LocalSelf;
1711+
llvm_unreachable("Already handled");
16901712
case ObjCMetatype:
1691-
return emitObjCMetadataRefForMetadata(*this, LocalSelf);
1713+
LocalSelf = emitObjCMetadataRefForMetadata(*this, LocalSelf);
1714+
SelfKind = SwiftMetatype;
1715+
break;
16921716
case ObjectReference:
1693-
return emitDynamicTypeOfOpaqueHeapObject(*this, LocalSelf,
1717+
LocalSelf = emitDynamicTypeOfOpaqueHeapObject(*this, LocalSelf,
16941718
MetatypeRepresentation::Thick);
1719+
SelfKind = SwiftMetatype;
1720+
break;
16951721
}
16961722

1697-
llvm_unreachable("Not a valid LocalSelfKind.");
1723+
return LocalSelf;
16981724
}
16991725

17001726
/// Given a non-tagged object pointer, load a pointer to its class object.

test/IRGen/dynamic_self_metadata.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,6 @@ class C {
5050
// CHECK-LABEL: define hidden swiftcc i64 @"$s21dynamic_self_metadata1CC0A18SelfConformingTypeACXDSgyF"(%T21dynamic_self_metadata1CC* swiftself)
5151
// CHECK: [[SELF:%.*]] = bitcast %T21dynamic_self_metadata1CC* %0 to %objc_object*
5252
// CHECK: [[SELF_TYPE:%.*]] = call %swift.type* @swift_getObjectType(%objc_object* [[SELF]])
53-
// CHECK: [[SELF:%.*]] = bitcast %T21dynamic_self_metadata1CC* %0 to %objc_object*
54-
// CHECK: [[SELF_TYPE:%.*]] = call %swift.type* @swift_getObjectType(%objc_object* [[SELF]])
5553
// CHECK: [[METADATA_RESPONSE:%.*]] = call swiftcc %swift.metadata_response @"$s21dynamic_self_metadata1GVMa"(i64 0, %swift.type* [[SELF_TYPE]])
5654
// CHECK: [[METADATA:%.*]] = extractvalue %swift.metadata_response [[METADATA_RESPONSE]], 0
5755
// CHECK: call i8** @swift_getWitnessTable(%swift.protocol_conformance_descriptor* bitcast ({{.*}} @"$s21dynamic_self_metadata1GVyxGAA1PAAMc" to %swift.protocol_conformance_descriptor*), %swift.type* [[METADATA]], i8*** undef)
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// RUN: %empty-directory(%t)
2+
3+
// RUN: %target-build-swift %s -o %t/main -swift-version 5
4+
// RUN: %target-codesign %t/main
5+
// RUN: %target-run %t/main
6+
7+
// REQUIRES: executable_test
8+
// REQUIRES: objc_interop
9+
10+
import Foundation
11+
import StdlibUnittest
12+
13+
func foo(_ x: @escaping () -> ()) { x() }
14+
15+
public class C {
16+
var x: Int = 0
17+
18+
init(blah: ()) {}
19+
20+
@objc convenience init() {
21+
self.init(blah: ())
22+
23+
foo { [weak self] in
24+
guard let `self` = self else { return }
25+
self.x += 1
26+
}
27+
}
28+
}
29+
30+
var ConvenienceInitSelfTest = TestSuite("ConvenienceInitSelf")
31+
32+
ConvenienceInitSelfTest.test("SelfMetadata") {
33+
let c = C()
34+
expectEqual(c.x, 1)
35+
}
36+
37+
runAllTests()

0 commit comments

Comments
 (0)