Skip to content

Add nullptr checks before accessing children in getUnspecialized #66686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

augusto2112
Copy link
Contributor

The demangler already has an error mechanism to report if demangling failed. Add null pointer checks before every access in Demangle::getUnspecialized, and return an error if the child doesn't exist.

rdar://110141007
(cherry picked from commit 4e2213d)

The demangler already has an error mechanism to report if demangling
failed. Add null pointer checks before every access in
Demangle::getUnspecialized, and return an error if the child doesn't
exist.

rdar://110141007
(cherry picked from commit 4e2213d)
@augusto2112 augusto2112 requested a review from a team as a code owner June 15, 2023 23:43
@augusto2112
Copy link
Contributor Author

@swift-ci test

@tbkka
Copy link
Contributor

tbkka commented Jun 15, 2023

What was the previous PR?

@tbkka tbkka requested a review from al45tair June 15, 2023 23:56
@augusto2112
Copy link
Contributor Author

@tbkka #66644 (l was planning on adding the required description after the tests ran)

Copy link
Contributor

@al45tair al45tair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was happy with the previous PR after the change to using DEMANGLER_ASSERT.

@augusto2112
Copy link
Contributor Author

Explanation: adds sanity checks before accessing children of demangle trees in Demangle::getUnspecialized, as we've seen crashes in the debugger from demangling invalid demangle trees.
Radar: rdar://110141007
Scope: Small, this just adds a few checks on the size of the demangle tree in one particular function.
Risk: Low.
Testing: N/A
Reviewed By: @adrian-prantl @al45tair

@augusto2112 augusto2112 merged commit 8ae1133 into swiftlang:release/5.9 Jun 16, 2023
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.

3 participants