[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

471–472

Maybe NumNames?

[sbc100]

One more split off change: rLLD322731

[sbc100]

Another change split off: D42193

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: ...

test/wasm/archive.ll

entry:
   %call2 = call i32 @missing_func() #2
   ret i32 %call2
 }
 
 ; Verify that multually dependant object files in an archive is handled
 ; correctly.
 
 ; CHECK:      00000003 T _start
-; CHECK-NEXT: 00000003 T _start
 ; CHECK-NEXT: 00000001 T bar
-; CHECK-NEXT: 00000001 T bar
-; CHECK-NEXT: 00000002 T foo
 ; CHECK-NEXT: 00000002 T foo
 
 ; Verify that symbols from unused objects don't appear in the symbol table
 ; CHECK-NOT: hello
 
 ; Specifying the same archive twice is allowed.
 ; RUN: lld -flavor wasm %t.a %t.a %t.o -o %t.wasm

test/wasm/relocatable.ll

 ; CHECK-NEXT:         Entries:
 ; CHECK-NEXT:           - Kind:            FUNCTION
 ; CHECK-NEXT:             Index:           4
 ; CHECK-NEXT:           - Kind:            DATA
 ; CHECK-NEXT:             Index:           4
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            name
 ; CHECK-NEXT:     FunctionNames:
-; 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: ...

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: ...

wasm/InputChunks.h

 namespace lld {
 namespace wasm {
 
 class ObjFile;
 class OutputSegment;
 
 class InputChunk {
 public:
-  virtual uint32_t getSize() const = 0;
+  virtual StringRef getName() const = 0;
+  virtual ArrayRef<uint8_t> data() const = 0;
+  virtual StringRef getComdat() const = 0;
 
   void copyRelocations(const WasmSection &Section);
 
+  uint32_t getSize() const { return data().size(); }
   void writeTo(uint8_t *SectionStart) const;
 
   void setOutputOffset(uint32_t Offset) {
     OutputOffset = Offset;
     calcRelocations();
   }
 
   uint32_t getOutputOffset() const { return OutputOffset; }
 
-  virtual StringRef getComdat() const = 0;
 
   bool Discarded = false;
   std::vector<OutputRelocation> OutRelocations;
 
 protected:
   InputChunk(const ObjFile *F) : File(F) {}
   virtual ~InputChunk() = default;
   void calcRelocations();
-  virtual ArrayRef<uint8_t> data() const = 0;
   virtual uint32_t getInputSectionOffset() const = 0;
 
   std::vector<WasmRelocation> Relocations;
   int32_t OutputOffset = 0;
   const ObjFile *File;
 };
 
 // Represents a WebAssembly data segment which can be included as part of
 // an output data segments.  Note that in WebAssembly, unlike ELF and other
 // formats, used the term "data segment" to refer to the continous regions of
 // memory that make on the data section. See:
 // https://webassembly.github.io/spec/syntax/modules.html#syntax-data
 //
 // For example, by default, clang will produce a separate data section for
 // each global variable.
 class InputSegment : public InputChunk {
 public:
   InputSegment(const WasmSegment &Seg, const ObjFile *F)
       : InputChunk(F), Segment(Seg) {}
 
+  ArrayRef<uint8_t> data() const override { return Segment.Data.Content; }
+  StringRef getName() const override { return Segment.Data.Name; }
+  StringRef getComdat() const override { return Segment.Data.Comdat; }
+
   // Translate an offset in the input segment to an offset in the output
   // segment.
   uint32_t translateVA(uint32_t Address) const;
 
   const OutputSegment *getOutputSegment() const { return OutputSeg; }
 
   void setOutputSegment(const OutputSegment *Segment, uint32_t Offset) {
     OutputSeg = Segment;
     OutputSegmentOffset = Offset;
   }
 
-  uint32_t getSize() const override { return Segment.Data.Content.size(); }
   uint32_t getAlignment() const { return Segment.Data.Alignment; }
   uint32_t startVA() const { return Segment.Data.Offset.Value.Int32; }
   uint32_t endVA() const { return startVA() + getSize(); }
-  StringRef getName() const { return Segment.Data.Name; }
-  StringRef getComdat() const override { return Segment.Data.Comdat; }
 
   int32_t OutputSegmentOffset = 0;
 
 protected:
-  ArrayRef<uint8_t> data() const override { return Segment.Data.Content; }
   uint32_t getInputSectionOffset() const override {
     return Segment.SectionOffset;
   }
 
   const WasmSegment &Segment;
   const OutputSegment *OutputSeg = nullptr;
 };
 
 // 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) {}
 
-  uint32_t getSize() const override { return Function->Size; }
+  StringRef getName() const override { return Function->Name; }
+  ArrayRef<uint8_t> data() const override {
+    return File->CodeSection->Content.slice(getInputSectionOffset(), Function->Size);
+  }
   StringRef getComdat() const override { return Function->Comdat; }
-  uint32_t getOutputIndex() const { return OutputIndex.getValue(); };
-  bool hasOutputIndex() const { return OutputIndex.hasValue(); };
+
+  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(), getSize());
-  }
   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) {}
