Skip to content

Conversation

atrick
Copy link
Contributor

@atrick atrick commented Apr 3, 2023

Fix a miscompile in Debug builds at -Onone.

This optimization ignores uses of owned values that aren't enclosed in borrow scopes. This is fairly eggregious since project_box instructions are never borrowed, which means that all local variables have this problem.

This is a well-known issue that occurs throughout OSSA optimizations. The reason that we don't see the problem often is that the optimizations are hidden behind a pile of ad-hoc pattern matching, so they only kick in for simple cases. This approach to optimization is great at hiding problems for a long time.

We're attempting to design away this class of problems in the next release. Until then, it's one miscompile at a time.

Fixes rdar://107420448 (Variable mutation in block isn't reflected in outer scope: new behavior in swift 5.9)

(cherry picked from commit 26a6264)

@atrick atrick requested review from meg-gupta and tbkka April 3, 2023 17:10
@atrick atrick requested a review from a team as a code owner April 3, 2023 17:10
@atrick
Copy link
Contributor Author

atrick commented Apr 3, 2023

@swift-ci test

@atrick
Copy link
Contributor Author

atrick commented Apr 3, 2023

The unit test needs to be fixed before I can merge this.

Fix a miscompile in Debug builds at -Onone.

This optimization ignores uses of owned values that aren't enclosed in
borrow scopes. This is fairly eggregious since project_box
instructions are never borrowed, which means that all local variables
have this problem.

This is a well-known issue that occurs throughout OSSA
optimizations. The reason that we don't see the problem often is that
the optimizations are hidden behind a pile of ad-hoc pattern matching,
so they only kick in for simple cases. This approach to optimization
is great at hiding problems for a long time.

We're attempting to design away this class of problems in the next
release. Until then, it's one miscompile at a time.

Fixes rdar://107420448 (Variable mutation in block isn't reflected
in outer scope: new behavior in swift 5.9)

(cherry picked from commit 26a6264)
@atrick atrick force-pushed the 5.9-fix-mandatory-arc branch from 5cedf61 to 7d10ee3 Compare April 4, 2023 17:10
@atrick
Copy link
Contributor Author

atrick commented Apr 4, 2023

@swift-ci test

@atrick
Copy link
Contributor Author

atrick commented Apr 4, 2023

@tbkka can you approve this for 5.9?

@atrick atrick merged commit 5a59df7 into swiftlang:release/5.9 Apr 4, 2023
@atrick atrick deleted the 5.9-fix-mandatory-arc branch April 4, 2023 23:31
@AnthonyLatsis AnthonyLatsis added the 🍒 release cherry pick Flag: Release branch cherry picks label May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants