[silgen] Fix lifetime management of tuple elements so we don't leak #39600
Conversation
This file contains 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
…re paths when initializing sub-tuple patterns" This reverts commit be922b9. By adding some extra scopes here we are triggering some broken behavior in a bunch of projects. I am going to see if I can do another fix for this. That being said in the short term, we are reverting to unblock those projects. rdar://83770295 (cherry picked from commit 49bc96d)
… elts and then perform sub-initialization. Previsouly we were evaluating a tuple elt and then performing the relevant sub-initialization. The problem is that a sub-initialization can invoke code that could perform an early exit cleanup. So any later tuple-elements that may need to be cleaned up along such path will not have had their cleanups initialized, resulting in a leak along such paths. With this commit, we instead evaluate all of the tuple elements and only them perform the sub-initialization ensuring that any early exits clean up all of the tuple elements. rdar://83770295 (cherry picked from commit f641808)
|
@swift-ci test |
|
@swift-ci nominate |
tbkka
approved these changes
Oct 15, 2021
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.
I also deleted the first fix that turned out to be incorrect.
rdar://83770295
Description: In SILGen, we were previously when initializing tuples in memory by preparing cleanups for that element and then emitting the sub-initialization, one by one. This behavior is incorrect since a sub-initialization /could/ cause an early exit meaning that any later tuple element would not have its cleanup created yet, causing us to leak along the early exit path. In an early commit (the one being reverted in this PR), I tried to work around this by introducing a new scope+some fancy stuff around using persistently active that didn't work out. In the non-revert commit I fix the actual issue by just setting up all of the tuple elements before I emit the sub-initializations.
Risk: Low, doesn't touch scopes. Just sets up cleanups before we run sub-initializations of tuples.
Review by: @atrick
Testing: Ran/added compiler tests.
Radar: rdar://83770295