[WebAssembly] Symbol changes #1, LLD

[ncw]

Summary

First chunk split out of D41955.

Must be committed simultaneously with LLD change, D42075.

Changes:

• Update for DEBUG_FUNCTION_NAME disappearing
• Simplify generation of "names" section by simply iterating over the DefinedFunctions array.
• Apparently, this even fixes some bugs, judging by the test changes required - some tests are asserting that functions are named multiple times, other tests are asserting that the "names" section contains the function's alias rather than its original name...
• Rolled in some tiny formatting changes...

[ncw]

Updated:

• Revert change to remove imports from NAME section; now names are written out for imported functions as before

[sbc100]

Landed two smaller parts as: rLLD322726 and rLLD322727

wasm/Writer.cpp

472 ↗

(On Diff #130124)

Maybe NumNames?

[sbc100]

One more split off change: rLLD322731

[sbc100]

Another change split off: D42193

lld/trunk/test/wasm/alias.ll

 ; CHECK-NEXT:         Body:            0B
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        0
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            name
 ; CHECK-NEXT:     FunctionNames:   
 ; CHECK-NEXT:       - Index:           0
-; CHECK-NEXT:         Name:            start_alias
+; CHECK-NEXT:         Name:            _start
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Name:            __wasm_call_ctors
 ; CHECK-NEXT: ...

lld/trunk/test/wasm/relocatable.ll

 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            puts
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Name:            foo_import
 ; CHECK-NEXT:       - Index:           2
 ; CHECK-NEXT:         Name:            hello
 ; CHECK-NEXT:       - Index:           3
 ; CHECK-NEXT:         Name:            my_func
+; CHECK-NEXT:       - Index:           4
+; CHECK-NEXT:         Name:            func_comdat
 ; CHECK-NEXT: ...

lld/trunk/test/wasm/weak-symbols.ll

 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        8
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            name
 ; CHECK-NEXT:     FunctionNames:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            _start
+; CHECK-NEXT:       - Index:           1
+; CHECK-NEXT:         Name:            weakFn
 ; CHECK-NEXT:       - Index:           2
 ; CHECK-NEXT:         Name:            exportWeak1
+; CHECK-NEXT:       - Index:           3
+; CHECK-NEXT:         Name:            weakFn
 ; CHECK-NEXT:       - Index:           4
 ; CHECK-NEXT:         Name:            exportWeak2
 ; CHECK-NEXT:       - Index:           5
 ; CHECK-NEXT:         Name:            __wasm_call_ctors
 ; CHECK-NEXT: ...

lld/trunk/wasm/InputChunks.h

 };
 
 // Represents a single wasm function within and input file.  These are
 // combined to create the final output CODE section.
 class InputFunction : public InputChunk {
 public:
   InputFunction(const WasmSignature &S, const WasmFunction *Func,
                 const ObjFile *F)
-      : InputChunk(F), Signature(S), WrittenToNameSec(false), Function(Func) {}
+      : InputChunk(F), Signature(S), Function(Func) {}
 
+  virtual StringRef getName() const { return Function->Name; }
   StringRef getComdat() const override { return Function->Comdat; }
   uint32_t getOutputIndex() const { return OutputIndex.getValue(); }
   bool hasOutputIndex() const { return OutputIndex.hasValue(); }
   void setOutputIndex(uint32_t Index);
 
   const WasmSignature &Signature;
 
-  unsigned WrittenToNameSec : 1;
-
 protected:
   ArrayRef<uint8_t> data() const override {
     return File->CodeSection->Content.slice(getInputSectionOffset(),
                                             Function->Size);
   }
   uint32_t getInputSectionOffset() const override {
     return Function->CodeSectionOffset;
   }
 
   const WasmFunction *Function;
   llvm::Optional<uint32_t> OutputIndex;
 };
 
 class SyntheticFunction : public InputFunction {
 public:
-  SyntheticFunction(const WasmSignature &S, ArrayRef<uint8_t> Body)
-      : InputFunction(S, nullptr, nullptr), Body(Body) {}
+  SyntheticFunction(const WasmSignature &S, ArrayRef<uint8_t> Body,
+                    StringRef Name)
+      : InputFunction(S, nullptr, nullptr), Name(Name), Body(Body) {}
+
+  StringRef getName() const override { return Name; }
 
 protected:
   ArrayRef<uint8_t> data() const override { return Body; }
 
+  StringRef Name;
   ArrayRef<uint8_t> Body;
 };
 
 } // namespace wasm
 } // namespace lld
 
 #endif // LLD_WASM_INPUT_CHUNKS_H

lld/trunk/wasm/Symbols.h

enum Kind {
     LazyKind,
     UndefinedFunctionKind,
     UndefinedGlobalKind,
 
     LastDefinedKind = DefinedGlobalKind,
     InvalidKind,
   };
 
