Skip to content

[4.0] When creating destroys for addresses passed into a partial apply, fir… #11240

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

Conversation

gottesmm
Copy link
Contributor

…st move the values into a stack location with a live range that is guaranteed to be larger than the partial apply's live range.

Otherwise, we may insert destroy_addrs on alloc_stack whose lifetimes have
ended.

rdar://33502257
(cherry picked from commit 81914b9)

…st move the values into a stack location with a live range that is guaranteed to be larger than the partial apply's live range.

Otherwise, we may insert destroy_addrs on alloc_stack whose lifetimes have
ended.

rdar://33502257
(cherry picked from commit 81914b9)
@gottesmm gottesmm requested a review from eeckstein July 28, 2017 04:54
@gottesmm
Copy link
Contributor Author

@swift-ci test

@gottesmm
Copy link
Contributor Author

@eeckstein I decided to go with the route that we originally discussed: put the alloc_stack at the beginning/end of the function, but just not process partial_applies that have arguments with dependent types.

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

LGTM!

@gottesmm
Copy link
Contributor Author

Explanation: We were not properly extending the live range of an alloc_stack when we were eliminating dead partial_applies. This fix ensures that we do do so. Without this patch we run into use-after frees as a result of the bad live range.
Scope: This is an important correctness issues and has been seen in the wild.
Radar (and possibly SR Issue): rdar://33502257
Risk: Minimal.
Testing: I added tests to the compiler that tests the relevant functionality.

@gottesmm gottesmm added this to the Swift 4.0 milestone Jul 28, 2017
@gottesmm
Copy link
Contributor Author

Approved by Ted via email.

@gottesmm gottesmm merged commit b7da742 into swiftlang:swift-4.0-branch Jul 28, 2017
@gottesmm gottesmm deleted the swift-4.0-branchrdar33502257 branch July 28, 2017 19:02
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