Repository navigation
refactor(labels): read symbols through the demangle package - #379
Conversation
danielplohmann
left a comment
There was a problem hiding this comment.
Thanks for this. Getting rid of the pycxxfilt build and the swift demangle subprocess is a real improvement, and labels for 431 mangled names from the test files came out the same as on master, apart from the cases below. A few things before it can go in.
Must fix
NOTICEdrops the Team bi0srust_demangler(MIT) section, butsrc/smda/common/labelprovider/rust_demangler/rust.pyandmain.pystill ship and are derived from it (sameRustDemangler/determine_typestructure, sameTypeNotFoundErrormessage). TheUnableTo*Demangleclasses kept inrust_legacy.pyandrust_v0.pycome from there too. Please keep a trimmed bi0s section. Dropping the Ghidra section is fine.
Please disclose or reconsider
-
The description and CHANGELOG say Rust labels are unchanged, but these expectations changed in
tests/testRustSymbolProvider.py/tests/testPdbSymbolProvider.py:_RNvC3foo3bar.llvm.1234567890abcdef:foo::bar→foo::bar.llvm.1234567890abcdef_RIC1aKh_E: raisedUnableTov0Demangle→a::<0>_ZN3$u$E: refused →$u$_ZN17h0000000000000000E:""→ raisesUnableToLegacyDemangle
Most of these follow rustc-demangle and are fine, but please list them in the CHANGELOG.
-
A side effect of the last one:
is_rust_language_evidenceis now False for a hash-only name, sodemangle_itanium_symbol/demangle_macho_symbollabel_ZN17h0000000000000000Eas C++h0000000000000000. Before, it stayed mangled. Rare, but it is a new wrong label. Can the Itanium path leave hash-only names alone? -
Several of the nine MSVC names moved from DECLINED to
REFERENCE_READSare not well-formed:??_C@_05ABCDEFGH@hi?$AA@(the length says 5, the literal is shorter) and?x@?Q@@3HA(invalid scope, read asint ?Q::x). The old demangler refused anything it did not fully understand, and I would like to keep that for malformed input. Labels feed into MCRIT, so a confident wrong name costs more than a mangled one.
The dependency
demanglebecomes the one place every smda and MCRIT label comes from, so please pin it exactly:demangle==0.5.0. With the floor-only pin, any new release would change labels on new installs without an smda release, and stored labels would then drift between installs. With an exact pin, every change in demangling output arrives as a deliberate bump in an smda PR that we review like any other code change.core/registry.pyloads plugins from thedemangle.languagesentry-point group, and a plugin can replace a built-in scheme such asitanium. For smda I would rather that did not happen implicitly. Is there a way to use only the built-ins, or could the library make plugin loading opt-in?
Small
- The description still says "Draft until 0.5 is on PyPI". 0.5.0 has been out since 2026-10-07.
ItaniumDemangler, MsvcDemangler, MachoDemangler and rust_demangler keep their public functions and become wrappers over demangle, pinned at 0.5.0, replacing the pycxxfilt build, the vendored MSVC and Rust demanglers and the swift demangle subprocess. Every Itanium, MSVC and Rust label on the bundled fixtures is byte-identical; 32 Swift labels on Mach-O that needed a toolchain are now demangled on any host. MSVC names the grammar forbids are refused before reaching the library, which follows llvm-undname in reading past them: a literal whose byte count disagrees with its recorded length, a non-hex unnamed-namespace discriminator, a scope fragment "?" does not open, and a conversion operator with parameters. A legacy Rust name holding only its hash stays mangled instead of being read as C++. NOTICE keeps a trimmed Team bi0s section for the Rust entry point and exception classes that still ship.
The counterbalanced passes run master and the PR in one environment, so a dependency the PR drops must still be present for the base tree to import.
0887185 to
7f1641c
Compare
|
Thanks for going through it this carefully.
|
|
Point 6: demangle 0.5.1 is on PyPI and only reads |
0.6.0 keeps entry points behind load_plugins() and returns the same output as 0.5.1 for all 20,430 names the test suite passes it. The pin comment notes that a host calling load_plugins() or register_language() itself can still replace a scheme. The CHANGELOG entry names the four modules again, restores the native-build baseline for the cost figure and links danielplohmann#379 and danielplohmann#383.
0.6.0 moves labels on non-ASCII Itanium and Swift names and on ARM64e MSVC names, none of which the fixtures contain. The danielplohmann#379 entry goes back to describing the 0.5.1 pin it was measured on, and the bump gets its own bullet naming those changes. The pin comment now says a replacing plugin can change labels, not only leave them mangled.
* build(labels): bump demangle to 0.6.0, take #383 review nits 0.6.0 keeps entry points behind load_plugins() and returns the same output as 0.5.1 for all 20,430 names the test suite passes it. The pin comment notes that a host calling load_plugins() or register_language() itself can still replace a scheme. The CHANGELOG entry names the four modules again, restores the native-build baseline for the cost figure and links #379 and #383. * docs(changelog): give the demangle 0.6.0 bump its own entry 0.6.0 moves labels on non-ASCII Itanium and Swift names and on ARM64e MSVC names, none of which the fixtures contain. The #379 entry goes back to describing the 0.5.1 pin it was measured on, and the bump gets its own bullet naming those changes. The pin comment now says a replacing plugin can change labels, not only leave them mangled.
What this changes
ItaniumDemangler,MsvcDemangler,MachoDemanglerandrust_demanglerkeep every public function the rest of smda and the tests import, and become thin wrappers over thedemanglepackage: one pure-Python dependency in place of thepycxxfiltbuild, the vendored 1,386-line MSVC demangler, the vendored Rust demangler and theswift demanglesubprocess. Swift names are now read in-process, so a Linux host without a Swift toolchain labels them too.primeSwiftSymbolsstays as a no-op for callers. NOTICE drops the section for the demangler behaviour that no longer ships and keeps a trimmed Team bi0s section for the Rust entry point and exception classes that still do.Under
src/: +85 / -2,514 lines.Measured on the bundled fixtures
All 34 test binaries, the 12 Mach-O images and both PDBs, through the PE, ELF, Mach-O, PDB and Rust providers and
LanguageAnalyzer's C++ checks: 8,793 demangler calls, 6,705 distinct names.msvc_reference_corpus.txt, 609Every C++ and Rust answer agrees with
llvm-cxxfilt18; every MSVC answer withllvm-undname18, except that malformed names it reads past are refused (below). Of the 7,937 labels the fixtures store, 32 change, all Swift (27 in JokerSpy, 5 in HLoader), e.g._$s7HLoader4mainyyF->HLoader.main() -> ().is_itanium_cpp_symbol/is_msvc_cpp_symbolanswer the same for every name. PE-decorated C names (_name@8,__imp_..., ordinals) stay unchanged.Labelling the fixtures end to end: 381 ms -> 297 ms cold (median of 7). Per name, C++ is slower than the native build, Rust faster.
Label changes outside the fixtures
None of these names occurs in the bundled fixtures; each is a test expectation that changed.
Rust follows rustc-demangle:
_RNvC3foo3bar.llvm.1234567890abcdef:foo::bar->foo::bar.llvm.1234567890abcdef(rustc-demangle drops only an uppercase-hex.llvm.suffix)_RIC1aKh_E: raisedUnableTov0Demangle->a::<0>_ZN3$u$E: refused ->$u$_ZN17h0000000000000000E:""-> raisesUnableToLegacyDemangle. It is no longer Rust evidence, sodemangle_itanium_symbol/demangle_macho_symbolleave a hash-only legacy name mangled rather than reading it as C++h0000000000000000.MSVC:
demangle_msvc_symbolrefuses malformed names before calling the library, which followsllvm-undnamein reading past them: a string literal whose byte count disagrees with its recorded length (MSVC's 32-byte truncation allowed), a non-hexadecimal unnamed-namespace discriminator, a?scope fragment that is not?A,?$,??or a numbered local scope, and a conversion operator with parameters. Of the nine names the vendored demangler refused andllvm-undnamereads, the seven malformed ones are refused again;void f(...)(?f@@YAXZZ) andoperator co_await(??__L_deg@@YAHO@Z) are well-formed and are read. Over the 897 MSVC names in the tests andmsvc_reference_corpus.txt, nothing the vendored demangler read is refused, and no fixture name reaches the new checks.Tests
Full suite: 2,211 passed, 1 skipped. Well-formed shapes the vendored demangler refused are in
REFERENCE_READSwithllvm-undname's spelling (variadic, co_await,?a..?zescapes, a wide literal, a truncated literal); the malformed ones are inMALFORMEDand must come back unchanged. A hash-only legacy Rust name is checked on the Itanium and Mach-O paths.Dependency
demangle==0.5.0, pinned exactly: every label comes from it, so a change in its output arrives only as a reviewed bump here.The pin moved to
demangle==0.5.1, which loads no plugin unless asked, in #383.