KeyPaths: Don't relative-reference selector refs.

include/swift/ABI/KeyPath.h

@@ -190,17 +190,17 @@ class KeyPathComponentHeader {
     VTableOffset,
   };
 
-  constexpr static uint32_t
-  getResolutionStrategy(ComputedPropertyIDKind idKind) {
-    return idKind == Pointer ? _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
-         : (assert("no resolution strategy implemented" && false), 0);
-  }
+  enum ComputedPropertyIDResolution {
+    Resolved,
+    IndirectPointer,
+    FunctionCall,
+  };
 
   constexpr static KeyPathComponentHeader
   forComputedProperty(ComputedPropertyKind kind,
                       ComputedPropertyIDKind idKind,
                       bool hasArguments,
-                      bool resolvedID) {
+                      ComputedPropertyIDResolution resolution) {
     return KeyPathComponentHeader(
       (_SwiftKeyPathComponentHeader_ComputedTag
         << _SwiftKeyPathComponentHeader_DiscriminatorShift)
@@ -213,8 +213,10 @@ class KeyPathComponentHeader {
       | (idKind == VTableOffset
            ? _SwiftKeyPathComponentHeader_ComputedIDByVTableOffsetFlag : 0)
       | (hasArguments ? _SwiftKeyPathComponentHeader_ComputedHasArgumentsFlag : 0)
-      | (resolvedID ? _SwiftKeyPathComponentHeader_ComputedIDResolved
-                    : getResolutionStrategy(idKind)));
+      | (resolution == Resolved ? _SwiftKeyPathComponentHeader_ComputedIDResolved
+       : resolution == IndirectPointer ? _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
+       : resolution == FunctionCall ? _SwiftKeyPathComponentHeader_ComputedIDUnresolvedFunctionCall
+       : (assert(false && "invalid resolution"), 0)));
   }
 
   constexpr static KeyPathComponentHeader

lib/IRGen/GenKeyPath.cpp

@@ -36,6 +36,7 @@
 #include "TypeInfo.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/Function.h"
 #include "swift/SIL/SILInstruction.h"
 #include "swift/SIL/SILLocation.h"
 #include "swift/SIL/TypeLowering.h"
