Skip to content

Conversation

@rtldg
Copy link
Contributor

@rtldg rtldg commented Dec 31, 2025

SymbolString returns a const char * so our auto name is a const char *. If the name was successfully demangled, then we assign name to demangled, and then immediately after free(demangled).

Switching back to using std::string resolves this since that duplicates the strings.

Popped up after #35 and I only encountered it because of a non-default memory-allocator.

@Kenzzer
Copy link
Collaborator

Kenzzer commented Jan 1, 2026

Took me a lot of back and forth to grasp the situation given the PR changes and one line wording, so I'm providing some clarifications below.

I disagree with one of the statements from the post

then we assign name to demangled, and then immediately after free(demangled)

This isn't what's happening, name isn't assigned to demangled.
demangled gets assigned whatever result stems from abi::__cxa_demangle(name.c_str(), nullptr, nullptr, &status)

But in any case this isn't what we should focus on, after mulling it over for a few minutes I realised the PR meant line 30 of the patch (which doesn't appear in the changes)

name = demangled;

name gets assigned demangled, and like said in OP demangled gets freed immediately in the line below so name has a dangling pointer.

This got me confused for a while because abi::__cxa_demangle, that get assigned to demangled, allocates memory for you. So I was focusing on the wrong line this whole time lol. I'm putting this here as a clarification note for history diggers.


Putting all of this aside, this is awesome work as always @rtldg and you're in all honesty a saviour. This PR demonstrates that extra scrutiny should be put on the breakpad patches, especially the older ones, everytime we move forward the submodule version.

Copy link
Collaborator

@Kenzzer Kenzzer left a comment

Choose a reason for hiding this comment

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

Happy new year, and first commit of 2026 for accelerator. Congratulations, and thanks again!

@Kenzzer Kenzzer merged commit dcf3449 into asherkin:master Jan 1, 2026
5 checks passed
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.

2 participants