6.2: [AllocBoxToStack] Don't destroy in dead-ends. #83962
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
It is valid to leak a value on paths into dead-end regions. Specifically, it is valid to leak an `alloc_box`. Thus, "final releases" in dead-end regions may not destroy the box and consequently may not release its contents. Therefore it's invalid to lower such final releases to `dealloc_stack`s, let alone `destroy_addr`s. The in-general invalidity of that transformation results in miscompiling whenever a box is leaked and its projected address is used after such final releases. Fix this by not treating final releases as boundary markers of the `alloc_box` and not lowering them to `destroy_addr`s and `dealloc_stack`s. rdar://158149082
a1b9d45 to
5798e71
Compare
|
@swift-ci please test |
tbkka
approved these changes
Aug 28, 2025
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: Fix an ancient use-after-free in AllocBoxToStack.
Out of SILGen,
vars are initially implemented with heap-allocated boxes containing the stored value. TheAllocBoxToStackpass attempts to promote such heap allocations to stack allocations. When doing this, it lowers destroys (strong_release,dealloc_box,destroy_value) of the box to destroys (destroy_addr) of the on-stack object and deallocation of the stack slot (dealloc_stack).This lowering is not totally straightforward because, among other things, the boxes are reference counted but the stack slots aren't. There's no decrementing the reference count of a stack slot, just deallocating it. Also, when the box is destroyed, the object stored within it is released; that is not true when the stack slot is deallocated.
To account for this difference, the pass inserts instructions to 1. destroy the object stored in the stack slot and 2. deallocate the slot. These pairs of instructions are inserted at the "final releases" of the box. In functions where all paths exit the function, this is correct because it is an invariant of SIL that retains and releases are balanced along such paths. Paths which do not exit the function (paths into "dead-end" regions), however, do not satisfy this invariant: it is permitted that there be more retains than releases along such paths; objects are permitted to leak. In particular, the box could be leaked. Consequently, the "final release" of the box along such a path need not destroy the box; nor release the object it stores.
Previously, this fact was not considered and the "final release" along such "dead-end" paths were treated like "final releases" along function-exiting paths. When such "final releases" did not in fact destroy the box in the incoming SIL, the effect of the pass was to shorten the lifetime of the stored value invalidly. If there were uses of the stored value after such "final releases", those could become uses after frees.
Here, this is fixed by considering whether the "final release" is in a "dead end" and skipping the destruction of the object and deallocation of the storage if so.
Scope: Affects code involving vars.
Issue: rdar://158149082
Original PR: #83907
Risk: Low. Results in inserting fewer lifetime ending instructions in non-loop dead end blocks.
Testing: Added several test cases for the pass.
Reviewer: Erik Eckstein ( @eeckstein )