Skip to content

Commit e49fa5c

Browse files
committed
SILGen: Guard against unexpected nulls passed into ObjC overrides.
We want to treat arguments to ObjC override and protocol conformance thunks like "call results", since they might be called from ObjC code that doesn't fulfill its nullability promises in practice. Fixes SR-7240 | rdar://problem/38675815.
1 parent 8b9ffd8 commit e49fa5c

File tree

3 files changed

+55
-38
lines changed

3 files changed

+55
-38
lines changed

lib/SILGen/SILGenBridging.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,12 +1336,16 @@ static SILFunctionType *emitObjCThunkArguments(SILGenFunction &SGF,
13361336

13371337
assert(bridgedArgs.size() == nativeInputs.size());
13381338
for (unsigned i = 0, size = bridgedArgs.size(); i < size; ++i) {
1339+
// Consider the bridged values to be "call results" since they're coming
1340+
// from potentially nil-unsound ObjC callers.
13391341
ManagedValue native =
13401342
SGF.emitBridgedToNativeValue(loc,
1341-
bridgedArgs[i],
1342-
bridgedFormalTypes[i],
1343-
nativeFormalTypes[i],
1344-
swiftFnTy->getParameters()[i].getSILStorageType());
1343+
bridgedArgs[i],
1344+
bridgedFormalTypes[i],
1345+
nativeFormalTypes[i],
1346+
swiftFnTy->getParameters()[i].getSILStorageType(),
1347+
SGFContext(),
1348+
/*isCallResult*/ true);
13451349
SILValue argValue;
13461350

13471351
if (nativeInputs[i].isConsumed()) {

test/Inputs/clang-importer-sdk/usr/include/Foundation.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,6 +1076,10 @@ extern NSString *NSHTTPRequestKey;
10761076

10771077
@end
10781078

1079+
@protocol NSIdLoving
1080+
- (void)takesIdViaProtocol:(id _Nonnull)x;
1081+
@end
1082+
10791083
#define NSTimeIntervalSince1970 978307200.0
10801084
#define NS_DO_SOMETHING 17
10811085

test/SILGen/objc_bridging_any.swift

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -module-name objc_bridging_any -Xllvm -sil-print-debuginfo -emit-silgen -enable-sil-ownership %s | %FileCheck %s
32
// REQUIRES: objc_interop
43

@@ -414,7 +413,7 @@ protocol Anyable {
414413
// Make sure we generate correct bridging thunks
415414
class SwiftIdLover : NSObject, Anyable {
416415

417-
func methodReturningAny() -> Any {}
416+
func methodReturningAny() -> Any { fatalError() }
418417
// SEMANTIC ARC TODO: This is another case of pattern matching the body of one
419418
// function in a different function... Just pattern match the unreachable case
420419
// to preserve behavior. We should check if it is correct.
@@ -440,31 +439,19 @@ class SwiftIdLover : NSObject, Anyable {
440439
// CHECK: return [[OBJC_RESULT]]
441440
// CHECK: } // end sil function '$S17objc_bridging_any12SwiftIdLoverC18methodReturningAnyypyFTo'
442441

443-
func methodReturningOptionalAny() -> Any? {}
442+
func methodReturningOptionalAny() -> Any? { fatalError() }
444443
// CHECK-LABEL: sil hidden @$S17objc_bridging_any12SwiftIdLoverC26methodReturningOptionalAnyypSgyF
445444
// CHECK-LABEL: sil hidden [thunk] @$S17objc_bridging_any12SwiftIdLoverC26methodReturningOptionalAnyypSgyFTo
446445
// CHECK: function_ref @$Ss27_bridgeAnythingToObjectiveC{{.*}}F
447446

448-
@objc func methodTakingAny(a: Any) {}
447+
@objc func methodTakingAny(a: Any) { fatalError() }
449448
// CHECK-LABEL: sil hidden [thunk] @$S17objc_bridging_any12SwiftIdLoverC15methodTakingAny1ayyp_tFTo : $@convention(objc_method) (AnyObject, SwiftIdLover) -> ()
450449
// CHECK: bb0([[ARG:%.*]] : @unowned $AnyObject, [[SELF:%.*]] : @unowned $SwiftIdLover):
451-
// CHECK-NEXT: [[ARG_COPY:%.*]] = copy_value [[ARG]]
452-
// CHECK-NEXT: [[SELF_COPY:%.*]] = copy_value [[SELF]]
453-
// CHECK-NEXT: [[OPENED_SELF:%.*]] = open_existential_ref [[ARG_COPY]]
454-
// CHECK-NEXT: [[RESULT:%.*]] = alloc_stack $Any
455-
// CHECK-NEXT: [[INIT:%.*]] = init_existential_addr [[RESULT]] : $*Any
456-
// CHECK-NEXT: store [[OPENED_SELF]] to [init] [[INIT]]
457-
// CHECK-NEXT: [[BORROWED_SELF_COPY:%.*]] = begin_borrow [[SELF_COPY]]
458-
// CHECK-NEXT: // function_ref
459-
// CHECK-NEXT: [[METHOD:%.*]] = function_ref @$S17objc_bridging_any12SwiftIdLoverC15methodTakingAny1ayyp_tF
460-
// CHECK-NEXT: apply [[METHOD]]([[RESULT]], [[BORROWED_SELF_COPY]])
461-
// CHECK-NEXT: end_borrow [[BORROWED_SELF_COPY]] from [[SELF_COPY]]
462-
// CHECK-NEXT: destroy_addr [[RESULT]]
463-
// CHECK-NEXT: dealloc_stack [[RESULT]]
464-
// CHECK-NEXT: destroy_value [[SELF_COPY]]
465-
// CHECK-NEXT: return
450+
// CHECK: function_ref [[BRIDGE_ANYOBJECT_TO_ANY:@\$Ss018_bridgeAnyObjectToB0yypyXlSgF]] : $@convention(thin) (@guaranteed Optional<AnyObject>) -> @out Any
451+
// CHECK: [[METHOD:%.*]] = function_ref @$S17objc_bridging_any12SwiftIdLoverC15methodTakingAny1ayyp_tF
452+
// CHECK-NEXT: apply [[METHOD]]([[RESULT:%.*]], [[BORROWED_SELF_COPY:%.*]]) :
466453

467-
func methodTakingOptionalAny(a: Any?) {}
454+
func methodTakingOptionalAny(a: Any?) { fatalError() }
468455
// CHECK-LABEL: sil hidden @$S17objc_bridging_any12SwiftIdLoverC23methodTakingOptionalAny1ayypSg_tF
469456

470457
// CHECK-LABEL: sil hidden [thunk] @$S17objc_bridging_any12SwiftIdLoverC23methodTakingOptionalAny1ayypSg_tFTo
@@ -502,7 +489,7 @@ class SwiftIdLover : NSObject, Anyable {
502489
// CHECK-NEXT: dealloc_stack [[TMP]]
503490
// CHECK-NEXT: return [[VOID]]
504491

505-
@objc func methodTakingBlockTakingAny(_: (Any) -> ()) {}
492+
@objc func methodTakingBlockTakingAny(_: (Any) -> ()) { fatalError() }
506493

507494
// CHECK-LABEL: sil hidden @$S17objc_bridging_any12SwiftIdLoverC29methodReturningBlockTakingAnyyypcyF : $@convention(method) (@guaranteed SwiftIdLover) -> @owned @callee_guaranteed (@in_guaranteed Any) -> ()
508495

@@ -543,9 +530,9 @@ class SwiftIdLover : NSObject, Anyable {
543530
// CHECK-NEXT: destroy_value [[FUNCTION]]
544531
// CHECK-NEXT: return [[VOID]] : $()
545532

546-
@objc func methodTakingBlockTakingOptionalAny(_: (Any?) -> ()) {}
533+
@objc func methodTakingBlockTakingOptionalAny(_: (Any?) -> ()) { fatalError() }
547534

548-
@objc func methodReturningBlockTakingAny() -> ((Any) -> ()) {}
535+
@objc func methodReturningBlockTakingAny() -> ((Any) -> ()) { fatalError() }
549536

550537
// CHECK-LABEL: sil hidden @$S17objc_bridging_any12SwiftIdLoverC29methodTakingBlockReturningAnyyyypyXEF : $@convention(method) (@noescape @callee_guaranteed () -> @out Any, @guaranteed SwiftIdLover) -> () {
551538

@@ -579,9 +566,9 @@ class SwiftIdLover : NSObject, Anyable {
579566
// CHECK-NEXT: destroy_value [[OPTIONAL]]
580567
// CHECK-NEXT: return [[EMPTY]]
581568

582-
@objc func methodReturningBlockTakingOptionalAny() -> ((Any?) -> ()) {}
569+
@objc func methodReturningBlockTakingOptionalAny() -> ((Any?) -> ()) { fatalError() }
583570

584-
@objc func methodTakingBlockReturningAny(_: () -> Any) {}
571+
@objc func methodTakingBlockReturningAny(_: () -> Any) { fatalError() }
585572

586573
// CHECK-LABEL: sil hidden @$S17objc_bridging_any12SwiftIdLoverC020methodReturningBlockH3AnyypycyF : $@convention(method) (@guaranteed SwiftIdLover) -> @owned @callee_guaranteed () -> @out Any
587574

@@ -626,26 +613,26 @@ class SwiftIdLover : NSObject, Anyable {
626613
// CHECK-NEXT: destroy_value [[FUNCTION]]
627614
// CHECK-NEXT: return [[BRIDGED]]
628615

629-
@objc func methodTakingBlockReturningOptionalAny(_: () -> Any?) {}
616+
@objc func methodTakingBlockReturningOptionalAny(_: () -> Any?) { fatalError() }
630617

631-
@objc func methodReturningBlockReturningAny() -> (() -> Any) {}
618+
@objc func methodReturningBlockReturningAny() -> (() -> Any) { fatalError() }
632619

633-
@objc func methodReturningBlockReturningOptionalAny() -> (() -> Any?) {}
620+
@objc func methodReturningBlockReturningOptionalAny() -> (() -> Any?) { fatalError() }
634621
// CHECK-LABEL: sil shared [transparent] [serializable] [reabstraction_thunk] @$SypSgIegr_yXlSgIeyBa_TR
635622
// CHECK: function_ref @$Ss27_bridgeAnythingToObjectiveC{{.*}}F
636623

637-
override init() { super.init() }
638-
@objc dynamic required convenience init(any: Any) { self.init() }
639-
@objc dynamic required convenience init(anyMaybe: Any?) { self.init() }
624+
override init() { fatalError() }
625+
@objc dynamic required convenience init(any: Any) { fatalError() }
626+
@objc dynamic required convenience init(anyMaybe: Any?) { fatalError() }
640627
@objc dynamic var anyProperty: Any
641628
@objc dynamic var maybeAnyProperty: Any?
642629

643-
subscript(_: IndexForAnySubscript) -> Any { get {} set {} }
630+
subscript(_: IndexForAnySubscript) -> Any { get { fatalError() } set { fatalError() } }
644631

645-
@objc func methodReturningAnyOrError() throws -> Any {}
632+
@objc func methodReturningAnyOrError() throws -> Any { fatalError() }
646633
}
647634

648-
class IndexForAnySubscript {}
635+
class IndexForAnySubscript { }
649636

650637
func dynamicLookup(x: AnyObject) {
651638
_ = x.anyProperty
@@ -685,6 +672,28 @@ func bridgeOptionalFunctionToAnyObject(fn: (() -> ())?) -> AnyObject {
685672
return fn as AnyObject
686673
}
687674

675+
// When bridging `id _Nonnull` values incoming from unknown ObjC code,
676+
// turn them into `Any` using a runtime call that defends against the
677+
// possibility they still may be nil.
678+
679+
// CHECK-LABEL: sil hidden @$S17objc_bridging_any22bridgeIncomingAnyValueyypSo9NSIdLoverCF
680+
func bridgeIncomingAnyValue(_ receiver: NSIdLover) -> Any {
681+
// CHECK: function_ref [[BRIDGE_ANYOBJECT_TO_ANY]]
682+
return receiver.makesId()
683+
}
684+
685+
class SwiftAnyEnjoyer: NSIdLover, NSIdLoving {
686+
// CHECK-LABEL: sil hidden [thunk] @$S17objc_bridging_any15SwiftAnyEnjoyerC7takesIdyyypFTo
687+
// CHECK: function_ref [[BRIDGE_ANYOBJECT_TO_ANY]]
688+
override func takesId(_ x: Any) { }
689+
690+
// CHECK-LABEL: sil hidden [thunk] @$S17objc_bridging_any15SwiftAnyEnjoyerC7takesId11viaProtocolyyp_tFTo
691+
// CHECK: function_ref [[BRIDGE_ANYOBJECT_TO_ANY]]
692+
func takesId(viaProtocol x: Any) { }
693+
}
694+
695+
696+
688697
// CHECK-LABEL: sil_witness_table shared [serialized] GenericOption: Hashable module objc_generics {
689698
// CHECK-NEXT: base_protocol Equatable: GenericOption: Equatable module objc_generics
690699
// CHECK-NEXT: method #Hashable.hashValue!getter.1: {{.*}} : @$SSo13GenericOptionas8HashableSCsACP9hashValueSivgTW

0 commit comments

Comments
 (0)