Merge pull request #4387 from jckarter/nil-id-as-any-3.0

[3.0] Handle inaccurate ObjC nullability in `id`-as-`Any` bridging.

Author: ematejska

include/swift/AST/KnownDecls.def

@@ -67,6 +67,8 @@ FUNC_DECL(ConditionallyBridgeFromObjectiveCBridgeable,
 
 FUNC_DECL(BridgeAnythingToObjectiveC,
           "_bridgeAnythingToObjectiveC")
+FUNC_DECL(BridgeAnyObjectToAny,
+          "_bridgeAnyObjectToAny")
 
 FUNC_DECL(ConvertToAnyHashable, "_convertToAnyHashable")
 

lib/SILGen/SILGenBridging.cpp

@@ -704,20 +704,25 @@ static ManagedValue emitCBridgedToNativeValue(SILGenFunction &gen,
       gen.getASTContext().getProtocol(KnownProtocolKind::AnyObject)
         ->getDeclaredType())
       && "Any should bridge to AnyObject");
-    
-    // Open the type of the reference and use it to build an Any.
-    auto openedTy = ArchetypeType::getOpened(loweredBridgedTy);
-    auto openedSILTy = SILType::getPrimitiveObjectType(openedTy);
+
     // TODO: Ever need to handle +0 values here?
     assert(v.hasCleanup());
-    auto opened = gen.B.createOpenExistentialRef(loc, v.forward(gen),
-                                                 openedSILTy);
-    auto result = gen.emitTemporaryAllocation(loc, nativeTy);
-    auto resultVal = gen.B.createInitExistentialAddr(loc, result,
-                                                     openedTy, openedSILTy,
-                                                     {});
-    gen.B.createStore(loc, opened, resultVal);
-    return gen.emitManagedRValueWithCleanup(result);
+
+    // Use a runtime call to bridge the AnyObject to Any. We do this instead of
+    // a simple AnyObject-to-Any upcast because the ObjC API may have returned
+    // a null object in spite of its annotation.
+    
+    // Bitcast to Optional. This provides a barrier to the optimizer to prevent
+    // it from attempting to eliminate null checks.
+    auto optionalBridgedTy = OptionalType::get(loweredBridgedTy)
+      ->getCanonicalType();
+    auto optionalV = gen.B.createUncheckedBitCast(loc, v.getValue(),
+                          SILType::getPrimitiveObjectType(optionalBridgedTy));
+    auto optionalMV = ManagedValue(optionalV, v.getCleanup());
+    return gen.emitApplyOfLibraryIntrinsic(loc,
+                           gen.getASTContext().getBridgeAnyObjectToAny(nullptr),
+                           {}, optionalMV, SGFContext())
+      .getAsSingleValue(gen, loc);
   }
 
   return v;

stdlib/public/core/BridgeObjectiveC.swift

