From 1963d71cb8b8b4e66d71d83cde3d23f059386bcc Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Wed, 17 Jan 2018 20:19:04 +0000 Subject: [PATCH] [WebAssembly] Simplify generation of "names" section 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 | 2 +- lld/test/wasm/relocatable.ll | 2 ++ lld/test/wasm/weak-symbols.ll | 4 +++ lld/wasm/InputChunks.h | 13 ++++--- lld/wasm/Symbols.h | 7 +--- lld/wasm/Writer.cpp | 66 +++++++++++------------------------ 6 files changed, 37 insertions(+), 57 deletions(-) diff --git a/lld/test/wasm/alias.ll b/lld/test/wasm/alias.ll index 6f9015a555a7..a28280d8ef2f 100644 --- a/lld/test/wasm/alias.ll +++ b/lld/test/wasm/alias.ll @@ -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: ... diff --git a/lld/test/wasm/relocatable.ll b/lld/test/wasm/relocatable.ll index 615a9469f172..8803fd91d746 100644 --- a/lld/test/wasm/relocatable.ll +++ b/lld/test/wasm/relocatable.ll @@ -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: ... diff --git a/lld/test/wasm/weak-symbols.ll b/lld/test/wasm/weak-symbols.ll index 9c1233475324..6fe8c9f8a1d2 100644 --- a/lld/test/wasm/weak-symbols.ll +++ b/lld/test/wasm/weak-symbols.ll @@ -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 diff --git a/lld/wasm/InputChunks.h b/lld/wasm/InputChunks.h index d5a9e5c668aa..d8d03582cc3f 100644 --- a/lld/wasm/InputChunks.h +++ b/lld/wasm/InputChunks.h @@ -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 data() const override { return File->CodeSection->Content.slice(getInputSectionOffset(), @@ -138,12 +137,16 @@ protected: class SyntheticFunction : public InputFunction { public: - SyntheticFunction(const WasmSignature &S, ArrayRef Body) - : InputFunction(S, nullptr, nullptr), Body(Body) {} + SyntheticFunction(const WasmSignature &S, ArrayRef Body, + StringRef Name) + : InputFunction(S, nullptr, nullptr), Name(Name), Body(Body) {} + + StringRef getName() const override { return Name; } protected: ArrayRef data() const override { return Body; } + StringRef Name; ArrayRef Body; }; diff --git a/lld/wasm/Symbols.h b/lld/wasm/Symbols.h index afda399965e5..3e80944ae97a 100644 --- a/lld/wasm/Symbols.h +++ b/lld/wasm/Symbols.h @@ -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; diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp index 5cb5d55277aa..14fc90e9751b 100644 --- a/lld/wasm/Writer.cpp +++ b/lld/wasm/Writer.cpp @@ -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 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) + 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 BodyArray( reinterpret_cast(CtorFunctionBody.data()), CtorFunctionBody.size()); - CtorFunction = llvm::make_unique(Signature, BodyArray); + CtorFunction = llvm::make_unique( + Signature, BodyArray, Config->CtorSymbol->getName()); + CtorFunction->setOutputIndex(FunctionIndex); DefinedFunctions.emplace_back(CtorFunction.get()); }