Skip to content

Commit b972815

Browse files
committed
SILGen: Fix no escape verification if the closure throws an exception
It is safe to omit the retain/release dance in the reabstraction thunk because we know we have an aditional reference outstanding for the is_escaping verification. The problem with throwing an objc exception inside a noescape closure is that we verify the reference count of the closure sentinel. The reabstraction thunk would increase the reference count call the implementation function that then throws skipping the decrement. rdar://40857699
1 parent e8b08de commit b972815

File tree

5 files changed

+74
-3
lines changed

5 files changed

+74
-3
lines changed

lib/SILGen/SILGenBridging.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,8 @@ static void buildFuncToBlockInvokeBody(SILGenFunction &SGF,
364364
CanAnyFunctionType formalBlockType,
365365
CanSILFunctionType funcTy,
366366
CanSILFunctionType blockTy,
367-
CanSILBlockStorageType blockStorageTy) {
367+
CanSILBlockStorageType blockStorageTy,
368+
bool isUnretainedClosureSafe) {
368369
Scope scope(SGF.Cleanups, CleanupLocation::get(loc));
369370
SILBasicBlock *entry = &*SGF.F.begin();
370371
SILFunctionConventions blockConv(blockTy, SGF.SGM.M);
@@ -388,7 +389,9 @@ static void buildFuncToBlockInvokeBody(SILGenFunction &SGF,
388389
auto storage = entry->createFunctionArgument(storageAddrTy);
389390
auto capture = SGF.B.createProjectBlockStorage(loc, storage);
390391
auto &funcTL = SGF.getTypeLowering(funcTy);
391-
auto fn = SGF.emitLoad(loc, capture, funcTL, SGFContext(), IsNotTake);
392+
auto fn = isUnretainedClosureSafe
393+
? SGF.emitManagedLoadBorrow(loc, capture)
394+
: SGF.emitLoad(loc, capture, funcTL, SGFContext(), IsNotTake);
392395

393396
// Collect the block arguments, which may have nonstandard conventions.
394397
assert(blockTy->getParameters().size() == funcTy->getParameters().size()
@@ -583,8 +586,11 @@ ManagedValue SILGenFunction::emitFuncToBlock(SILLocation loc,
583586
thunk->setGenericEnvironment(genericEnv);
584587
SILGenFunction thunkSGF(SGM, *thunk);
585588
auto loc = RegularLocation::getAutoGeneratedLocation();
589+
// Not retaining the closure in the reabstraction thunk is safe if we hold
590+
// another reference for the is_escaping sentinel.
586591
buildFuncToBlockInvokeBody(thunkSGF, loc, funcType, blockType,
587-
loweredFuncTy, loweredBlockTy, storageTy);
592+
loweredFuncTy, loweredBlockTy, storageTy,
593+
useWithoutEscapingVerifcation);
588594
}
589595

590596
// Form the block on the stack.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#import <Foundation/Foundation.h>
2+
3+
@interface ExceptionCatcher : NSObject
4+
- (NSException* _Nullable)tryBlock:(__attribute__((noescape)) void(^ _Nonnull)(void))unsafeBlock NS_SWIFT_NAME(tryBlock(_:));
5+
@end
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#import "ObjCException.h"
2+
3+
@implementation ExceptionCatcher
4+
- (NSException* _Nullable) tryBlock:(__attribute__((noescape)) void(^_Nonnull)(void))unsafeBlock {
5+
@try {
6+
unsafeBlock();
7+
}
8+
@catch (NSException *exception) {
9+
return exception;
10+
}
11+
return nil;
12+
}
13+
@end
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// RUN: %empty-directory(%t)
2+
//
3+
// RUN: %target-clang -fobjc-arc %S/Inputs/ObjCException.m -c -o %t/ObjCException.o
4+
// RUN: %target-build-swift -import-objc-header %S/Inputs/ObjcException.h -Xlinker %t/ObjCException.o %s -o %t/a.out
5+
// RUN: %target-run %t/a.out
6+
7+
// REQUIRES: executable_test
8+
// REQUIRES: objc_interop
9+
10+
import Foundation
11+
import StdlibUnittest
12+
13+
var ThrowingTestSuite = TestSuite("Throwing")
14+
15+
16+
ThrowingTestSuite.test("noescape verification") {
17+
let catcher = ExceptionCatcher()
18+
let e = catcher.tryBlock {
19+
NSException(name: NSExceptionName(rawValue: "Flames"), reason: "Fire", userInfo: nil).raise()
20+
}
21+
expectEqual(e!.reason, "Fire")
22+
}
23+
24+
25+
runAllTests()

test/SILGen/objc_thunks.swift

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,3 +570,25 @@ func registerAnsible() {
570570
// CHECK: function_ref @$S11objc_thunks15registerAnsibleyyFyyycSgcfU_
571571
Ansible.anseAsync({ completion in completion!() })
572572
}
573+
@_silgen_name("noescape")
574+
func noescape(f: @convention(block) () -> ())
575+
576+
// CHECK: sil hidden @$S11objc_thunks21testObjCNoescapeThunkyyF : $@convention(thin) () -> () {
577+
// CHECK: [[REABSTRACT:%.*]] = function_ref @$SIeg_IyB_TR
578+
// CHECK: init_block_storage_header {{.*}} : $*@block_storage @callee_guaranteed () -> (), invoke [[REABSTRACT]]
579+
// CHECK: return
580+
func testObjCNoescapeThunk() {
581+
noescape {
582+
}
583+
}
584+
585+
// Noescape verification relies on there not being a retain/release in order to
586+
// work in the presence of a objective c throwing implementation function.
587+
// CHECK: sil {{.*}} @$SIeg_IyB_TR
588+
// CHECK: bb0([[T0:%.*]] : @trivial $*@block_storage @callee_guaranteed () -> ()):
589+
// CHECK-NEXT: [[T1:%.*]] = project_block_storage [[T0]]
590+
// CHECK-NEXT: [[T2:%.*]] = load_borrow [[T1]]
591+
// CHECK-NEXT: [[T3:%.*]] = apply [[T2]]()
592+
// CHECK-NEXT: [[T4:%.*]] = tuple ()
593+
// CHECK-NEXT: end_borrow [[T2]] from [[T1]]
594+
// CHECK-NEXT: return [[T4]]

0 commit comments

Comments
 (0)