This is an archive of the discontinued LLVM Phabricator instance.

llvm-nm: Add suport for symbol demangling (-C/--demangle)
ClosedPublic

Authored by sbc100 on Jun 26 2017, 10:54 PM.

Event Timeline

sbc100 created this revision.Jun 26 2017, 10:54 PM
sbc100 updated this revision to Diff 104222.Jun 27 2017, 11:20 AM
  • add test
sbc100 added a subscriber: llvm-commits.
davide requested changes to this revision.Jun 27 2017, 11:24 AM

See the comment inline. If you have an example of mangled symbols starting with ___Z, you might consider adding a test for it.

tools/llvm-nm/llvm-nm.cpp
668–669

I'm aware that Itanium mangled symbols start with _Z but I'm not aware of the ___Z prefix, do you have an example?

This revision now requires changes to proceed.Jun 27 2017, 11:24 AM
sbc100 added inline comments.Jun 27 2017, 11:37 AM
tools/llvm-nm/llvm-nm.cpp
668–669

I'm mostly cargo culting this from elsewhere in the codebase.

See tools/llvm-cxxdump/llvm-cxxdump.cpp. It does a lot of checking for _Z symbols and it always checks for the double under version as well.

tools/llvm-objdump/MachODump.cpp seems to only check for the double under version.

tools/llvm-cxxfilt/llvm-cxxfilt.cpp checks both.

git grep __Z seems to indicate that its the Mach-O format the includes the extra _ which would explain why MachODump.cpp only checks for that.

I'll dig a bit deeper and try to add another test.

sbc100 updated this revision to Diff 104231.Jun 27 2017, 11:58 AM
sbc100 edited edge metadata.
  • Add test coverage for Mach-O symbols which start with _

Turned out to be mach-o thing. I added test coverage and a comment,

sbc100 updated this revision to Diff 104266.Jun 27 2017, 1:58 PM
  • simplify test case

I'm fine with this but I'd appreciate another pair of eyes as it's been a while since I touched the dumpers.

tools/llvm-nm/llvm-nm.cpp
759

Do you need a copy here?

sbc100 updated this revision to Diff 104304.Jun 27 2017, 4:05 PM
  • simlify test case
  • only strip leaning _ on MachO
sbc100 updated this revision to Diff 104703.Jun 29 2017, 11:28 AM
  • remove redundant size check
This revision was automatically updated to reflect the committed changes.
ruiu added a subscriber: ruiu.Jun 29 2017, 11:58 AM
ruiu added inline comments.
llvm/trunk/tools/llvm-nm/llvm-nm.cpp
667 ↗(On Diff #104704)

This may be a dumb question, but does MachO use the Itanium mangling scheme?

Does it use both "__Z" and "_Z" as prefixes?

668 ↗(On Diff #104704)

You can use startswith

sbc100 added inline comments.Jun 29 2017, 12:03 PM
llvm/trunk/tools/llvm-nm/llvm-nm.cpp
667 ↗(On Diff #104704)

The macho files that llvm generates certainly seem too (see the tests code in this CL).

It seems they always add the "_", so you either get "__Z" of MachO and "_Z" on Elf. One or the other, not both. Based purely on the empirical behaviour of llvm today.