Skip to content

[rbi] When translating a Store, make sure to require src even if the value being stored is Sendable #85164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[rbi] When translating a Store, make sure to require src even if the value being stored is Sendable #85164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions lib/SILOptimizer/Analysis/RegionAnalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2993,7 +2993,7 @@ class PartitionOpTranslator {
/// dest to src. If the \p dest could be aliased, then we must instead treat
/// them as merges, to ensure any aliases of \p dest are also updated.
void translateSILStore(Operand *dest, Operand *src,
SILIsolationInfo resultIsolationInfoOverride = {}) {
SILIsolationInfo resultIsolationInfoOverride = {}) {
SILValue destValue = dest->get();

if (auto destResult = tryToTrackValue(destValue)) {
Expand Down Expand Up @@ -3021,7 +3021,14 @@ class PartitionOpTranslator {
resultIsolationInfoOverride);
}

// Stores to storage of non-Sendable type can be ignored.
// If we reached this point, our destination is something that is Sendable
// and is not an address that comes from a non-Sendable base... so we can
// ignore the store part. But we still need tosee if our src (which also
// must be Sendable) comes from a non-Sendable base. In such a case, we need
// to require that.
if (auto srcResult = tryToTrackValue(src->get())) {
builder.addRequire(*srcResult);
}
}

void translateSILTupleAddrConstructor(TupleAddrConstructorInst *inst) {
Expand Down
33 changes: 33 additions & 0 deletions test/Concurrency/transfernonsendable.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2001,3 +2001,36 @@ nonisolated(nonsending) func testCallNonisolatedNonsending(_ x: NonSendableKlass
// expected-ni-note @-1 {{sending nonisolated(nonsending) task-isolated 'x' to nonisolated global function 'useValueAsyncConcurrent' risks causing data races between nonisolated and nonisolated(nonsending) task-isolated uses}}
// expected-ni-ns-note @-2 {{sending task-isolated 'x' to @concurrent global function 'useValueAsyncConcurrent' risks causing data races between @concurrent and task-isolated uses}}
}

enum RequireSrcWhenStoringEvenWhenSendable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any chance the solution here can be extended to address this case as well:

// https://github.com/swiftlang/swift/issues/82827
@MainActor
func test_82827() {
    var s = 1
    Task {
        for _ in 0..<100 {
            s += 1
        }
    }
    Task.detached { _ = s } // not diagnosed
}

or is that a separate problem?

func test<T: Sendable>(t: T) {
var result: T = t
Task { // expected-warning {{sending value of non-Sendable type '() async -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to initializer 'init(name:priority:operation:)' risks causing races in between local and caller code}}
result = t
}
useValue(result) // expected-note {{access can happen concurrently}}
}

func test2() {
var result: Any = 0
Task { // expected-warning {{sending value of non-Sendable type '() async -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to initializer 'init(name:priority:operation:)' risks causing races in between local and caller code}}
result = 0
}
useValue(result) // expected-note {{access can happen concurrently}}
}

protocol Initializable {
init()
}

func test3<T: Initializable & SendableMetatype>(type: T.Type) {
var result = type.init()
Task { // expected-warning {{sending value of non-Sendable type '() async -> ()' risks causing data races}}
// expected-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to initializer 'init(name:priority:operation:)' risks causing races in between local and caller code}}
result = type.init()
}
useValue(result) // expected-note {{access can happen concurrently}}
}
}