[WebAssembly] Simplify generation of "names" section
authorSam Clegg <sbc@chromium.org>
Wed, 17 Jan 2018 20:19:04 +0000 (20:19 +0000)
committerSam Clegg <sbc@chromium.org>
Wed, 17 Jan 2018 20:19:04 +0000 (20:19 +0000)
Simplify generation of "names" section by simply iterating
over the DefinedFunctions array.

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

Patch by Nicholas Wilson!

Differential Revision: https://reviews.llvm.org/D42076

llvm-svn: 322751

lld/test/wasm/alias.ll
lld/test/wasm/relocatable.ll
lld/test/wasm/weak-symbols.ll
lld/wasm/InputChunks.h
lld/wasm/Symbols.h
lld/wasm/Writer.cpp

index 6f9015a..a28280d 100644 (file)
@@ -77,7 +77,7 @@ entry:
 ; 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: ...
index 615a946..8803fd9 100644 (file)
@@ -239,4 +239,6 @@ entry:
 ; CHECK-NEXT:         Name:            hello
 ; CHECK-NEXT:       - Index:           3
 ; CHECK-NEXT:         Name:            my_func
+; CHECK-NEXT:       - Index:           4
+; CHECK-NEXT:         Name:            func_comdat
 ; CHECK-NEXT: ...
index 9c12334..6fe8c9f 100644 (file)
@@ -114,8 +114,12 @@ entry:
 ; 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
index d5a9e5c..d8d0358 100644 (file)
@@ -112,8 +112,9 @@ 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(); }
@@ -121,8 +122,6 @@ public:
 
   const WasmSignature &Signature;
 
-  unsigned WrittenToNameSec : 1;
-
 protected:
   ArrayRef<uint8_t> data() const override {
     return File->CodeSection->Content.slice(getInputSectionOffset(),
@@ -138,12 +137,16 @@ protected:
 
 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;
 };
 
index afda399..3e80944 100644 (file)
@@ -38,8 +38,7 @@ public:
     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; }
 
@@ -100,10 +99,6 @@ public:
   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;
index 5cb5d55..14fc90e 100644 (file)
@@ -469,59 +469,33 @@ void Writer::createLinkingSection() {
 
 // 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);
-  };
+  unsigned NumNames = ImportedFunctions.size();
+  for (const InputFunction *F : DefinedFunctions)
+    if (!F->getName().empty())
+      ++NumNames;
 
-  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)
+    return;
 
   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());
@@ -784,7 +758,9 @@ void Writer::createCtorFunction() {
   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());
 }