[WebAssembly] Distinguish symbol/debug names for functions and test

[ncw]

Summary

Previously, the function's "name" was sometimes mangled, sometimes not. Now we have a clear differentiation between symbol names and debug names.

Also adds support for --print-gc-sections for synthetic functions, so that the tests can be extended to cover the function names and debug names separately.

[sbc100]

lgtm, but maybe fix the names first.

test/wasm/undefined-weak-call.ll

16 ↗

(On Diff #138221)

Is this really what the stubs are called? I think we should at least make them valid C identifiers. __undefined_xxx?

[ncw]

test/wasm/undefined-weak-call.ll

16 ↗

(On Diff #138221)

In the symbol table, the stubs would have the original name. In the "name" section they have this pretty format.

Your suggested prefixed C identifier would only appear in this one single place... We could do it, it might be cleaner, but I've just played around with a couple of options and they're somewhat inelegant.

We basically want to differentiate between an InputFunction's identifier, and the "name" it has as a WasmFunction (which is demangled). We haven't needed that distinction until now.

I was actually thinking of putting the boot on the other foot: really, the -print-gc-sections log messages should be demangled for all symbols, don't you think? That would be another way to regain consistency.

[ncw]

Updated, with clear differentiation between (symbol) name and debug name.

Requires corresponding LLVM commit D45021

lld/trunk/test/wasm/undefined-weak-call.ll

 ; RUN: llc -filetype=obj %s -o %t.o
-; RUN: wasm-ld --check-signatures --no-entry %t.o -o %t.wasm
+; RUN: wasm-ld --check-signatures --no-entry --print-gc-sections %t.o \
+; RUN:     -o %t.wasm 2>&1 | FileCheck -check-prefix=CHECK-GC %s
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
 ; Check that calling an undefined weak function generates an appropriate stub
 ; that will fail at runtime with "unreachable".
 
 target triple = "wasm32-unknown-unknown-wasm"
 
 declare extern_weak void @weakFunc1()
 declare extern_weak void @weakFunc2()         ; same signature
 declare extern_weak void @weakFunc3(i32 %arg) ; different
 declare extern_weak void @weakFunc4()         ; should be GC'd as not called
 
+; CHECK-GC: removing unused section {{.*}}:(weakFunc4)
+
 define i32 @callWeakFuncs() {
   call void @weakFunc1()
   call void @weakFunc2()
   call void @weakFunc3(i32 2)
   %addr1 = ptrtoint void ()* @weakFunc1 to i32
   %addr4 = ptrtoint void ()* @weakFunc4 to i32
   %sum = add i32 %addr1, %addr4
   ret i32 %sum

lld/trunk/wasm/Driver.cpp

for (Symbol *Sym : Symtab->getSymbols()) {
     // It is possible for undefined functions not to have a signature (eg. if
     // added via "--undefined"), but weak undefined ones do have a signature.
     assert(FuncSym->getFunctionType());
     const WasmSignature &Sig = *FuncSym->getFunctionType();
 
     // Add a synthetic dummy for weak undefined functions.  These dummies will
     // be GC'd if not used as the target of any "call" instructions.
     Optional<std::string> SymName = demangleItanium(Sym->getName());
-    StringRef StubName =
+    StringRef DebugName =
         Saver.save("undefined function " +
                    (SymName ? StringRef(*SymName) : Sym->getName()));
-    SyntheticFunction *Func = make<SyntheticFunction>(Sig, StubName);
+    SyntheticFunction *Func =
+        make<SyntheticFunction>(Sig, Sym->getName(), DebugName);
     Func->setBody(UnreachableFn);
     // Ensure it compares equal to the null pointer, and so that table relocs
     // don't pull in the stub body (only call-operand relocs should do that).
     Func->setTableIndex(0);
     Symtab->SyntheticFunctions.emplace_back(Func);
     // Hide our dummy to prevent export.
     uint32_t Flags = WASM_SYMBOL_VISIBILITY_HIDDEN;
     replaceSymbol<DefinedFunction>(Sym, Sym->getName(), Flags, nullptr, Func);

lld/trunk/wasm/InputChunks.h

public:
 
   void copyRelocations(const WasmSection &Section);
 
   void writeTo(uint8_t *SectionStart) const;
 
   ArrayRef<WasmRelocation> getRelocations() const { return Relocations; }
 
   virtual StringRef getName() const = 0;
+  virtual StringRef getDebugName() const = 0;
   virtual uint32_t getComdat() const = 0;
   StringRef getComdatName() const;
 
   size_t NumRelocations() const { return Relocations.size(); }
   void writeRelocations(llvm::raw_ostream &OS) const;
 
   ObjFile *File;
   int32_t OutputOffset = 0;

 public:
   InputSegment(const WasmSegment &Seg, ObjFile *F)
       : InputChunk(F, InputChunk::DataSegment), Segment(Seg) {}
 
   static bool classof(const InputChunk *C) { return C->kind() == DataSegment; }
 
   uint32_t getAlignment() const { return Segment.Data.Alignment; }
   StringRef getName() const override { return Segment.Data.Name; }
+  StringRef getDebugName() const override { return StringRef(); }
   uint32_t getComdat() const override { return Segment.Data.Comdat; }
 
   const OutputSegment *OutputSeg = nullptr;
   int32_t OutputSegmentOffset = 0;
 
 protected:
   ArrayRef<uint8_t> data() const override { return Segment.Data.Content; }
   uint32_t getInputSectionOffset() const override {
public:
   InputFunction(const WasmSignature &S, const WasmFunction *Func, ObjFile *F)
       : InputChunk(F, InputChunk::Function), Signature(S), Function(Func) {}
 
   static bool classof(const InputChunk *C) {
     return C->kind() == InputChunk::Function ||
            C->kind() == InputChunk::SyntheticFunction;
   }
 
-  StringRef getName() const override { return Function->Name; }
+  StringRef getName() const override { return Function->SymbolName; }
+  StringRef getDebugName() const override { return Function->DebugName; }
   uint32_t getComdat() const override { return Function->Comdat; }
   uint32_t getFunctionIndex() const { return FunctionIndex.getValue(); }
   bool hasFunctionIndex() const { return FunctionIndex.hasValue(); }
   void setFunctionIndex(uint32_t Index);
   uint32_t getTableIndex() const { return TableIndex.getValue(); }
   bool hasTableIndex() const { return TableIndex.hasValue(); }
   void setTableIndex(uint32_t Index);
 
protected:
 
   const WasmFunction *Function;
   llvm::Optional<uint32_t> FunctionIndex;
   llvm::Optional<uint32_t> TableIndex;
 };
 
 class SyntheticFunction : public InputFunction {
 public:
-  SyntheticFunction(const WasmSignature &S, StringRef Name)
-      : InputFunction(S, nullptr, nullptr), Name(Name) {
+  SyntheticFunction(const WasmSignature &S, StringRef Name,
+                    StringRef DebugName = {})
+      : InputFunction(S, nullptr, nullptr), Name(Name), DebugName(DebugName) {
     SectionKind = InputChunk::SyntheticFunction;
   }
 
   static bool classof(const InputChunk *C) {
     return C->kind() == InputChunk::SyntheticFunction;
   }
 
   StringRef getName() const override { return Name; }
+  StringRef getDebugName() const override { return DebugName; }
   uint32_t getComdat() const override { return UINT32_MAX; }
 
   void setBody(ArrayRef<uint8_t> Body_) { Body = Body_; }
 
 protected:
   ArrayRef<uint8_t> data() const override { return Body; }
 
   StringRef Name;
+  StringRef DebugName;
   ArrayRef<uint8_t> Body;
 };
 
 // Represents a single Wasm Section within an input file.
 class InputSection : public InputChunk {
 public:
   InputSection(const WasmSection &S, ObjFile *F)
       : InputChunk(F, InputChunk::Section), Section(S) {
     assert(Section.Type == llvm::wasm::WASM_SEC_CUSTOM);
   }
 
   StringRef getName() const override { return Section.Name; }
+  StringRef getDebugName() const override { return StringRef(); }
   uint32_t getComdat() const override { return UINT32_MAX; }
 
 protected:
   ArrayRef<uint8_t> data() const override { return Section.Content; }
 
   // Offset within the input section.  This is only zero since this chunk
   // type represents an entire input section, not part of one.
   uint32_t getInputSectionOffset() const override { return 0; }

lld/trunk/wasm/MarkLive.cpp

if (Config->PrintGcSections) {
     for (const ObjFile *Obj : Symtab->ObjectFiles) {
       for (InputChunk *C : Obj->Functions)
         if (!C->Live)
           message("removing unused section " + toString(C));
       for (InputChunk *C : Obj->Segments)
         if (!C->Live)
           message("removing unused section " + toString(C));
     }
+    for (InputChunk *C : Symtab->SyntheticFunctions)
+      if (!C->Live)
+        message("removing unused section " + toString(C));
   }
 }

lld/trunk/wasm/Writer.cpp

if (!Comdats.empty()) {
     Sub.writeTo(OS);
   }
 }
 
 // Create the custom "name" section containing debug symbol names.
 void Writer::createNameSection() {
   unsigned NumNames = NumImportedFunctions;
   for (const InputFunction *F : InputFunctions)
-    if (!F->getName().empty())
+    if (!F->getName().empty() || !F->getDebugName().empty())
       ++NumNames;
 
   if (NumNames == 0)
     return;
 
   SyntheticSection *Section = createSyntheticSection(WASM_SEC_CUSTOM, "name");
 
   SubSection Sub(WASM_NAMES_FUNCTION);
   writeUleb128(Sub.OS, NumNames, "name count");
 
   // Names must appear in function index order.  As it happens ImportedSymbols
   // and InputFunctions are numbered in order with imported functions coming
   // first.
   for (const Symbol *S : ImportedSymbols) {
     if (auto *F = dyn_cast<FunctionSymbol>(S)) {
       writeUleb128(Sub.OS, F->getFunctionIndex(), "func index");
       Optional<std::string> Name = demangleItanium(F->getName());
       writeStr(Sub.OS, Name ? StringRef(*Name) : F->getName(), "symbol name");
     }
   }
   for (const InputFunction *F : InputFunctions) {
     if (!F->getName().empty()) {
       writeUleb128(Sub.OS, F->getFunctionIndex(), "func index");
+      if (!F->getDebugName().empty()) {
+        writeStr(Sub.OS, F->getDebugName(), "symbol name");
+      } else {
         Optional<std::string> Name = demangleItanium(F->getName());
         writeStr(Sub.OS, Name ? StringRef(*Name) : F->getName(), "symbol name");
       }
     }
+  }
 
   Sub.writeTo(Section->getStream());
 }
 
 void Writer::writeHeader() {
   memcpy(Buffer->getBufferStart(), Header.data(), Header.size());
 }