-  Symbol(StringRef Name, uint32_t Flags)
-      : WrittenToNameSec(0), Flags(Flags), Name(Name) {}
+  Symbol(StringRef Name, uint32_t Flags) : Flags(Flags), Name(Name) {}
 
   Kind getKind() const { return SymbolKind; }
 
   bool isLazy() const { return SymbolKind == LazyKind; }
   bool isDefined() const { return SymbolKind <= LastDefinedKind; }
   bool isUndefined() const {
     return SymbolKind == UndefinedGlobalKind ||
            SymbolKind == UndefinedFunctionKind;
public:
   void update(Kind K, InputFile *F = nullptr, uint32_t Flags = 0,
               const InputSegment *Segment = nullptr,
               InputFunction *Function = nullptr, uint32_t Address = UINT32_MAX);
 
   void setArchiveSymbol(const Archive::Symbol &Sym) { ArchiveSymbol = Sym; }
   const Archive::Symbol &getArchiveSymbol() { return ArchiveSymbol; }
   InputFunction *getFunction() { return Function; }
 
-  // This bit is used by Writer::writeNameSection() to prevent
-  // symbols from being written to the symbol table more than once.
-  unsigned WrittenToNameSec : 1;
-
 protected:
   uint32_t Flags;
   uint32_t VirtualAddress = 0;
 
   StringRef Name;
   Archive::Symbol ArchiveSymbol = {nullptr, 0, 0};
   Kind SymbolKind = InvalidKind;
   InputFile *File = nullptr;

lld/trunk/wasm/Writer.cpp

if (!Comdats.empty()) {
     }
     SubSection.finalizeContents();
     SubSection.writeToStream(OS);
   }
 }
 
 // Create the custom "name" section containing debug symbol names.
 void Writer::createNameSection() {
-  // Create an array of all function sorted by function index space
-  std::vector<const Symbol *> Names;
+  unsigned NumNames = ImportedFunctions.size();
+  for (const InputFunction *F : DefinedFunctions)
+    if (!F->getName().empty())
+      ++NumNames;
 
-  auto AddToNames = [&](Symbol* S) {
-    if (!S->isFunction() || S->WrittenToNameSec)
-        return;
-    // We also need to guard against two different symbols (two different
-    // names) for the same wasm function.  While this is possible (aliases)
-    // it is not legal in the "name" section.
-    InputFunction *Function = S->getFunction();
-    if (Function) {
-      if (Function->WrittenToNameSec)
-        return;
-      Function->WrittenToNameSec = true;
-    }
-    S->WrittenToNameSec = true;
-    Names.emplace_back(S);
-  };
-
-  for (ObjFile *File : Symtab->ObjectFiles) {
-    Names.reserve(Names.size() + File->getSymbols().size());
-    DEBUG(dbgs() << "adding names from: " << File->getName() << "\n");
-    for (Symbol *S : File->getSymbols()) {
-      if (S->isWeak())
-        continue;
-      AddToNames(S);
-    }
-  }
-
-  DEBUG(dbgs() << "adding symtab names\n");
-  for (Symbol *S : Symtab->getSymbols()) {
-    DEBUG(dbgs() << "sym: " << S->getName() << "\n");
-    if (S->getFile())
-      continue;
-    AddToNames(S);
-  }
+  if (NumNames == 0)
 
   SyntheticSection *Section = createSyntheticSection(WASM_SEC_CUSTOM, "name");
 
-  std::sort(Names.begin(), Names.end(), [](const Symbol *A, const Symbol *B) {
-    return A->getOutputIndex() < B->getOutputIndex();
-  });
-
   SubSection FunctionSubsection(WASM_NAMES_FUNCTION);
   raw_ostream &OS = FunctionSubsection.getStream();
-  writeUleb128(OS, Names.size(), "name count");
+  writeUleb128(OS, NumNames, "name count");
 
-  // We have to iterate through the inputs twice so that all the imports
-  // appear first before any of the local function names.
-  for (const Symbol *S : Names) {
-    writeUleb128(OS, S->getOutputIndex(), "func index");
+  // Names must appear in function index order.  As it happens ImportedFunctions
+  // and DefinedFunctions are numbers in order with imported functions coming
+  // first.
+  for (const Symbol *S : ImportedFunctions) {
+    writeUleb128(OS, S->getOutputIndex(), "import index");
     writeStr(OS, S->getName(), "symbol name");
   }
+  for (const InputFunction *F : DefinedFunctions) {
+    if (!F->getName().empty()) {
+      writeUleb128(OS, F->getOutputIndex(), "func index");
+      writeStr(OS, F->getName(), "symbol name");
+    }
+  }
 
   FunctionSubsection.finalizeContents();
   FunctionSubsection.writeToStream(Section->getStream());
 }
 
 void Writer::writeHeader() {
   memcpy(Buffer->getBufferStart(), Header.data(), Header.size());
 }
void Writer::createCtorFunction() {
   // Once we know the size of the body we can create the final function body
   raw_string_ostream OS(CtorFunctionBody);
   writeUleb128(OS, FunctionBody.size(), "function size");
   OS.flush();
   CtorFunctionBody += FunctionBody;
   ArrayRef<uint8_t> BodyArray(
       reinterpret_cast<const uint8_t *>(CtorFunctionBody.data()),
       CtorFunctionBody.size());
-  CtorFunction = llvm::make_unique<SyntheticFunction>(Signature, BodyArray);
+  CtorFunction = llvm::make_unique<SyntheticFunction>(
+      Signature, BodyArray, Config->CtorSymbol->getName());
+  CtorFunction->setOutputIndex(FunctionIndex);
   DefinedFunctions.emplace_back(CtorFunction.get());
 }
 
 // Populate InitFunctions vector with init functions from all input objects.
 // This is then used either when creating the output linking section or to
 // synthesize the "__wasm_call_ctors" function.
 void Writer::calculateInitFunctions() {
   for (ObjFile *File : Symtab->ObjectFiles) {