-
-  uint32_t getSize() const override { return Body.size(); }
+  SyntheticFunction(const WasmSignature &S, ArrayRef<uint8_t> Body,
+                    StringRef Name)
+      : InputFunction(S, nullptr, nullptr), Name(Name), Body(Body) {}
 
-protected:
+  StringRef getName() const override { return Name; }
   ArrayRef<uint8_t> data() const override { return Body; }
 
+protected:
+  StringRef Name;
   ArrayRef<uint8_t> Body;
 };
 
 } // namespace wasm
 } // namespace lld
 
 #endif // LLD_WASM_INPUT_CHUNKS_H

wasm/InputFiles.cpp

case WasmSymbol::SymbolType::GLOBAL_EXPORT: {
       } else {
         Segment->Discarded = true;
         LLVM_FALLTHROUGH; // Exclude global, and add the symbol as undefined
       }
     }
     case WasmSymbol::SymbolType::GLOBAL_IMPORT:
       S = createUndefined(WasmSym, Symbol::Kind::UndefinedGlobalKind);
       break;
-    case WasmSymbol::SymbolType::DEBUG_FUNCTION_NAME:
-      // These are for debugging only, no need to create linker symbols for them
-      continue;
     }
 
     Symbols.push_back(S);
     if (WasmSym.isFunction()) {
       FunctionSymbols[WasmSym.ElementIndex] = S;
       if (WasmSym.HasAltIndex)
         FunctionSymbols[WasmSym.AltIndex] = S;
     } else {

wasm/Symbols.h

enum Kind {
     UndefinedFunctionKind,
     UndefinedGlobalKind,
 
     LastDefinedKind = DefinedGlobalKind,
     InvalidKind,
   };
 
   Symbol(StringRef Name, uint32_t Flags)
-      : WrittenToSymtab(0), WrittenToNameSec(0), Flags(Flags), Name(Name) {}
+      : 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 WrittenToSymtab : 1;
-  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;

wasm/Writer.cpp

private:
   void openFile();
 
   uint32_t lookupType(const WasmSignature &Sig);
   uint32_t registerType(const WasmSignature &Sig);
   void createCtorFunction();
   void calculateInitFunctions();
   void assignIndexes();
   void calculateImports();
-  void calculateOffsets();
   void calculateTypes();
   void createOutputSegments();
   void layoutMemory();
   void createHeader();
   void createSections();
   SyntheticSection *createSyntheticSection(uint32_t Type,
                                            StringRef Name = "");
 
if (Segments.size()) {
     }
     SubSection.finalizeContents();
     SubSection.writeToStream(OS);
   }
 
   if (!InitFunctions.empty()) {
     SubSection SubSection(WASM_INIT_FUNCS);
     writeUleb128(SubSection.getStream(), InitFunctions.size(),
-                 "num init functionsw");
+                 "num init functions");
     for (const WasmInitFunc &F : InitFunctions) {
       writeUleb128(SubSection.getStream(), F.Priority, "priority");
       writeUleb128(SubSection.getStream(), F.FunctionIndex, "function index");
     }
     SubSection.finalizeContents();
     SubSection.writeToStream(OS);
   }
 
for (const auto &C : Comdats) {
       }
     }
     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;
-
-  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);
-  }
+  unsigned Names = 0;
+  for (const InputFunction *F : DefinedFunctions)
+    if (!F->getName().empty())
+      ++Names;
 
   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, Names, "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");
-    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) {