[5.9] Fix two memory corruption bugs in SwiftSyntax #1719
Merged
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.
This is a slightly less risky version of #1708
SyntaxData.forRoot
takes aRawSyntax
, which holds a reference to itsSyntaxArena
butRawSyntax
doesn’t retain its arena. We were already taking care of this in multiple places by adding awithExtendedLifetime
for the arena around it but missed a case inSyntaxData.replacingSelf
. AddwithExtendedLifetime
here as well.replacingChild
function took aRawSyntax
, which again doesn’t retain its arena. When changing any syntax node’s child, we have avalue
, which is aSyntax
node but all we did in the setter was to passvalue.raw
intoreplacingChild
. This left an opportunity for the optimizer to deallocate theSyntax
node (and thus it’s arena) before the call toreplacingChild
because all we needed from hat node was theRawSyntax
. To fix this, add another version ofreplacingChild
that takes aSyntaxData
(which does retain the arena) and use that instead.SyntaxArena.rewrite
passed a newSyntaxArena
toreplacingSelf
, butreplacingSelf
expected to be passed the arena in whichrewritten
was allocated.withExtendedLifetime
callsswift-format
doesn’t regress in performance