Add MSVC demangler correctness fixes and generated test corpus#8140
Open
plafosse wants to merge 2 commits into
Open
Add MSVC demangler correctness fixes and generated test corpus#8140plafosse wants to merge 2 commits into
plafosse wants to merge 2 commits into
Conversation
plafosse
force-pushed
the
test_ms_demangler_fixes
branch
3 times, most recently
from
4b906ea to
15dd98a
Compare
Member
Author
plafosse
force-pushed
the
test_ms_demangler_fixes
branch
2 times, most recently
from
1cd85f5 to
02cebd9
Compare
Split MSVC symbol parsing from finalization by carrying the parsed qualified name in DemangleContext instead of using mutable demangler-wide name state. Function parsing now returns a DemangledFunction with the parsed DemangledTypeNode plus optional decoded thunk adjustor metadata, and the symbol-context callers finalize name-dependent work such as thunk suffixes and implicit this parameter synthesis. Expand DemangledTypeNode into the shared representation used by the MSVC path for delayed type construction, platform-aware widths, calling convention resolution, member pointers, postfix forms, and explicit implicit-this parameters. This lets MSVC parsing produce structured type/name data first and defer Binary Ninja Type construction until finalization has the platform and view context. Also route MSVC string literals, raw type-info names, vtables, RTTI, dynamic init/fini stubs, local guards, and referenced-symbol template values through explicit DemangledTypeNode/name flow. Extensively tested with the MSVC demangler unit suite and a 199,500-symbol regression corpus with zero output changes or failures.
Update the GNU3 parser to use shared DemangledTypeNode references for substitutions, template substitutions, and nested type/name construction so backrefs preserve structure instead of copying stale formatted strings. Fix template argument parsing for non-type template parameter declarations, expression arguments, argument packs, and generic lambda auto parameters while preserving enclosing template substitution state. Carry platform context through GNU3 demangling/finalization so rendered and finalized types use the same delayed DemangledTypeNode representation as the other demangler paths.
plafosse
force-pushed
the
test_ms_demangler_fixes
branch
from
02cebd9 to
7862d28
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 should be considered a ground up re-write and so you probably won't get any value out of looking at diffs of the actual demangler. It pretty much solves all known demangler accuracy issues. And lays the ground work for more performance unlocks when we get the new simplifier integrated.
Major changes:
to provide an abstraction layer between c++ features and binary
ninja's type objects.
this is due to cutting down on extraneous string copies and type object allocations
of back references.
I didn't feel that adding all those commits would be helpful in understanding what's
actually going on here.
Fixes: