Skip to content

Commit b603c3c

Browse files
committed
Emit a null check before asserting isolation in a function with optional isolation.
Fixes rdar://132478429
1 parent db157c9 commit b603c3c

File tree

2 files changed

+86
-0
lines changed

2 files changed

+86
-0
lines changed

lib/SILGen/SILGenConcurrency.cpp

+31
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,23 @@ void SILGenFunction::emitPreconditionCheckExpectedExecutor(
736736
// We don't want the debugger to step into these.
737737
loc.markAutoGenerated();
738738

739+
// If the function is isolated to an optional actor reference,
740+
// check dynamically whether it's non-null. We don't need to
741+
// do an assertion if the expected expected is nil.
742+
SILBasicBlock *noneBB = nullptr;
743+
bool isOptional = (bool) executorOrActor->getType().getOptionalObjectType();
744+
if (isOptional) {
745+
// Start by emiting the .some path.
746+
noneBB = createBasicBlock();
747+
auto someBB = createBasicBlockBefore(noneBB);
748+
749+
executorOrActor =
750+
B.createSwitchOptional(loc, executorOrActor, someBB, noneBB,
751+
executorOrActor->getOwnershipKind());
752+
753+
B.emitBlock(someBB);
754+
}
755+
739756
// Get the executor.
740757
SILValue executor = B.createExtractExecutor(loc, executorOrActor);
741758

@@ -747,6 +764,20 @@ void SILGenFunction::emitPreconditionCheckExpectedExecutor(
747764
{args.filenameStartPointer, args.filenameLength, args.filenameIsAscii,
748765
args.line, ManagedValue::forObjectRValueWithoutOwnership(executor)},
749766
SGFContext());
767+
768+
// Complete the optional control flow if we had an optional value.
769+
if (isOptional) {
770+
assert(noneBB);
771+
// Finish the .some path by branching to the continuation block.
772+
auto contBB = createBasicBlockAfter(noneBB);
773+
B.createBranch(loc, contBB);
774+
775+
// The .none path is trivial.
776+
B.emitBlock(noneBB);
777+
B.createBranch(loc, contBB);
778+
779+
B.emitBlock(contBB);
780+
}
750781
}
751782

752783
bool SILGenFunction::unsafelyInheritsExecutor() {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// RUN: %empty-directory(%t/src)
2+
// RUN: split-file %s %t/src
3+
4+
/// Build the library A
5+
// RUN: %target-swift-frontend -emit-module %t/src/Preconcurrency.swift \
6+
// RUN: -module-name Preconcurrency -swift-version 5 -enable-library-evolution \
7+
// RUN: -emit-module-path %t/Preconcurrency.swiftmodule
8+
9+
// RUN: %target-swift-emit-silgen -swift-version 6 -disable-availability-checking -I %t %t/src/test.swift -o - -verify | %FileCheck %s
10+
11+
// REQUIRES: concurrency
12+
13+
//--- Preconcurrency.swift
14+
15+
public func takeNonSendableClosure_preconcurrency(_ fn: @escaping () -> Int) {}
16+
public func takeNonSendableClosure_preconcurrency_generic<T>(_ fn: @escaping () -> T) {}
17+
18+
//--- test.swift
19+
20+
import Preconcurrency
21+
22+
func takeNonSendableClosure_strict(_ fn: @escaping () -> Int) { }
23+
@preconcurrency
24+
25+
actor MyActor {
26+
var counter = 0
27+
}
28+
func forceIsolation(isolation: isolated MyActor?) {}
29+
30+
// rdar://132478429
31+
//
32+
// We were trying to emit dynamic isolation checks in functions that are
33+
// isolated to optional actor references by just passing that reference
34+
// directly to extract_executor, which is incorrect --- we need to skip
35+
// the check when the reference is nil.
36+
37+
// CHECK-LABEL: sil private [ossa] @$s4test0A25OptionalIsolation_checked9isolationyAA7MyActorCSgYi_tFSiycfU_ :
38+
// CHECK: [[T0:%.*]] = copy_value %0 : $Optional<MyActor>
39+
// CHECK-NEXT: [[BORROW:%.*]] = begin_borrow [[T0]] :
40+
// CHECK-NEXT: switch_enum [[BORROW]] : $Optional<MyActor>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
41+
// CHECK: bb1([[T0:%.*]] : @guaranteed $MyActor):
42+
// CHECK-NEXT: extract_executor [[T0]] : $MyActor
43+
// CHECK: // function_ref _checkExpectedExecutor
44+
// CHECK: br bb3
45+
// CHECK: bb2:
46+
// CHECK-NEXT: br bb3
47+
func testOptionalIsolation_checked(isolation: isolated MyActor?) {
48+
takeNonSendableClosure_preconcurrency {
49+
// This closure inherits isolation because it's non-Sendable, and
50+
// it requires a dynamic check because we're passing it to a
51+
// preconcurrency function.
52+
forceIsolation(isolation: isolation)
53+
return 0
54+
}
55+
}

0 commit comments

Comments
 (0)