Skip to content

Commit 41f472e

Browse files
jckarterjckarter
committed
KeyPaths: Don't relative-reference selector refs.
ld64 doesn't like this. Instead, generate a stub function to grab the instantiated selector. Fixes rdar://problem/47184763.
1 parent 471b4a1 commit 41f472e

File tree

7 files changed

+115
-35
lines changed

7 files changed

+115
-35
lines changed

‎include/swift/ABI/KeyPath.h

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -190,17 +190,17 @@ class KeyPathComponentHeader {
190190
VTableOffset,
191191
};
192192

193-
constexpr static uint32_t
194-
getResolutionStrategy(ComputedPropertyIDKind idKind) {
195-
return idKind == Pointer ? _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
196-
: (assert("no resolution strategy implemented" && false), 0);
197-
}
193+
enum ComputedPropertyIDResolution {
194+
Resolved,
195+
IndirectPointer,
196+
FunctionCall,
197+
};
198198

199199
constexpr static KeyPathComponentHeader
200200
forComputedProperty(ComputedPropertyKind kind,
201201
ComputedPropertyIDKind idKind,
202202
bool hasArguments,
203-
bool resolvedID) {
203+
ComputedPropertyIDResolution resolution) {
204204
return KeyPathComponentHeader(
205205
(_SwiftKeyPathComponentHeader_ComputedTag
206206
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
@@ -213,8 +213,10 @@ class KeyPathComponentHeader {
213213
| (idKind == VTableOffset
214214
? _SwiftKeyPathComponentHeader_ComputedIDByVTableOffsetFlag : 0)
215215
| (hasArguments ? _SwiftKeyPathComponentHeader_ComputedHasArgumentsFlag : 0)
216-
| (resolvedID ? _SwiftKeyPathComponentHeader_ComputedIDResolved
217-
: getResolutionStrategy(idKind)));
216+
| (resolution == Resolved ? _SwiftKeyPathComponentHeader_ComputedIDResolved
217+
: resolution == IndirectPointer ? _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
218+
: resolution == FunctionCall ? _SwiftKeyPathComponentHeader_ComputedIDUnresolvedFunctionCall
219+
: (assert(false && "invalid resolution"), 0)));
218220
}
219221

220222
constexpr static KeyPathComponentHeader

‎lib/IRGen/GenKeyPath.cpp

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "TypeInfo.h"
3737
#include "llvm/ADT/SetVector.h"
3838
#include "llvm/IR/Module.h"
39+
#include "llvm/IR/Function.h"
3940
#include "swift/SIL/SILInstruction.h"
4041
#include "swift/SIL/SILLocation.h"
4142
#include "swift/SIL/TypeLowering.h"
@@ -942,7 +943,7 @@ emitKeyPathComponent(IRGenModule &IGM,
942943
auto id = component.getComputedPropertyId();
943944
KeyPathComponentHeader::ComputedPropertyIDKind idKind;
944945
llvm::Constant *idValue;
945-
bool idResolved;
946+
KeyPathComponentHeader::ComputedPropertyIDResolution idResolution;
946947
switch (id.getKind()) {
947948
case KeyPathPatternComponent::ComputedPropertyId::Function: {
948949
idKind = KeyPathComponentHeader::Pointer;
@@ -952,7 +953,9 @@ emitKeyPathComponent(IRGenModule &IGM,
952953
idValue = idRef.getValue();
953954
// If we got an indirect reference, we'll need to resolve it at
954955
// instantiation time.
955-
idResolved = !idRef.isIndirect();
956+
idResolution = idRef.isIndirect()
957+
? KeyPathComponentHeader::IndirectPointer
958+
: KeyPathComponentHeader::Resolved;
956959
break;
957960
}
958961
case KeyPathPatternComponent::ComputedPropertyId::DeclRef: {
@@ -964,8 +967,35 @@ emitKeyPathComponent(IRGenModule &IGM,
964967
if (declRef.isForeign) {
965968
assert(IGM.ObjCInterop && "foreign keypath component w/o objc interop?!");
966969
idKind = KeyPathComponentHeader::Pointer;
967-
idValue = IGM.getAddrOfObjCSelectorRef(declRef);
968-
idResolved = false;
970+
// FIXME: In non-JIT mode, ideally we would just refer to the selector
971+
// reference variable here with an indirectpointer resolution,
972+
// but ld64 section coalescing on the __objc_sel section can break
973+
// relative references (and on some platforms, mach-o just doesn't
974+
// support the necessary relocations).
975+
// As a workaround, generate a stub function to resolve the selector.
976+
//
977+
// Note that we'd need to do this anyway in JIT mode because we would
978+
// need to unique the selector at runtime anyway.
979+
auto selectorName = IGM.getObjCSelectorName(declRef);
980+
llvm::Type *fnParams[] = {IGM.Int8PtrTy};
981+
auto fnTy = llvm::FunctionType::get(IGM.Int8PtrTy, fnParams, false);
982+
SmallString<32> fnName;
983+
fnName.append("keypath_get_selector_");
984+
fnName.append(selectorName);
985+
auto fn = cast<llvm::Function>(
986+
IGM.Module.getOrInsertFunction(fnName, fnTy));
987+
if (fn->empty()) {
988+
fn->setLinkage(llvm::Function::PrivateLinkage);
989+
IRGenFunction subIGF(IGM, fn);
990+
if (IGM.DebugInfo)
991+
IGM.DebugInfo->emitArtificialFunction(subIGF, fn);
992+
993+
auto selectorValue = subIGF.emitObjCSelectorRefLoad(selectorName);
994+
subIGF.Builder.CreateRet(selectorValue);
995+
}
996+
997+
idValue = fn;
998+
idResolution = KeyPathComponentHeader::FunctionCall;
969999
} else {
9701000
if (auto overridden = declRef.getOverriddenVTableEntry())
9711001
declRef = overridden;
@@ -984,7 +1014,9 @@ emitKeyPathComponent(IRGenModule &IGM,
9841014
LinkEntity::forMethodDescriptor(declRef));
9851015

9861016
idValue = idRef.getValue();
987-
idResolved = !idRef.isIndirect();
1017+
idResolution = idRef.isIndirect()
1018+
? KeyPathComponentHeader::IndirectPointer
1019+
: KeyPathComponentHeader::Resolved;
9881020
break;
9891021
}
9901022

@@ -995,7 +1027,7 @@ emitKeyPathComponent(IRGenModule &IGM,
9951027
auto index = protoInfo.getFunctionIndex(
9961028
cast<AbstractFunctionDecl>(declRef.getDecl()));
9971029
idValue = llvm::ConstantInt::get(IGM.SizeTy, -index.getValue());
998-
idResolved = true;
1030+
idResolution = KeyPathComponentHeader::Resolved;
9991031
}
10001032
break;
10011033
}
@@ -1008,7 +1040,7 @@ emitKeyPathComponent(IRGenModule &IGM,
10081040
// Scan the stored properties of the struct to find the index. We should
10091041
// only ever use a struct field as a uniquing key from inside the
10101042
// struct's own module, so this is OK.
1011-
idResolved = true;
1043+
idResolution = KeyPathComponentHeader::Resolved;
10121044
Optional<unsigned> structIdx;
10131045
unsigned i = 0;
10141046
for (auto storedProp : struc->getStoredProperties()) {
@@ -1029,7 +1061,7 @@ emitKeyPathComponent(IRGenModule &IGM,
10291061
case FieldAccess::ConstantDirect:
10301062
case FieldAccess::ConstantIndirect:
10311063
case FieldAccess::NonConstantDirect:
1032-
idResolved = true;
1064+
idResolution = KeyPathComponentHeader::Resolved;
10331065
idValue = llvm::ConstantInt::get(IGM.SizeTy,
10341066
getClassFieldIndex(classDecl, property));
10351067
break;
@@ -1042,7 +1074,7 @@ emitKeyPathComponent(IRGenModule &IGM,
10421074
}
10431075

10441076
auto header = KeyPathComponentHeader::forComputedProperty(componentKind,
1045-
idKind, !isInstantiableOnce, idResolved);
1077+
idKind, !isInstantiableOnce, idResolution);
10461078

10471079
fields.addInt32(header.getData());
10481080
switch (idKind) {

‎lib/IRGen/GenObjC.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,11 @@ llvm::Constant *IRGenModule::getAddrOfObjCSelectorRef(SILDeclRef method) {
534534
return getAddrOfObjCSelectorRef(Selector(method).str());
535535
}
536536

537+
std::string IRGenModule::getObjCSelectorName(SILDeclRef method) {
538+
assert(method.isForeign);
539+
return Selector(method).str();
540+
}
541+
537542
static llvm::Value *emitSuperArgument(IRGenFunction &IGF,
538543
bool isInstanceMethod,
539544
llvm::Value *selfValue,

‎lib/IRGen/IRGenModule.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,7 @@ class IRGenModule {
832832
llvm::Constant *getAddrOfGlobalUTF16String(StringRef utf8);
833833
llvm::Constant *getAddrOfObjCSelectorRef(StringRef selector);
834834
llvm::Constant *getAddrOfObjCSelectorRef(SILDeclRef method);
835+
std::string getObjCSelectorName(SILDeclRef method);
835836
llvm::Constant *getAddrOfObjCMethodName(StringRef methodName);
836837
llvm::Constant *getAddrOfObjCProtocolRecord(ProtocolDecl *proto,
837838
ForDefinition_t forDefinition);

‎stdlib/public/SwiftShims/KeyPath.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDResolved
106106
= 0x00000000U;
107107
static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
108108
= 0x00000002U;
109+
static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDUnresolvedFunctionCall
110+
= 0x00000001U;
109111

110112
extern const void *_Nonnull (swift_keyPathGenericWitnessTable[]);
111113

‎stdlib/public/core/KeyPath.swift

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,12 @@ internal enum KeyPathComputedIDKind {
739739
case vtableOffset
740740
}
741741

742+
internal enum KeyPathComputedIDResolution {
743+
case resolved
744+
case indirectPointer
745+
case functionCall
746+
}
747+
742748
internal struct RawKeyPathComponent {
743749
internal init(header: Header, body: UnsafeRawBufferPointer) {
744750
self.header = header
@@ -890,9 +896,20 @@ internal struct RawKeyPathComponent {
890896
internal static var computedIDUnresolvedIndirectPointer: UInt32 {
891897
return _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
892898
}
893-
internal var isComputedIDResolved: Bool {
894-
return
895-
payload & Header.computedIDResolutionMask == Header.computedIDResolved
899+
internal static var computedIDUnresolvedFunctionCall: UInt32 {
900+
return _SwiftKeyPathComponentHeader_ComputedIDUnresolvedFunctionCall
901+
}
902+
internal var computedIDResolution: KeyPathComputedIDResolution {
903+
switch payload & Header.computedIDResolutionMask {
904+
case Header.computedIDResolved:
905+
return .resolved
906+
case Header.computedIDUnresolvedIndirectPointer:
907+
return .indirectPointer
908+
case Header.computedIDUnresolvedFunctionCall:
909+
return .functionCall
910+
default:
911+
_internalInvariantFailure("invalid key path resolution")
912+
}
896913
}
897914

898915
internal var _value: UInt32
@@ -2512,7 +2529,7 @@ internal protocol KeyPathPatternVisitor {
25122529
offset: KeyPathPatternStoredOffset)
25132530
mutating func visitComputedComponent(mutating: Bool,
25142531
idKind: KeyPathComputedIDKind,
2515-
idResolved: Bool,
2532+
idResolution: KeyPathComputedIDResolution,
25162533
idValueBase: UnsafeRawPointer,
25172534
idValue: Int32,
25182535
getter: UnsafeRawPointer,
@@ -2694,7 +2711,7 @@ internal func _walkKeyPathPattern<W: KeyPathPatternVisitor>(
26942711

26952712
walker.visitComputedComponent(mutating: header.isComputedMutating,
26962713
idKind: header.computedIDKind,
2697-
idResolved: header.isComputedIDResolved,
2714+
idResolution: header.computedIDResolution,
26982715
idValueBase: idValueBase,
26992716
idValue: idValue,
27002717
getter: getter,
@@ -2785,7 +2802,7 @@ internal func _walkKeyPathPattern<W: KeyPathPatternVisitor>(
27852802
walker.visitComputedComponent(
27862803
mutating: descriptorHeader.isComputedMutating,
27872804
idKind: descriptorHeader.computedIDKind,
2788-
idResolved: descriptorHeader.isComputedIDResolved,
2805+
idResolution: descriptorHeader.computedIDResolution,
27892806
idValueBase: idValueBase,
27902807
idValue: idValue,
27912808
getter: getter,
@@ -2893,7 +2910,7 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern
28932910

28942911
mutating func visitComputedComponent(mutating: Bool,
28952912
idKind: KeyPathComputedIDKind,
2896-
idResolved: Bool,
2913+
idResolution: KeyPathComputedIDResolution,
28972914
idValueBase: UnsafeRawPointer,
28982915
idValue: Int32,
28992916
getter: UnsafeRawPointer,
@@ -3149,7 +3166,7 @@ internal struct InstantiateKeyPathBuffer : KeyPathPatternVisitor {
31493166

31503167
mutating func visitComputedComponent(mutating: Bool,
31513168
idKind: KeyPathComputedIDKind,
3152-
idResolved: Bool,
3169+
idResolution: KeyPathComputedIDResolution,
31533170
idValueBase: UnsafeRawPointer,
31543171
idValue: Int32,
31553172
getter: UnsafeRawPointer,
@@ -3168,7 +3185,7 @@ internal struct InstantiateKeyPathBuffer : KeyPathPatternVisitor {
31683185

31693186
switch idKind {
31703187
case .storedPropertyIndex, .vtableOffset:
3171-
_internalInvariant(idResolved)
3188+
_internalInvariant(idResolution == .resolved)
31723189
// Zero-extend the integer value to get the instantiated id.
31733190
let value = UInt(UInt32(bitPattern: idValue))
31743191
resolvedID = UnsafeRawPointer(bitPattern: value)
@@ -3177,11 +3194,26 @@ internal struct InstantiateKeyPathBuffer : KeyPathPatternVisitor {
31773194
// Resolve the sign-extended relative reference.
31783195
var absoluteID: UnsafeRawPointer? = idValueBase + Int(idValue)
31793196

3180-
// If the pointer ID is "unresolved", then it needs another indirection
3181-
// to get the final value.
3182-
if !idResolved {
3197+
// If the pointer ID is unresolved, then it needs work to get to
3198+
// the final value.
3199+
switch idResolution {
3200+
case .resolved:
3201+
break
3202+
3203+
case .indirectPointer:
3204+
// The pointer in the pattern is an indirect pointer to the real
3205+
// identifier pointer.
31833206
absoluteID = absoluteID.unsafelyUnwrapped
31843207
.load(as: UnsafeRawPointer?.self)
3208+
3209+
case .functionCall:
3210+
// The pointer in the pattern is to a function that generates the
3211+
// identifier pointer.
3212+
typealias Resolver = @convention(c) (UnsafeRawPointer?) -> UnsafeRawPointer?
3213+
let resolverFn = unsafeBitCast(absoluteID.unsafelyUnwrapped,
3214+
to: Resolver.self)
3215+
3216+
absoluteID = resolverFn(patternArgs)
31853217
}
31863218
resolvedID = absoluteID
31873219
}
@@ -3341,7 +3373,7 @@ internal struct ValidatingInstantiateKeyPathBuffer: KeyPathPatternVisitor {
33413373
}
33423374
mutating func visitComputedComponent(mutating: Bool,
33433375
idKind: KeyPathComputedIDKind,
3344-
idResolved: Bool,
3376+
idResolution: KeyPathComputedIDResolution,
33453377
idValueBase: UnsafeRawPointer,
33463378
idValue: Int32,
33473379
getter: UnsafeRawPointer,
@@ -3350,7 +3382,7 @@ internal struct ValidatingInstantiateKeyPathBuffer: KeyPathPatternVisitor {
33503382
externalArgs: UnsafeBufferPointer<Int32>?) {
33513383
sizeVisitor.visitComputedComponent(mutating: mutating,
33523384
idKind: idKind,
3353-
idResolved: idResolved,
3385+
idResolution: idResolution,
33543386
idValueBase: idValueBase,
33553387
idValue: idValue,
33563388
getter: getter,
@@ -3359,7 +3391,7 @@ internal struct ValidatingInstantiateKeyPathBuffer: KeyPathPatternVisitor {
33593391
externalArgs: externalArgs)
33603392
instantiateVisitor.visitComputedComponent(mutating: mutating,
33613393
idKind: idKind,
3362-
idResolved: idResolved,
3394+
idResolution: idResolution,
33633395
idValueBase: idValueBase,
33643396
idValue: idValue,
33653397
getter: getter,

‎test/IRGen/keypaths_objc.sil

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ sil_vtable C {}
1818
sil @x_get : $@convention(thin) (@in_guaranteed C) -> @out NSString
1919

2020
// CHECK: [[KEYPATH_A:@keypath(\..*)?]] = private global
21-
// -- computed, get-only, indirect identifier
22-
// CHECK-SAME: <i32 0x0200_0002>,
23-
// CHECK-SAME: i8** @"\01L_selector(x)"
21+
// -- computed, get-only, function-instantiated identifier
22+
// CHECK-SAME: <i32 0x0200_0001>,
23+
// CHECK-SAME: i8* (i8*)* [[SELECTOR_FN:@[A-Za-z0-9_.]+]] to
2424

2525
// CHECK: [[KEYPATH_B:@keypath(\..*)?]] = private global
2626
// -- class mutable stored property with indirect offset
@@ -45,6 +45,12 @@ entry(%0 : @trivial $@objc_metatype C.Type):
4545
unreachable
4646
}
4747

48+
// CHECK: define private i8* [[SELECTOR_FN]]
49+
// CHECK-NEXT: entry:
50+
// CHECK-NEXT: %1 = load {{.*}}selector(x)
51+
// CHECK-NEXT: ret i8* %1
52+
53+
// CHECK-LABEL: define swiftcc void @objc_stored_property()
4854
sil @objc_stored_property : $@convention(thin) () -> () {
4955
entry:
5056
// CHECK: call %swift.refcounted* @swift_getKeyPath({{.*}} [[KEYPATH_B]]

0 commit comments

Comments
 (0)