[PATCH] D44316: [WebAssembly] Demangle symbol names for use by the browser debugger

[Nicholas Wilson via Phabricator via llvm-commits]

ncw added a comment.

I opened a can of worms! I think it can be cleaned up though...

In https://reviews.llvm.org/D44316#1037421, @sbc100 wrote:

> Hmm.. its kind of shame maybe that we decided to demangle the names in the name section as they no longer match the symbol table names.
>
> If we want to stick with the decision than maybe we should move to having libObject completely ignore the name section.  For example, all of the native tooling expects symbol names to be mangled with the option to de-mangle them (think `nm` vs `nm -C`).


That's currently pretty-much how it is. libObject does use the "name" section to assign names to Functions (and will assign names to Segments when the "name" section is soon extended to allow for that...) - but that's fairly reasonable. We never currently have unmangled symbol names, which is what the tooling expects. Symbol names are sacrosanct, I haven't gone near them.

> Anyway, I think all internal names and symbols should be mangled and valid C identifiers.   Demangling things should be reserved for the moment of displaying.   For this reason I think we should either reverse our decision to store demangled names in the name section,  or ignore the name section when it comes to naming things internally and rely on the symbol table instead.

I'm not so sure - is there really a big problem with having free-form text for Function/Segment names? That's what it comes down to... the browser needs them unmangled, and the "name" section is a straightforward assignment of names to the Wasm objects... so why not have our Wasm objects named with some demangled names?

If that's //not// allowed... then the fix should be fairly straightforward. We'll just create a "getDebugName()" field on InputChunk, which will be similar to the existing getName() field, it will be set from the "name" section by libObject, and written out to the "name" section (defaulting to just getName() if there's no DebugName on the Chunk).

I think that would satisfy all the concerns above?

In https://reviews.llvm.org/D44316#1037454, @dschuff wrote:

> I think the symbol table (and any names that actually have meaning to the compiler and/or linker) should be mangled names, and only the name section should have demangled names. I also think it makes sense for libObject to only use the symbol table and ignore the name section. In other words, the name section should be thought of as metadata or debug info and in that sense shouldn't be "trusted" to always be correct (although of course we should preserve it wherever we can).


Yup, that's roughly much how it is. All meaningful names are mangled currently. Except that currently, libObject passes through the "name" section names, by setting the WasmFunction's Name field - however that's not a "meaningful" name to LLD, it's only used as the string for some logging messages, and for writing back into the "name" section.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44316