@@ -162,6 +162,20 @@ public func _bridgeAnythingToObjectiveC<T>(_ x: T) -> AnyObject {
 @_silgen_name("_swift_bridgeAnythingNonVerbatimToObjectiveC")
 public func _bridgeAnythingNonVerbatimToObjectiveC<T>(_ x: T) -> AnyObject
 
+/// Convert a purportedly-nonnull `id` value from Objective-C into an Any.
+///
+/// Since Objective-C APIs sometimes get their nullability annotations wrong,
+/// this includes a failsafe against nil `AnyObject`s, wrapping them up as
+/// a nil `AnyObject?`-inside-an-`Any`.
+///
+/// COMPILER_INTRINSIC
+public func _bridgeAnyObjectToAny(_ possiblyNullObject: AnyObject?) -> Any {
+  if let nonnullObject = possiblyNullObject {
+    return nonnullObject // AnyObject-in-Any
+  }
+  return possiblyNullObject // AnyObject?-in-Any
+}
+
 /// Convert `x` from its Objective-C representation to its Swift
 /// representation.
 ///

test/Inputs/ObjCBridging/Appliances.h

@@ -17,3 +17,7 @@
 @interface APPManufacturerInfo <DataType> : NSObject
 @property (nonatomic,nonnull,readonly) DataType value;
 @end
+
+@interface APPBroken : NSObject
+@property (nonatomic,nonnull,readonly) id thing;
+@end

test/Inputs/ObjCBridging/Appliances.m

@@ -26,3 +26,11 @@ -(instancetype)init {
 
 @implementation APPManufacturerInfo
 @end
+
+@implementation APPBroken
+
+- (id _Nonnull)thing {
+  return (id _Nonnull)nil;
+}
+
+@end

test/Interpreter/SDK/objc_bridge.swift

@@ -47,5 +47,12 @@ if let f2 = obj as? Refrigerator {
   print("Fridge has temperature \(f2.temperature)")
 }
 
+// Check improper nullability auditing of `id` interfaces. `nil` should come
+// through as a nonnull `Any` without crashing.
+autoreleasepool {
+  let broken = APPBroken()
+  let thing = broken.thing
+}
+
 // CHECK: DONE
 print("DONE")

test/SILGen/objc_bridging_any.swift

@@ -358,10 +358,11 @@ class SwiftIdLover : NSObject, Anyable {
   // CHECK:     bb0(%0 : $AnyObject, %1 : $SwiftIdLover):
   // CHECK-NEXT:  strong_retain %0
   // CHECK-NEXT:  strong_retain %1
-  // CHECK-NEXT:  [[OPENED:%.*]] = open_existential_ref %0
+  // CHECK-NEXT:  [[OPTIONAL:%.*]] = unchecked_ref_cast %0
+  // CHECK-NEXT:  // function_ref
+  // CHECK-NEXT:  [[BRIDGE_TO_ANY:%.*]] = function_ref [[BRIDGE_TO_ANY_FUNC:@.*]] :
   // CHECK-NEXT:  [[RESULT:%.*]] = alloc_stack $Any
-  // CHECK-NEXT:  [[RESULT_VAL:%.*]] = init_existential_addr [[RESULT]]
-  // CHECK-NEXT:  store [[OPENED]] to [[RESULT_VAL]]
+  // CHECK-NEXT:  [[RESULT_VAL:%.*]] = apply [[BRIDGE_TO_ANY]]([[RESULT]], [[OPTIONAL]])
   // CHECK-NEXT:  // function_ref
   // CHECK-NEXT:  [[METHOD:%.*]] = function_ref @_TFC17objc_bridging_any12SwiftIdLover15methodTakingAnyfT1aP__T_
   // CHECK-NEXT:  apply [[METHOD]]([[RESULT]], %1)
@@ -373,7 +374,7 @@ class SwiftIdLover : NSObject, Anyable {
   // CHECK-LABEL: sil hidden @_TFC17objc_bridging_any12SwiftIdLover23methodTakingOptionalAny
 
   // CHECK-LABEL: sil hidden [thunk] @_TToFC17objc_bridging_any12SwiftIdLover23methodTakingOptionalAny
-  // CHECK: init_existential_addr %11 : $*Any, $@opened({{.*}}) AnyObject
+  // CHECK: function_ref [[BRIDGE_TO_ANY_FUNC]]
 
   // CHECK-LABEL: sil hidden @_TFC17objc_bridging_any12SwiftIdLover26methodTakingBlockTakingAnyfFP_T_T_ : $@convention(method) (@owned @callee_owned (@in Any) -> (), @guaranteed SwiftIdLover) -> ()
 
@@ -427,10 +428,11 @@ class SwiftIdLover : NSObject, Anyable {
   // CHECK-NEXT:  [[FUNCTION:%.*]] = load [[BLOCK_STORAGE_ADDR]]
   // CHECK-NEXT:  strong_retain [[FUNCTION]]
   // CHECK-NEXT:  strong_retain %1
-  // CHECK-NEXT:  [[OPENED:%.*]] = open_existential_ref %1
+  // CHECK-NEXT:  [[OPTIONAL:%.*]] = unchecked_ref_cast %1
+  // CHECK-NEXT:  // function_ref
+  // CHECK-NEXT:  [[BRIDGE_TO_ANY:%.*]] = function_ref [[BRIDGE_TO_ANY_FUNC:@.*]] :
   // CHECK-NEXT:  [[RESULT:%.*]] = alloc_stack $Any
-  // CHECK-NEXT:  [[RESULT_VAL:%.*]] = init_existential_addr [[RESULT]] : $*Any
-  // CHECK-NEXT:  store [[OPENED]] to [[RESULT_VAL]]
+  // CHECK-NEXT:  [[RESULT_VAL:%.*]] = apply [[BRIDGE_TO_ANY]]([[RESULT]], [[OPTIONAL]])
   // CHECK-NEXT:  apply [[FUNCTION]]([[RESULT]])
   // CHECK-NEXT:  [[VOID:%.*]] = tuple ()
   // CHECK-NEXT:  dealloc_stack [[RESULT]]
@@ -456,10 +458,11 @@ class SwiftIdLover : NSObject, Anyable {
   // CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRXFdCb__aPs9AnyObject__XFo__iP__ : $@convention(thin) (@owned @convention(block) () -> @autoreleased AnyObject) -> @out Any
   // CHECK:     bb0(%0 : $*Any, %1 : $@convention(block) () -> @autoreleased AnyObject):
   // CHECK-NEXT:  [[BRIDGED:%.*]] = apply %1()
-  // CHECK-NEXT:  [[OPENED:%.*]] = open_existential_ref [[BRIDGED]]
+  // CHECK-NEXT:  [[OPTIONAL:%.*]] = unchecked_ref_cast [[BRIDGED]]
+  // CHECK-NEXT:  // function_ref
+  // CHECK-NEXT:  [[BRIDGE_TO_ANY:%.*]] = function_ref [[BRIDGE_TO_ANY_FUNC:@.*]] :
   // CHECK-NEXT:  [[RESULT:%.*]] = alloc_stack $Any
-  // CHECK-NEXT:  [[RESULT_VAL:%.*]] = init_existential_addr [[RESULT]]
-  // CHECK-NEXT:  store [[OPENED]] to [[RESULT_VAL]]
+  // CHECK-NEXT:  [[RESULT_VAL:%.*]] = apply [[BRIDGE_TO_ANY]]([[RESULT]], [[OPTIONAL]])
 
   // TODO: Should elide the copy
   // CHECK-NEXT:  copy_addr [take] [[RESULT]] to [initialization] %0