@@ -947,7 +948,7 @@ emitKeyPathComponent(IRGenModule &IGM,
     auto id = component.getComputedPropertyId();
     KeyPathComponentHeader::ComputedPropertyIDKind idKind;
     llvm::Constant *idValue;
-    bool idResolved;
+    KeyPathComponentHeader::ComputedPropertyIDResolution idResolution;
     switch (id.getKind()) {
     case KeyPathPatternComponent::ComputedPropertyId::Function: {
       idKind = KeyPathComponentHeader::Pointer;
@@ -957,7 +958,9 @@ emitKeyPathComponent(IRGenModule &IGM,
       idValue = idRef.getValue();
       // If we got an indirect reference, we'll need to resolve it at
       // instantiation time.
-      idResolved = !idRef.isIndirect();
+      idResolution = idRef.isIndirect()
+        ? KeyPathComponentHeader::IndirectPointer
+        : KeyPathComponentHeader::Resolved;
       break;
     }
     case KeyPathPatternComponent::ComputedPropertyId::DeclRef: {
@@ -969,8 +972,35 @@ emitKeyPathComponent(IRGenModule &IGM,
       if (declRef.isForeign) {
         assert(IGM.ObjCInterop && "foreign keypath component w/o objc interop?!");
         idKind = KeyPathComponentHeader::Pointer;
-        idValue = IGM.getAddrOfObjCSelectorRef(declRef);
-        idResolved = false;
+        // FIXME: In non-JIT mode, ideally we would just refer to the selector
+        // reference variable here with an indirectpointer resolution,
+        // but ld64 section coalescing on the __objc_sel section can break
+        // relative references (and on some platforms, mach-o just doesn't
+        // support the necessary relocations).
+        // As a workaround, generate a stub function to resolve the selector.
+        //
+        // Note that we'd need to do this anyway in JIT mode because we would
+        // need to unique the selector at runtime anyway.
+        auto selectorName = IGM.getObjCSelectorName(declRef);
+        llvm::Type *fnParams[] = {IGM.Int8PtrTy};
+        auto fnTy = llvm::FunctionType::get(IGM.Int8PtrTy, fnParams, false);
+        SmallString<32> fnName;
+        fnName.append("keypath_get_selector_");
+        fnName.append(selectorName);
+        auto fn = cast<llvm::Function>(
+          IGM.Module.getOrInsertFunction(fnName, fnTy));
+        if (fn->empty()) {
+          fn->setLinkage(llvm::Function::PrivateLinkage);
+          IRGenFunction subIGF(IGM, fn);
+          if (IGM.DebugInfo)
+            IGM.DebugInfo->emitArtificialFunction(subIGF, fn);
+
+          auto selectorValue = subIGF.emitObjCSelectorRefLoad(selectorName);
+          subIGF.Builder.CreateRet(selectorValue);
+        }
+
+        idValue = fn;
+        idResolution = KeyPathComponentHeader::FunctionCall;
       } else {
         if (auto overridden = declRef.getOverriddenVTableEntry())
           declRef = overridden;
@@ -989,7 +1019,9 @@ emitKeyPathComponent(IRGenModule &IGM,
             LinkEntity::forMethodDescriptor(declRef));
 
           idValue = idRef.getValue();
-          idResolved = !idRef.isIndirect();
+          idResolution = idRef.isIndirect()
+            ? KeyPathComponentHeader::IndirectPointer
+            : KeyPathComponentHeader::Resolved;
           break;
         }
 
@@ -1000,7 +1032,7 @@ emitKeyPathComponent(IRGenModule &IGM,
         auto index = protoInfo.getFunctionIndex(
                              cast<AbstractFunctionDecl>(declRef.getDecl()));
         idValue = llvm::ConstantInt::get(IGM.SizeTy, -index.getValue());
-        idResolved = true;
+        idResolution = KeyPathComponentHeader::Resolved;
       }
       break;
     }
@@ -1013,7 +1045,7 @@ emitKeyPathComponent(IRGenModule &IGM,
         // Scan the stored properties of the struct to find the index. We should
         // only ever use a struct field as a uniquing key from inside the
         // struct's own module, so this is OK.
-        idResolved = true;
+        idResolution = KeyPathComponentHeader::Resolved;
         Optional<unsigned> structIdx;
         unsigned i = 0;
         for (auto storedProp : struc->getStoredProperties()) {
@@ -1034,7 +1066,7 @@ emitKeyPathComponent(IRGenModule &IGM,
         case FieldAccess::ConstantDirect:
         case FieldAccess::ConstantIndirect:
         case FieldAccess::NonConstantDirect:
-          idResolved = true;
+          idResolution = KeyPathComponentHeader::Resolved;
           idValue = llvm::ConstantInt::get(IGM.SizeTy,
                                        getClassFieldIndex(classDecl, property));
           break;
@@ -1047,7 +1079,7 @@ emitKeyPathComponent(IRGenModule &IGM,
     }
 
     auto header = KeyPathComponentHeader::forComputedProperty(componentKind,
-                                      idKind, !isInstantiableOnce, idResolved);
+                                     idKind, !isInstantiableOnce, idResolution);
 
     fields.addInt32(header.getData());
     switch (idKind) {

lib/IRGen/GenObjC.cpp

@@ -534,6 +534,11 @@ llvm::Constant *IRGenModule::getAddrOfObjCSelectorRef(SILDeclRef method) {
   return getAddrOfObjCSelectorRef(Selector(method).str());
 }
 
+std::string IRGenModule::getObjCSelectorName(SILDeclRef method) {
+  assert(method.isForeign);
+  return Selector(method).str();
+}
+
 static llvm::Value *emitSuperArgument(IRGenFunction &IGF,
                                       bool isInstanceMethod,
                                       llvm::Value *selfValue,

lib/IRGen/IRGenModule.h

@@ -828,6 +828,7 @@ class IRGenModule {
   llvm::Constant *getAddrOfGlobalUTF16String(StringRef utf8);
   llvm::Constant *getAddrOfObjCSelectorRef(StringRef selector);
   llvm::Constant *getAddrOfObjCSelectorRef(SILDeclRef method);
+  std::string getObjCSelectorName(SILDeclRef method);
   llvm::Constant *getAddrOfObjCMethodName(StringRef methodName);
   llvm::Constant *getAddrOfObjCProtocolRecord(ProtocolDecl *proto,
                                               ForDefinition_t forDefinition);

stdlib/public/SwiftShims/KeyPath.h

@@ -106,6 +106,8 @@ static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDResolved
   = 0x00000000U;
 static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
   = 0x00000002U;
+static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDUnresolvedFunctionCall
+  = 0x00000001U;
 
 extern const void *_Nonnull (swift_keyPathGenericWitnessTable[]);
 

stdlib/public/core/KeyPath.swift

@@ -739,6 +739,12 @@ internal enum KeyPathComputedIDKind {
   case vtableOffset
 }
 
+internal enum KeyPathComputedIDResolution {
+  case resolved
+  case indirectPointer
+  case functionCall
+}
+
 internal struct RawKeyPathComponent {
   internal init(header: Header, body: UnsafeRawBufferPointer) {
     self.header = header
@@ -890,9 +896,20 @@ internal struct RawKeyPathComponent {
     internal static var computedIDUnresolvedIndirectPointer: UInt32 {
       return _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
     }
-    internal var isComputedIDResolved: Bool {
-      return
-        payload & Header.computedIDResolutionMask == Header.computedIDResolved
+    internal static var computedIDUnresolvedFunctionCall: UInt32 {
+      return _SwiftKeyPathComponentHeader_ComputedIDUnresolvedFunctionCall
+    }
+    internal var computedIDResolution: KeyPathComputedIDResolution {
+      switch payload & Header.computedIDResolutionMask {
+      case Header.computedIDResolved:
+        return .resolved
+      case Header.computedIDUnresolvedIndirectPointer:
+        return .indirectPointer
+      case Header.computedIDUnresolvedFunctionCall:
+        return .functionCall
+      default:
+        _internalInvariantFailure("invalid key path resolution")
+      }
     }
 
     internal var _value: UInt32
@@ -2512,7 +2529,7 @@ internal protocol KeyPathPatternVisitor {
                                      offset: KeyPathPatternStoredOffset)
   mutating func visitComputedComponent(mutating: Bool,
                                        idKind: KeyPathComputedIDKind,
-                                       idResolved: Bool,
+                                       idResolution: KeyPathComputedIDResolution,
                                        idValueBase: UnsafeRawPointer,
                                        idValue: Int32,
                                        getter: UnsafeRawPointer,
@@ -2694,7 +2711,7 @@ internal func _walkKeyPathPattern<W: KeyPathPatternVisitor>(
 
       walker.visitComputedComponent(mutating: header.isComputedMutating,
                                     idKind: header.computedIDKind,
-                                    idResolved: header.isComputedIDResolved,
+                                    idResolution: header.computedIDResolution,
                                     idValueBase: idValueBase,
                                     idValue: idValue,
                                     getter: getter,
@@ -2785,7 +2802,7 @@ internal func _walkKeyPathPattern<W: KeyPathPatternVisitor>(
         walker.visitComputedComponent(
           mutating: descriptorHeader.isComputedMutating,
           idKind: descriptorHeader.computedIDKind,
-          idResolved: descriptorHeader.isComputedIDResolved,
+          idResolution: descriptorHeader.computedIDResolution,
           idValueBase: idValueBase,
           idValue: idValue,
           getter: getter,
@@ -2893,7 +2910,7 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern
 
   mutating func visitComputedComponent(mutating: Bool,
                                    idKind: KeyPathComputedIDKind,
-                                   idResolved: Bool,
+                                   idResolution: KeyPathComputedIDResolution,
                                    idValueBase: UnsafeRawPointer,
                                    idValue: Int32,
                                    getter: UnsafeRawPointer,
@@ -3149,7 +3166,7 @@ internal struct InstantiateKeyPathBuffer : KeyPathPatternVisitor {
 
   mutating func visitComputedComponent(mutating: Bool,
                                    idKind: KeyPathComputedIDKind,
-                                   idResolved: Bool,
+                                   idResolution: KeyPathComputedIDResolution,
                                    idValueBase: UnsafeRawPointer,
                                    idValue: Int32,
                                    getter: UnsafeRawPointer,
@@ -3168,7 +3185,7 @@ internal struct InstantiateKeyPathBuffer : KeyPathPatternVisitor {
 
     switch idKind {
     case .storedPropertyIndex, .vtableOffset:
-      _internalInvariant(idResolved)
+      _internalInvariant(idResolution == .resolved)
       // Zero-extend the integer value to get the instantiated id.
       let value = UInt(UInt32(bitPattern: idValue))
       resolvedID = UnsafeRawPointer(bitPattern: value)
@@ -3177,11 +3194,26 @@ internal struct InstantiateKeyPathBuffer : KeyPathPatternVisitor {
       // Resolve the sign-extended relative reference.
       var absoluteID: UnsafeRawPointer? = idValueBase + Int(idValue)
 
-      // If the pointer ID is "unresolved", then it needs another indirection
-      // to get the final value.
-      if !idResolved {
+      // If the pointer ID is unresolved, then it needs work to get to
+      // the final value.
+      switch idResolution {
+      case .resolved:
+        break
+
+      case .indirectPointer:
+        // The pointer in the pattern is an indirect pointer to the real
+        // identifier pointer.
         absoluteID = absoluteID.unsafelyUnwrapped
           .load(as: UnsafeRawPointer?.self)
+
+      case .functionCall:
+        // The pointer in the pattern is to a function that generates the
+        // identifier pointer.
+        typealias Resolver = @convention(c) (UnsafeRawPointer?) -> UnsafeRawPointer?
+        let resolverFn = unsafeBitCast(absoluteID.unsafelyUnwrapped,
+                                       to: Resolver.self)
+
+        absoluteID = resolverFn(patternArgs)
       }
       resolvedID = absoluteID
     }
@@ -3341,7 +3373,7 @@ internal struct ValidatingInstantiateKeyPathBuffer: KeyPathPatternVisitor {
   }
   mutating func visitComputedComponent(mutating: Bool,
                                    idKind: KeyPathComputedIDKind,
-                                   idResolved: Bool,
+                                   idResolution: KeyPathComputedIDResolution,
                                    idValueBase: UnsafeRawPointer,
                                    idValue: Int32,
                                    getter: UnsafeRawPointer,
@@ -3350,7 +3382,7 @@ internal struct ValidatingInstantiateKeyPathBuffer: KeyPathPatternVisitor {
                                    externalArgs: UnsafeBufferPointer<Int32>?) {
     sizeVisitor.visitComputedComponent(mutating: mutating,
                                        idKind: idKind,
-                                       idResolved: idResolved,
+                                       idResolution: idResolution,
                                        idValueBase: idValueBase,
                                        idValue: idValue,
                                        getter: getter,
@@ -3359,7 +3391,7 @@ internal struct ValidatingInstantiateKeyPathBuffer: KeyPathPatternVisitor {
                                        externalArgs: externalArgs)
     instantiateVisitor.visitComputedComponent(mutating: mutating,
                                        idKind: idKind,
-                                       idResolved: idResolved,
+                                       idResolution: idResolution,
                                        idValueBase: idValueBase,
                                        idValue: idValue,
                                        getter: getter,

test/IRGen/keypaths_objc.sil

@@ -18,9 +18,9 @@ sil_vtable C {}
 sil @x_get : $@convention(thin) (@in_guaranteed C) -> @out NSString
 
 // CHECK: [[KEYPATH_A:@keypath(\..*)?]] = private global
-// --             computed, get-only, indirect identifier
-// CHECK-SAME: <i32 0x0200_0002>,
-// CHECK-SAME: i8** @"\01L_selector(x)"
+// --             computed, get-only, function-instantiated identifier
+// CHECK-SAME: <i32 0x0200_0001>,
+// CHECK-SAME: i8* (i8*)* [[SELECTOR_FN:@[A-Za-z0-9_.]+]] to
 
 // CHECK: [[KEYPATH_B:@keypath(\..*)?]] = private global
 // --             class mutable stored property with indirect offset
@@ -45,6 +45,12 @@ entry(%0 : $@objc_metatype C.Type):
   unreachable
 }
 
+// CHECK: define private i8* [[SELECTOR_FN]]
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %1 = load {{.*}}selector(x)
+// CHECK-NEXT:   ret i8* %1
+
+// CHECK-LABEL: define swiftcc void @objc_stored_property()
 sil @objc_stored_property : $@convention(thin) () -> () {
 entry:
   // CHECK: call %swift.refcounted* @swift_getKeyPath({{.*}} [[KEYPATH_B]]