Skip to content

Commit 9892543

Browse files
committed
[region-isolation] Disconnected regions in global actor isolated function are not accessible again after call to nonisolated async function
I also fixed the nonisolated(unsafe) issue. rdar://126667934 rdar://126174959 (cherry picked from commit 0e21c58)
1 parent 64320b9 commit 9892543

File tree

6 files changed

+99
-42
lines changed

6 files changed

+99
-42
lines changed

include/swift/SILOptimizer/Utils/PartitionUtils.h

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,13 +1192,10 @@ struct PartitionOpEvaluator {
11921192
//
11931193
// DISCUSSION: We couldn't not emit this earlier since we needed the
11941194
// dynamic isolation info of our value.
1195-
if (transferredRegionIsolation.isActorIsolated()) {
1196-
if (auto calleeIsolationInfo =
1197-
SILIsolationInfo::get(op.getSourceInst())) {
1198-
if (transferredRegionIsolation.hasSameIsolation(
1199-
calleeIsolationInfo)) {
1200-
return;
1201-
}
1195+
if (auto calleeIsolationInfo =
1196+
SILIsolationInfo::get(op.getSourceInst())) {
1197+
if (transferredRegionIsolation.hasSameIsolation(calleeIsolationInfo)) {
1198+
return;
12021199
}
12031200
}
12041201

lib/SILOptimizer/Utils/PartitionUtils.cpp

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include "swift/SILOptimizer/Utils/PartitionUtils.h"
1414

15+
#include "swift/AST/ASTWalker.h"
1516
#include "swift/AST/Expr.h"
1617
#include "swift/SIL/ApplySite.h"
1718
#include "swift/SIL/InstructionUtils.h"
@@ -70,6 +71,37 @@ getGlobalActorInitIsolation(SILFunction *fn) {
7071
return getActorIsolation(globalDecl);
7172
}
7273

74+
static DeclRefExpr *getDeclRefExprFromExpr(Expr *expr) {
75+
struct LocalWalker final : ASTWalker {
76+
DeclRefExpr *result = nullptr;
77+
78+
PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {
79+
assert(!result && "Shouldn't have a result yet");
80+
81+
if (auto *dre = dyn_cast<DeclRefExpr>(expr)) {
82+
result = dre;
83+
return Action::Stop();
84+
}
85+
86+
if (isa<CoerceExpr, MemberRefExpr, ImplicitConversionExpr, IdentityExpr>(
87+
expr))
88+
return Action::Continue(expr);
89+
90+
return Action::Stop();
91+
}
92+
};
93+
94+
LocalWalker walker;
95+
96+
if (auto *ae = dyn_cast<AssignExpr>(expr)) {
97+
ae->getSrc()->walk(walker);
98+
} else {
99+
expr->walk(walker);
100+
}
101+
102+
return walker.result;
103+
}
104+
73105
SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
74106
if (auto fas = FullApplySite::isa(inst)) {
75107
if (auto crossing = fas.getIsolationCrossing()) {
@@ -176,15 +208,30 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
176208
// actor isolated method. Use the AST to compute the actor isolation and
177209
// check if we are self. If we are not self, we want this to be
178210
// disconnected.
179-
if (auto *declRefExpr = cmi->getLoc().getAsASTNode<DeclRefExpr>()) {
180-
if (auto isolation = swift::getActorIsolation(declRefExpr->getDecl())) {
181-
if (isolation.isActorIsolated() &&
182-
(isolation.getKind() != ActorIsolation::ActorInstance ||
183-
isolation.getActorInstanceParameter() == 0)) {
184-
auto actor = cmi->getOperand()->getType().isAnyActor()
185-
? cmi->getOperand()
186-
: SILValue();
187-
return SILIsolationInfo::getActorIsolated(cmi, actor, isolation);
211+
if (auto *expr = cmi->getLoc().getAsASTNode<Expr>()) {
212+
if (auto *dre = getDeclRefExprFromExpr(expr)) {
213+
if (auto isolation = swift::getActorIsolation(dre->getDecl())) {
214+
if (isolation.isActorIsolated() &&
215+
(isolation.getKind() != ActorIsolation::ActorInstance ||
216+
isolation.getActorInstanceParameter() == 0)) {
217+
auto actor = cmi->getOperand()->getType().isAnyActor()
218+
? cmi->getOperand()
219+
: SILValue();
220+
return SILIsolationInfo::getActorIsolated(cmi, actor, isolation);
221+
}
222+
}
223+
224+
if (auto type = dre->getType()->getNominalOrBoundGenericNominal()) {
225+
if (auto isolation = swift::getActorIsolation(type)) {
226+
if (isolation.isActorIsolated() &&
227+
(isolation.getKind() != ActorIsolation::ActorInstance ||
228+
isolation.getActorInstanceParameter() == 0)) {
229+
auto actor = cmi->getOperand()->getType().isAnyActor()
230+
? cmi->getOperand()
231+
: SILValue();
232+
return SILIsolationInfo::getActorIsolated(cmi, actor, isolation);
233+
}
234+
}
188235
}
189236
}
190237
}
@@ -266,9 +313,15 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
266313
// of the actor.
267314
if (ApplyExpr *apply = inst->getLoc().getAsASTNode<ApplyExpr>()) {
268315
if (auto crossing = apply->getIsolationCrossing()) {
269-
if (crossing->getCalleeIsolation().isActorIsolated())
316+
auto calleeIsolation = crossing->getCalleeIsolation();
317+
if (calleeIsolation.isActorIsolated()) {
270318
return SILIsolationInfo::getActorIsolated(
271319
SILValue(), SILValue(), crossing->getCalleeIsolation());
320+
}
321+
322+
if (calleeIsolation.isNonisolated()) {
323+
return SILIsolationInfo::getDisconnected();
324+
}
272325
}
273326
}
274327

test/Concurrency/experimental_feature_strictconcurrency.swift

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,7 @@ struct Test2: TestProtocol { // expected-warning{{conformance of 'C2' to 'Sendab
2828
@MainActor
2929
func iterate(stream: AsyncStream<Int>) async {
3030
nonisolated(unsafe) var it = stream.makeAsyncIterator()
31-
// FIXME: Region isolation should consider a value from a 'nonisolated(unsafe)'
32-
// declaration to be in a disconnected region
3331

34-
// expected-region-isolation-warning @+3 {{transferring 'it' may cause a data race}}
35-
// expected-region-isolation-note @+2 {{transferring disconnected 'it' to nonisolated callee could cause races in between callee nonisolated and local main actor-isolated uses}}
36-
// expected-region-isolation-note @+1 {{use here could race}}
3732
while let element = await it.next() {
3833
print(element)
3934
}

test/Concurrency/transfernonsendable.swift

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1552,9 +1552,8 @@ func functionArgumentIntoClosure(_ x: @escaping () -> ()) async {
15521552
var c = NonSendableKlass()
15531553
for _ in 0..<1024 {
15541554
await useValueAsync(c) // expected-tns-warning {{transferring 'c' may cause a data race}}
1555-
// expected-tns-note @-1 {{transferring disconnected 'c' to nonisolated callee could cause races in between callee nonisolated and local main actor-isolated uses}}
1556-
// expected-tns-note @-2 {{use here could race}}
1557-
// expected-complete-warning @-3 {{passing argument of non-sendable type 'NonSendableKlass' outside of main actor-isolated context may introduce data races}}
1555+
// expected-tns-note @-1 {{transferring main actor-isolated 'c' to nonisolated callee could cause races between nonisolated and main actor-isolated uses}}
1556+
// expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableKlass' outside of main actor-isolated context may introduce data races}}
15581557
c = a.klass
15591558
}
15601559
}
@@ -1563,11 +1562,9 @@ func functionArgumentIntoClosure(_ x: @escaping () -> ()) async {
15631562
let a = MainActorIsolatedKlass()
15641563
var c = NonSendableKlass()
15651564
for _ in 0..<1024 {
1566-
await useValueAsync(c) // expected-tns-warning 2{{transferring 'c' may cause a data race}}
1567-
// expected-tns-note @-1 {{transferring disconnected 'c' to nonisolated callee could cause races in between callee nonisolated and local main actor-isolated uses}}
1568-
// expected-tns-note @-2 {{use here could race}}
1569-
// expected-tns-note @-3 {{transferring main actor-isolated 'c' to nonisolated callee could cause races between nonisolated and main actor-isolated uses}}
1570-
// expected-complete-warning @-4 {{passing argument of non-sendable type 'NonSendableKlass' outside of main actor-isolated context may introduce data races}}
1565+
await useValueAsync(c) // expected-tns-warning {{transferring 'c' may cause a data race}}
1566+
// expected-tns-note @-1 {{transferring main actor-isolated 'c' to nonisolated callee could cause races between nonisolated and main actor-isolated uses}}
1567+
// expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableKlass' outside of main actor-isolated context may introduce data races}}
15711568
c = a.klassLet
15721569
}
15731570
}

test/Concurrency/transfernonsendable_global_actor.swift

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,12 @@ private class NonSendableLinkedListNode<T> { // expected-complete-note 3{{}}
9797
@CustomActor func useCustomActor5() async {
9898
let x = NonSendableLinkedListNode<Int>()
9999

100-
await transferToNonIsolated(x) // expected-tns-warning {{transferring 'x' may cause a data race}}
101-
// expected-tns-note @-1 {{transferring disconnected 'x' to nonisolated callee could cause races in between callee nonisolated and local global actor 'CustomActor'-isolated uses}}
102-
// expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableLinkedListNode<Int>' outside of global actor 'CustomActor'-isolated context may introduce data races}}
100+
// This is ok since the nonisolated function cannot transfer x, so once we
101+
// return x will be isolated again.
102+
await transferToNonIsolated(x)
103+
// expected-complete-warning @-1 {{passing argument of non-sendable type 'NonSendableLinkedListNode<Int>' outside of global actor 'CustomActor'-isolated context may introduce data races}}
103104

104-
useValue(x) // expected-tns-note {{use here could race}}
105+
useValue(x)
105106
}
106107

107108
private struct StructContainingValue { // expected-complete-note 2{{}}
@@ -113,11 +114,12 @@ private struct StructContainingValue { // expected-complete-note 2{{}}
113114
var x = StructContainingValue()
114115
x = StructContainingValue()
115116

116-
await transferToNonIsolated(x) // expected-tns-warning {{transferring 'x' may cause a data race}}
117-
// expected-tns-note @-1 {{transferring disconnected 'x' to nonisolated callee could cause races in between callee nonisolated and local global actor 'CustomActor'-isolated uses}}
118-
// expected-complete-warning @-2 {{passing argument of non-sendable type 'StructContainingValue' outside of global actor 'CustomActor'-isolated context may introduce data races}}
117+
// This is ok since the nonisolated function cannot transfer x meaning after
118+
// we return we know that x will be disconnected upon return as well.
119+
await transferToNonIsolated(x)
120+
// expected-complete-warning @-1 {{passing argument of non-sendable type 'StructContainingValue' outside of global actor 'CustomActor'-isolated context may introduce data races}}
119121

120-
useValue(x) // expected-tns-note {{use here could race}}
122+
useValue(x)
121123
}
122124

123125
@CustomActor func useCustomActor7() async {
@@ -135,12 +137,12 @@ private struct StructContainingValue { // expected-complete-note 2{{}}
135137
var x = (NonSendableLinkedList<Int>(), NonSendableLinkedList<Int>())
136138
x = (NonSendableLinkedList<Int>(), NonSendableLinkedList<Int>())
137139

138-
await transferToNonIsolated(x) // expected-tns-warning {{transferring 'x' may cause a data race}}
139-
// expected-tns-note @-1 {{transferring disconnected 'x' to nonisolated callee could cause races in between callee nonisolated and local global actor 'CustomActor'-isolated uses}}
140+
// This is safe since the nonisolated function cannot transfer x further.
141+
await transferToNonIsolated(x)
142+
// expected-complete-warning @-1 {{passing argument of non-sendable type '(NonSendableLinkedList<Int>, NonSendableLinkedList<Int>)' outside of global actor 'CustomActor'-isolated context may introduce data races}}
140143
// expected-complete-warning @-2 {{passing argument of non-sendable type '(NonSendableLinkedList<Int>, NonSendableLinkedList<Int>)' outside of global actor 'CustomActor'-isolated context may introduce data races}}
141-
// expected-complete-warning @-3 {{passing argument of non-sendable type '(NonSendableLinkedList<Int>, NonSendableLinkedList<Int>)' outside of global actor 'CustomActor'-isolated context may introduce data races}}
142144

143-
useValue(x) // expected-tns-note {{use here could race}}
145+
useValue(x)
144146
}
145147

146148
@CustomActor func useCustomActor9() async {

test/Concurrency/transfernonsendable_nonisolated.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,16 @@ func callMyFinalActor_NonIsolatedSync_SyncUseAfter(_ a: MyFinalActor) async {
117117
a.syncNonisolated(x)
118118
useValueSync(x)
119119
}
120+
121+
func callIsolatedFunction() async {
122+
let x = NonSendable()
123+
await useValueAsync(x)
124+
useValueSync(x)
125+
}
126+
127+
@MainActor
128+
func callMainActorIsolatedFunction() async {
129+
let x = NonSendable()
130+
await useValueAsync(x)
131+
useValueSync(x)
132+
}

0 commit comments

Comments
 (0)