Fix crash caused by repeatedly cloning the same path hierarchy node #1220
+200
−52
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
rdar://150706871
|
@swift-ci please test |
|
@swift-ci please test |
binamaniar
reviewed
May 16, 2025
|
@swift-ci please test |
|
@d-ronnqvist Thank you for taking time to answer pr comments. |
binamaniar
approved these changes
May 19, 2025
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
Bug/issue #, if applicable: rdar://150706871&148247074
Summary
This primarily fixes a crash caused by repeatedly cloning the same path hierarchy node. (e161b2a)
Then if fixes additional crashes that were made more common due to cloned nodes (4187f62 and c12c7d1)
Seeing these changes in aggregate I would like to take some time to reorganize the path hierarchy initializer to better distinguish the expected code paths from the code paths that try to gracefully handle incomplete symbol graph files. However, I would want to cherry-pick these crash fixes into the 6.2 release and I'm not yet sure about the scope of the other code cleanup so I'd prefer to open a separate PR for that.
Dependencies
None.
Testing
Because most of these issues are related to unexpectedly incomplete symbol graph files it's fairly difficult to test this all the way from symbol graph generation. I could manually craft a symbol graph file that is based on one of the added unit tests, but I don't find that that would provide any more assurances than running the tests which the CI already does.
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded