Page MenuHomePhabricator

[llvm-symbolizer] Remove underscores and other C mangling on Windows
ClosedPublic

Authored by rnk on Aug 5 2015, 3:51 PM.

Details

Summary

This makes it so that reports symbolized after the fact with
llvm-symbolizer are more similar to the ones we generate at runtime with
in-process dbghelp.

Diff Detail

Repository
rL LLVM

Event Timeline

rnk updated this revision to Diff 31410.Aug 5 2015, 3:51 PM
rnk retitled this revision from to [llvm-symbolizer] Remove underscores and other C mangling on Windows.
rnk updated this object.
rnk added a reviewer: eugenis.
rnk added a subscriber: llvm-commits.
eugenis edited reviewers, added: samsonov; removed: eugenis.Aug 5 2015, 4:02 PM
eugenis added a subscriber: eugenis.
samsonov added inline comments.Aug 5 2015, 4:11 PM
tools/llvm-symbolizer/LLVMSymbolize.cpp
226 ↗(On Diff #31410)

This is not the place we demangle the function names - see LLVMSymbolizer::DemangleName. At that point we already lose the knowledge of whether the module was ELF, or COFF, or MachO, but is it that important? Also, why doesn't ::UnDecorateSymbolName work for you?

rnk added inline comments.Aug 5 2015, 4:17 PM
tools/llvm-symbolizer/LLVMSymbolize.cpp
226 ↗(On Diff #31410)

Yes, it's important that we only do this for 32-bit x86 symbols, which requires looking at the machine type of the module. See isWin32Module().

UnDecorateSymbolName only handles C++ symbols. This won't interfere because C++ symbols all start with '?'.

samsonov added inline comments.Aug 6 2015, 5:52 PM
tools/llvm-symbolizer/LLVMSymbolize.cpp
92 ↗(On Diff #31410)

See below - if you make it a part of DemangleName , you can simplify the code with early-returns.

226 ↗(On Diff #31410)

Still, let's keep all the demangling work in a single place - where we actually render DILineInfo entries. If you need to know smth. about the object file that produced these DILineInfo entries (which looks reasonable - demangling is obviously format-dependent) - just pass it down to printDILineInfo/DemangleName.

rnk updated this revision to Diff 31709.Aug 10 2015, 12:38 PM
  • Add IsWin32Symbol to DILineInfo
tools/llvm-symbolizer/LLVMSymbolize.cpp
226 ↗(On Diff #31410)

I can do that, but it seems kind of silly to add a field to DILineInfo that is only used in LLVMSymbolize.cpp. There are other platforms with underscore prefixes, like MachO, that don't go this way. The underscore is removed earlier under an isMachO() in addSymbol().

samsonov added inline comments.Aug 10 2015, 1:44 PM
tools/llvm-symbolizer/LLVMSymbolize.cpp
188 ↗(On Diff #31709)

Ah, sorry, I wasn't clear enough :(
See below.

217 ↗(On Diff #31709)

You can pass ModuleInfo* (or the kind of that object file) to printDILineInfo here.

233 ↗(On Diff #31709)

and here

542 ↗(On Diff #31709)

and use the type of object file here to figure out which demangling you should actually do

rnk updated this revision to Diff 31717.Aug 10 2015, 1:54 PM
  • Pass ModuleInfo down into printDILineInfo
tools/llvm-symbolizer/LLVMSymbolize.cpp
188 ↗(On Diff #31709)

Oh, that makes a lot more sense. :)

samsonov accepted this revision.Aug 10 2015, 2:24 PM
samsonov edited edge metadata.

LGTM with a note.

tools/llvm-symbolizer/LLVMSymbolize.cpp
568 ↗(On Diff #31717)

This code will be unreachable if _MSC_VER is not defined.

This revision is now accepted and ready to land.Aug 10 2015, 2:24 PM
This revision was automatically updated to reflect the committed changes.