Skip to content

[Demangler] Fix assertion failure. #72654

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
merged 1 commit into from
Mar 28, 2024

Conversation

al45tair
Copy link
Contributor

It's illegal to call node->addChild() with a NULL child argument; it's possible to construct unexpected Node trees by passing invalid manglings, and in this case that was causing popTypeAndGetChild() to fail (because the top node was not a Type node), which then meant that the call to addChild had a NULL child argument.

The simplest fix is to use createWithChildren() to do the node construction, because that function checks its arguments for NULLs.

rdar://125350219

It's illegal to call `node->addChild()` with a `NULL` child argument;
it's possible to construct unexpected `Node` trees by passing invalid
manglings, and in this case that was causing `popTypeAndGetChild()` to
fail (because the top node was not a `Type` node), which then meant
that the call to `addChild` had a `NULL` child argument.

The simplest fix is to use `createWithChildren()` to do the node
construction, because that function checks its arguments for `NULL`s.

rdar://125350219
@al45tair al45tair added 🍒 release cherry pick Flag: Release branch cherry picks swift 6.0 labels Mar 28, 2024
@al45tair al45tair requested a review from a team as a code owner March 28, 2024 10:40
@al45tair
Copy link
Contributor Author

This is a cherry pick of #72653.

@al45tair
Copy link
Contributor Author

@swift-ci Please test

@al45tair
Copy link
Contributor Author

Explanation: The new demangleLifetimeDependenceKind code called addChild() with the result of popTypeAndGetChild(), but the latter can return NULL if the node tree has been constructed from a bad mangled name, and in that case we'd call addChild() with a NULL pointer. (Historically that was allowed, but I changed it to assert always, even in non-asserts builds, because landing an unexpected NULL in the node tree typically causes a crash much later on in a context where we can't easily find out what added it.)
Original PR: #72653
Reviewed by: @meg-gupta
Resolves: rdar://125350219
Tests: I've added a test that fails before this change and passes after it.

@al45tair al45tair merged commit abbeec3 into swiftlang:release/6.0 Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants