Skip to content

Commit 6eee52f

Browse files
committed
[rbi] When checking for partial apply reachability of a value at a user, include the user itself in case the user is the actual partial apply
The specific issue was when we were walking instructions looking to see if there was a partial apply escaping instruction, we were not including the user itself. That means that if the user was the partial apply escaping instruction, we would return that no escape occured. rdar://149414471
1 parent 4b5d519 commit 6eee52f

File tree

4 files changed

+38
-6
lines changed

4 files changed

+38
-6
lines changed

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1264,6 +1264,9 @@ bool PartialApplyReachabilityDataflow::isReachable(SILValue value,
12641264

12651265
// We were not reachable at entry but are at our exit... walk the block and
12661266
// see if our user is before a gen instruction.
1267+
1268+
// To do so, first find the range of pairs in valueToGenInsts that contains
1269+
// our base value.
12671270
auto genStart = std::lower_bound(
12681271
valueToGenInsts.begin(), valueToGenInsts.end(),
12691272
std::make_pair(baseValue, nullptr),
@@ -1272,20 +1275,29 @@ bool PartialApplyReachabilityDataflow::isReachable(SILValue value,
12721275
if (genStart == valueToGenInsts.end() || genStart->first != baseValue)
12731276
return false;
12741277

1278+
// Then determine the end of that range.
12751279
auto genEnd = genStart;
12761280
while (genEnd != valueToGenInsts.end() && genEnd->first == baseValue)
12771281
++genEnd;
12781282

12791283
// Walk forward from the beginning of the block to user. If we do not find a
12801284
// gen instruction, then we know the gen occurs after the op.
1285+
//
1286+
// NOTE: It is important that we include the user within our range since our
1287+
// user might also be a gen instruction.
12811288
return llvm::any_of(
1282-
user->getParent()->getRangeEndingAtInst(user), [&](SILInstruction &inst) {
1289+
llvm::make_range(user->getParent()->begin(),
1290+
std::next(user->getIterator())), [&](SILInstruction &inst) {
1291+
// Our computation works by computing a lower bound and seeing if we
1292+
// have (baseValue, inst) within our sorted range.
12831293
auto iter = std::lower_bound(
12841294
genStart, genEnd, std::make_pair(baseValue, &inst),
12851295
[](const std::pair<SILValue, SILInstruction *> &p1,
12861296
const std::pair<SILValue, SILInstruction *> &p2) {
12871297
return p1 < p2;
12881298
});
1299+
1300+
// If we did not hit end and found our pair, then we are done.
12891301
return iter != valueToGenInsts.end() && iter->first == baseValue &&
12901302
iter->second == &inst;
12911303
});

test/Concurrency/dispatch_inference.swift

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
1-
// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple %import-libdispatch -strict-concurrency=complete %s -emit-sil -o /dev/null -verify
21
// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple %import-libdispatch -strict-concurrency=complete %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
3-
// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple %import-libdispatch -strict-concurrency=complete %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
4-
// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple %import-libdispatch -strict-concurrency=complete %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-upcoming-feature RegionBasedIsolation
2+
// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple %import-libdispatch -strict-concurrency=complete %s -emit-sil -o /dev/null -verify -verify-additional-prefix tns-
53

64
// REQUIRES: concurrency
75
// REQUIRES: libdispatch
8-
// REQUIRES: swift_feature_RegionBasedIsolation
96

107
import Dispatch
118

@@ -27,8 +24,10 @@ func testUnsafeSendableInMainAsync() async {
2724
var x = 5
2825
DispatchQueue.main.async {
2926
x = 17 // expected-warning{{mutation of captured var 'x' in concurrently-executing code}}
27+
// expected-tns-warning @-1 {{sending 'x' risks causing data races}}
28+
// expected-tns-note @-2 {{'x' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
3029
}
31-
print(x)
30+
print(x) // expected-tns-note {{access can happen concurrently}}
3231
}
3332

3433
func testUnsafeSendableInAsync(queue: DispatchQueue) async {

test/Concurrency/transfernonsendable.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1997,3 +1997,13 @@ func localCaptureDataRace3() async {
19971997

19981998
print(x)
19991999
}
2000+
2001+
nonisolated func localCaptureDataRace4() {
2002+
var x = 0
2003+
_ = x
2004+
2005+
Task.detached { @MainActor in x = 1 } // expected-tns-warning {{sending 'x' risks causing data races}}
2006+
// expected-tns-note @-1 {{'x' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
2007+
2008+
x = 2 // expected-tns-note {{access can happen concurrently}}
2009+
}

test/Concurrency/transfernonsendable_global_actor.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,3 +329,14 @@ struct Clock {
329329
// expected-complete-warning @-2 {{passing argument of non-sendable type '() -> ()' into global actor 'CustomActor'-isolated context may introduce data races}}
330330
// expected-complete-note @-3 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
331331
}
332+
333+
@MainActor
334+
func localCaptureDataRace5() {
335+
var x = 0
336+
_ = x
337+
338+
Task.detached { @CustomActor in x = 1 } // expected-tns-warning {{sending 'x' risks causing data races}}
339+
// expected-tns-note @-1 {{'x' is captured by a global actor 'CustomActor'-isolated closure. global actor 'CustomActor'-isolated uses in closure may race against later main actor-isolated uses}}
340+
341+
x = 2 // expected-tns-note {{access can happen concurrently}}
342+
}

0 commit comments

Comments
 (0)