Skip to content

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Aug 25, 2025

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_stacks, let alone destroy_addrs. 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_addrs and dealloc_stacks.

rdar://158149082

@nate-chandler nate-chandler force-pushed the rdar158149082 branch 3 times, most recently from d6b4908 to 611a380 Compare August 26, 2025 03:13
@nate-chandler nate-chandler force-pushed the rdar158149082 branch 4 times, most recently from 55ea383 to 981073c Compare August 27, 2025 14:48
@nate-chandler nate-chandler marked this pull request as ready for review August 27, 2025 14:56
@nate-chandler nate-chandler requested review from eeckstein and removed request for jckarter August 27, 2025 14:56
The rewrite was missing the intentional omission of `dealloc_stack`s
corresponding to `[dead_end]` `dealloc_box`es.  Add the necessary
bridging to get to parity with the original.

Without this check, `dealloc_box [dead_end]`s are promoted to
`dealloc_stack`s but the memory projected out of such `alloc_box`s need
not be valid.

rdar://159271158
It will be used by both AllocBoxToStack in addition to StackPromotion
and for the same reason.
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
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

@nate-chandler
Copy link
Contributor Author

@swift-ci please apple silicon benchmark

@nate-chandler
Copy link
Contributor Author

@swift-ci please test macos platform

1 similar comment
@nate-chandler
Copy link
Contributor Author

@swift-ci please test macos platform

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!

@nate-chandler nate-chandler merged commit 9fe4383 into swiftlang:main Aug 28, 2025
5 of 8 checks passed
@nate-chandler nate-chandler deleted the rdar158149082 branch August 28, 2025 20:39
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