[5.6] Fix misoptimization in OptimizeHopToExecutor #41176
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please test |
|
Build failed |
|
Build failed |
The problem is in `OptimizeHopToExecutor::collectActors`, which is causing `removeRedundantHopToExecutors` to drop the `hop_to_executor`. The issue is triggered by the order in which the Actors map is populated, and the reuse of a SILValue representing an executor. Suppose we have a function like this: ``` bb0: %7 = builtin "getCurrentExecutor"() : $Optional<Builtin.Executor> // ... cond_br %12, bb2, bb1 bb1: // ... hop_to_executor %7 : $Optional<Builtin.Executor> bb2: // ... hop_to_executor %7 : $Optional<Builtin.Executor> // ... hop_to_executor %28 : $MainActor ``` Since `collectActors` goes through each function's instructions top-down, and assigns to each unique SILValue an ID equal to the size of the Actors map prior to inserting the SILValue in the map, and we end up with the wrong actor IDs. This commit changes the ID numbering scheme to correct the issue.
kavon
force-pushed
the
5.6-fix-sr15789
branch
from
53f52bf to
12fcb35
Compare
|
@swift-ci please test |
|
@swift-ci please nominate |
1 similar comment
|
@swift-ci please nominate |
DougGregor
approved these changes
Feb 4, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation: A misoptimization of Swift concurrency features (
hop_to_executor) can lead to incorrect runtime behavior, where actor isolation is not actually gained, which can lead to data races, etc.Scope: Difficult to estimate, but large enough to have received one bug report from a user. The conditions in which the misoptimization will trigger are not easy to describe in terms of the Swift language. That's because very inoccuous source changes can perturb the code generation enough to hide or show the misoptimization. Since the errant optimization pass is always run, all programs using Swift Concurrency are at risk. A change that would make this misoptimization more likely to happen landed for Swift 5.6 recently, so it's only likely to get worse.
Radar/SR Issue: rdar://88285600
Risk: Extremely low.
Testing: PR testing and CI .
Reviewed By: Doug Gregor and Erik Eckstein
Original PR: #41175