[5.9] SILCombine: correctly set the [stack] flag when replacing alloc_ref_dynamic with alloc_ref
#66816
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
…f_dynamic` with `alloc_ref` Fixes a verifier crash. swiftlang#66312
|
@swift-ci test |
DougGregor
approved these changes
Jun 21, 2023
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: This is a fix for a SIL verifier crash. When optimizing
alloc_ref_dynamicinstructions in SILCombine, the[stack]attribute was not set correctly. Beside a verifier crash, this also results in a memory leak in the compiled code (if the verifier is disabled, like in no-assert compiler builds). Interestingly, this bug is in the compiler since 7 years. It's surprising that it didn't show up earlier.Scope: Although the
alloc_ref_dynamicinstruction is used in the stdlib's array and dictionary code, it is obviously very rare that this optimizer bug really shows up.Issue: SIL verification failure when evaluating
$0in certain closures. #66312Risk: Low. The change is small. Some variants of the peephole optimization are disabled now in case of an
alloc_ref_dynamic [stack]. In the remaining cases, the[stack]attribute is set correctly. These are the cases where the verifier would have crashed before.Testing: With a regression test
Reviewer: @nate-chandler
Main branch PR: SILCombine: correctly set the
[stack]flag when replacingalloc_ref_dynamicwithalloc_ref#66800