Skip to content

[6.2][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 #80903

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.

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

Merged
merged 1 commit into from
Apr 21, 2025

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Apr 18, 2025

  • Explanation:
    In RBI, we need to be more conservative around later uses of sent boxes that are also escaped by reference before the send point. To do this, we perform an earlier forward dataflow that determines partial applies that boxes escape to and then propagate reachability forward. Then when we send the box later, we mark the box as having already escaped at the sending point by using the reachability analysis. If the reachability analysis determines that the sending instruction is in a block where reachability starts, we walk the block to determine if the send is before or after the partial apply instruction. Sadly during this walk, we included all instructions up to the send instruction and not the send instruction itself. The result of this is that if the escaping partial apply and the send instruction were the same, we would say that the partial apply was not escaped and would say in cases like the following that the more conservative behavior did not need to be applied:

    nonisolated func localCaptureDataRace4() {
      var x = 0
      _ = x
    
      Task.detached { @MainActor in x = 1 } // expected-tns-warning {{sending 'x' risks causing data races}}
      // 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}}
    
      x = 2 // expected-tns-note {{access can happen concurrently}}
    }

    The result of this less conservative behavior is that we would think that no error needed to be emitted here since we would assume that x is never passed by reference so we do not need to consider concurrent writes to x when we see the use of x = 2. Since no concurrent writes could occur to the x box and x contains a Sendable value, the box containing x is Sendable in a flow sensitive manner at that point.

  • Scope: Just tweaks a little bit of logic so that we include the partial apply "gen" instruction that we are searching for as the user that sends the value.

  • Resolves: rdar://149414471

  • Main PR: [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 #80863

  • Risk: Low. This just tweaks an instruction walk slightly.

  • Testing: Added tests that show that the concurrency hole is eliminated.

  • Reviewer: @xedin

…er, 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
(cherry picked from commit 6eee52f)
@gottesmm gottesmm requested a review from a team as a code owner April 18, 2025 01:08
@gottesmm
Copy link
Contributor Author

@swift-ci test

@gottesmm
Copy link
Contributor Author

@swift-ci test macOS platform

@gottesmm gottesmm merged commit 3b21613 into swiftlang:release/6.2 Apr 21, 2025
5 checks passed
@gottesmm gottesmm deleted the release/6.2-149414471 branch April 21, 2025 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants