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

[Nicholas Wilson via Phabricator via llvm-commits]

ncw added inline comments.


================
Comment at: wasm/Driver.cpp:246
+        Saver.save("undefined function " +
+                   (SymName ? StringRef(*SymName) : Sym->getName()));
     SyntheticFunction *Func = make<SyntheticFunction>(Sig, StubName);
----------------
sbc100 wrote:
> ncw wrote:
> > sbc100 wrote:
> > > Why this change?   
> > To make it consistent - here we're demangling a symbol name and putting it in the "name" section, and in Writer.cpp we're also demangling a symbol name and putting it in a name section.
> > 
> > In Writer, you agreed we wanted to demangle, regardless of whether `Config->Demangle` was set... so here we should also be demangling regardless of whether the config option is set.
> > 
> > I've added this condition to the test, to make it clearer that it's part of the same change, and is tested in the same place.
> It looks like what you are doing here is creating a new function, with a given name.  Function name should be mangled shouldn't they?  They only get demangled when you make the name section.  Don't you want the stub function to have the same name as the symbol?  I'm still confused why you demangle here.
I'm demangling here because in my mind the function name is the demangled thing? The InputChunk's name is set from reading in the "name" section - or if that doesn't exist it falls back to using the symbol name.

If we want the symbol to appear as "prefix + demangled name" (eg "undefined function foo()") in the browser stack traces, then we kind of have to demangle at the point where we create the SyntheticFunction. We can't create something that will demangle to that, given the prefix...

So what's messy is that WasmFunction::Name is already a sometimes-mangled-sometimes-demangled field (since it can be set from the "name" section or the symbol name).

Do you have a suggestion for how you'd do it - so that its name in the "name" section is "undefined function foo()", but so that the SyntheticFunction itself has a different name?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44316