From eb7fd730faeb23938117f4dd8bd6867cd1143d61 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Wed, 17 Jan 2018 19:28:43 +0000 Subject: [PATCH] [WebAssembly] Remove debug names from symbol table Get rid of DEBUG_FUNCTION_NAME symbols. When we actually debug data, maybe we'll want somewhere to put it... but having a symbol that just stores the name of another symbol seems odd. It means you have multiple Symbols with the same name, one containing the actual function and another containing the name! Store the names in a vector on the WasmObjectFile when reading them in. Also stash them on the WasmFunctions themselves. The names are //not// "symbol names" or aliases or anything, they're just the name that a debugger should show against the function body itself. NB. The WasmObjectFile stores them so that they can be exported in the YAML losslessly, and hence the tests can be precise. Enforce that the CODE section has been read in before reading the "names" section. Requires minor adjustment to some tests. Patch by Nicholas Wilson! Differential Revision: https://reviews.llvm.org/D42075 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@322741 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/BinaryFormat/Wasm.h | 12 +++++-- include/llvm/Object/Wasm.h | 6 ++-- lib/MC/WasmObjectWriter.cpp | 25 ++++++------- lib/Object/WasmObjectFile.cpp | 35 +++++++++++++------ test/MC/WebAssembly/weak-alias.ll | 6 ---- test/ObjectYAML/wasm/weak_symbols.yaml | 8 +++++ test/tools/llvm-nm/wasm/exports.yaml | 17 +++++++++ test/tools/llvm-nm/wasm/weak-symbols.yaml | 14 ++++++++ .../WebAssembly/symbol-table.test | 3 -- test/tools/llvm-readobj/symbols.test | 15 -------- tools/llvm-readobj/WasmDumper.cpp | 1 - tools/obj2yaml/wasm2yaml.cpp | 9 ++--- 12 files changed, 90 insertions(+), 61 deletions(-) diff --git a/include/llvm/BinaryFormat/Wasm.h b/include/llvm/BinaryFormat/Wasm.h index 80fd9a47a8d..d2ebe187cea 100644 --- a/include/llvm/BinaryFormat/Wasm.h +++ b/include/llvm/BinaryFormat/Wasm.h @@ -95,7 +95,8 @@ struct WasmFunction { ArrayRef Body; uint32_t CodeSectionOffset; uint32_t Size; - StringRef Comdat; + StringRef Name; // from the "names" section + StringRef Comdat; // from the "comdat info" section }; struct WasmDataSegment { @@ -105,7 +106,7 @@ struct WasmDataSegment { StringRef Name; uint32_t Alignment; uint32_t Flags; - StringRef Comdat; + StringRef Comdat; // from the "comdat info" section }; struct WasmElemSegment { @@ -116,7 +117,7 @@ struct WasmElemSegment { struct WasmRelocation { uint32_t Type; // The type of the relocation. - uint32_t Index; // Index into function to global index space. + uint32_t Index; // Index into function or global index space. uint64_t Offset; // Offset from the start of the section. int64_t Addend; // A value to add to the symbol. }; @@ -126,6 +127,11 @@ struct WasmInitFunc { uint32_t FunctionIndex; }; +struct WasmFunctionName { + uint32_t Index; + StringRef Name; +}; + struct WasmLinkingData { uint32_t DataSize; std::vector InitFunctions; diff --git a/include/llvm/Object/Wasm.h b/include/llvm/Object/Wasm.h index fce8603ac59..22e19a16bc7 100644 --- a/include/llvm/Object/Wasm.h +++ b/include/llvm/Object/Wasm.h @@ -39,7 +39,6 @@ public: FUNCTION_EXPORT, GLOBAL_IMPORT, GLOBAL_EXPORT, - DEBUG_FUNCTION_NAME, }; WasmSymbol(StringRef Name, SymbolType Type, uint32_t Section, @@ -70,8 +69,7 @@ public: bool isFunction() const { return Type == WasmSymbol::SymbolType::FUNCTION_IMPORT || - Type == WasmSymbol::SymbolType::FUNCTION_EXPORT || - Type == WasmSymbol::SymbolType::DEBUG_FUNCTION_NAME; + Type == WasmSymbol::SymbolType::FUNCTION_EXPORT; } @@ -150,6 +148,7 @@ public: ArrayRef dataSegments() const { return DataSegments; } ArrayRef functions() const { return Functions; } ArrayRef comdats() const { return Comdats; } + ArrayRef debugNames() const { return DebugNames; } uint32_t startFunction() const { return StartFunction; } void moveSymbolNext(DataRefImpl &Symb) const override; @@ -253,6 +252,7 @@ private: std::vector Functions; std::vector Symbols; std::vector Comdats; + std::vector DebugNames; uint32_t StartFunction = -1; bool HasLinkingSection = false; wasm::WasmLinkingData LinkingData; diff --git a/lib/MC/WasmObjectWriter.cpp b/lib/MC/WasmObjectWriter.cpp index ab44f8200a6..0fe04f188d7 100644 --- a/lib/MC/WasmObjectWriter.cpp +++ b/lib/MC/WasmObjectWriter.cpp @@ -221,6 +221,7 @@ class WasmObjectWriter : public MCObjectWriter { FunctionTypeIndices; SmallVector FunctionTypes; SmallVector Globals; + unsigned NumFunctionImports = 0; unsigned NumGlobalImports = 0; // TargetObjectWriter wrappers. @@ -252,6 +253,7 @@ private: FunctionTypes.clear(); Globals.clear(); MCObjectWriter::reset(); + NumFunctionImports = 0; NumGlobalImports = 0; } @@ -286,8 +288,7 @@ private: ArrayRef Functions); void writeDataSection(ArrayRef Segments); void writeNameSection(ArrayRef Functions, - ArrayRef Imports, - uint32_t NumFuncImports); + ArrayRef Imports); void writeCodeRelocSection(); void writeDataRelocSection(); void writeLinkingMetaDataSection( @@ -852,11 +853,9 @@ void WasmObjectWriter::writeDataSection(ArrayRef Segments) { endSection(Section); } -void WasmObjectWriter::writeNameSection( - ArrayRef Functions, - ArrayRef Imports, - unsigned NumFuncImports) { - uint32_t TotalFunctions = NumFuncImports + Functions.size(); +void WasmObjectWriter::writeNameSection(ArrayRef Functions, + ArrayRef Imports) { + uint32_t TotalFunctions = NumFunctionImports + Functions.size(); if (TotalFunctions == 0) return; @@ -1023,7 +1022,6 @@ void WasmObjectWriter::writeObject(MCAssembler &Asm, SmallVector, 4> SymbolFlags; SmallVector, 2> InitFuncs; std::map> Comdats; - unsigned NumFuncImports = 0; SmallVector DataSegments; uint32_t DataSize = 0; @@ -1113,8 +1111,7 @@ void WasmObjectWriter::writeObject(MCAssembler &Asm, const auto &WS = static_cast(S); // Register types for all functions, including those with private linkage - // (making them - // because wasm always needs a type signature. + // (because wasm always needs a type signature). if (WS.isFunction()) registerFunctionType(WS); @@ -1131,8 +1128,8 @@ void WasmObjectWriter::writeObject(MCAssembler &Asm, if (WS.isFunction()) { Import.Kind = wasm::WASM_EXTERNAL_FUNCTION; Import.Type = getFunctionType(WS); - SymbolIndices[&WS] = NumFuncImports; - ++NumFuncImports; + SymbolIndices[&WS] = NumFunctionImports; + ++NumFunctionImports; } else { Import.Kind = wasm::WASM_EXTERNAL_GLOBAL; Import.Type = int32_t(PtrType); @@ -1216,7 +1213,7 @@ void WasmObjectWriter::writeObject(MCAssembler &Asm, "function symbols must have a size set with .size"); // A definition. Take the next available index. - Index = NumFuncImports + Functions.size(); + Index = NumFunctionImports + Functions.size(); // Prepare the function. WasmFunction Func; @@ -1410,7 +1407,7 @@ void WasmObjectWriter::writeObject(MCAssembler &Asm, writeElemSection(TableElems); writeCodeSection(Asm, Layout, Functions); writeDataSection(DataSegments); - writeNameSection(Functions, Imports, NumFuncImports); + writeNameSection(Functions, Imports); writeCodeRelocSection(); writeDataRelocSection(); writeLinkingMetaDataSection(DataSegments, DataSize, SymbolFlags, diff --git a/lib/Object/WasmObjectFile.cpp b/lib/Object/WasmObjectFile.cpp index 60c87caca0a..132471ab7f5 100644 --- a/lib/Object/WasmObjectFile.cpp +++ b/lib/Object/WasmObjectFile.cpp @@ -270,6 +270,10 @@ Error WasmObjectFile::parseSection(WasmSection &Sec) { Error WasmObjectFile::parseNameSection(const uint8_t *Ptr, const uint8_t *End) { llvm::DenseSet Seen; + if (Functions.size() != FunctionTypes.size()) { + return make_error("Names must come after code section", + object_error::parse_failed); + } while (Ptr < End) { uint8_t Type = readVarint7(Ptr); @@ -284,10 +288,15 @@ Error WasmObjectFile::parseNameSection(const uint8_t *Ptr, const uint8_t *End) { return make_error("Function named more than once", object_error::parse_failed); StringRef Name = readString(Ptr); - if (!Name.empty()) - Symbols.emplace_back(Name, - WasmSymbol::SymbolType::DEBUG_FUNCTION_NAME, - Sections.size(), Index); + if (!isValidFunctionIndex(Index) || Name.empty()) + return make_error("Invalid name entry", + object_error::parse_failed); + DebugNames.push_back(wasm::WasmFunctionName{Index, Name}); + if (Index >= NumImportedFunctions) { + // Override any existing name; the name specified by the "names" + // section is the Function's canonical name. + Functions[Index - NumImportedFunctions].Name = Name; + } } break; } @@ -360,12 +369,24 @@ void WasmObjectFile::populateSymbolTable() { << " sym index:" << SymIndex << "\n"); } } + if (Export.Kind == wasm::WASM_EXTERNAL_FUNCTION) { + auto &Function = Functions[Export.Index - NumImportedFunctions]; + if (Function.Name.empty()) { + // Use the export's name to set a name for the Function, but only if one + // hasn't already been set. + Function.Name = Export.Name; + } + } } } Error WasmObjectFile::parseLinkingSection(const uint8_t *Ptr, const uint8_t *End) { HasLinkingSection = true; + if (Functions.size() != FunctionTypes.size()) { + return make_error( + "Linking data must come after code section", object_error::parse_failed); + } // Only populate the symbol table with imports and exports if the object // has a linking section (i.e. its a relocatable object file). Otherwise @@ -867,10 +888,6 @@ uint32_t WasmObjectFile::getSymbolFlags(DataRefImpl Symb) const { case WasmSymbol::SymbolType::FUNCTION_EXPORT: Result |= SymbolRef::SF_Executable; break; - case WasmSymbol::SymbolType::DEBUG_FUNCTION_NAME: - Result |= SymbolRef::SF_Executable; - Result |= SymbolRef::SF_FormatSpecific; - break; case WasmSymbol::SymbolType::GLOBAL_IMPORT: Result |= SymbolRef::SF_Undefined; break; @@ -914,7 +931,6 @@ uint64_t WasmObjectFile::getWasmSymbolValue(const WasmSymbol& Sym) const { case WasmSymbol::SymbolType::FUNCTION_IMPORT: case WasmSymbol::SymbolType::GLOBAL_IMPORT: case WasmSymbol::SymbolType::FUNCTION_EXPORT: - case WasmSymbol::SymbolType::DEBUG_FUNCTION_NAME: return Sym.ElementIndex; case WasmSymbol::SymbolType::GLOBAL_EXPORT: { uint32_t GlobalIndex = Sym.ElementIndex - NumImportedGlobals; @@ -949,7 +965,6 @@ WasmObjectFile::getSymbolType(DataRefImpl Symb) const { switch (Sym.Type) { case WasmSymbol::SymbolType::FUNCTION_IMPORT: case WasmSymbol::SymbolType::FUNCTION_EXPORT: - case WasmSymbol::SymbolType::DEBUG_FUNCTION_NAME: return SymbolRef::ST_Function; case WasmSymbol::SymbolType::GLOBAL_IMPORT: case WasmSymbol::SymbolType::GLOBAL_EXPORT: diff --git a/test/MC/WebAssembly/weak-alias.ll b/test/MC/WebAssembly/weak-alias.ll index 83757f48a16..1698913564f 100644 --- a/test/MC/WebAssembly/weak-alias.ll +++ b/test/MC/WebAssembly/weak-alias.ll @@ -239,12 +239,6 @@ entry: ; CHECK-NEXT: ... ; CHECK-SYMS: SYMBOL TABLE: -; CHECK-SYMS-NEXT: 00000000 g F name foo_alias -; CHECK-SYMS-NEXT: 00000001 g F name foo -; CHECK-SYMS-NEXT: 00000002 g F name call_direct -; CHECK-SYMS-NEXT: 00000003 g F name call_alias -; CHECK-SYMS-NEXT: 00000004 g F name call_direct_ptr -; CHECK-SYMS-NEXT: 00000005 g F name call_alias_ptr ; CHECK-SYMS-NEXT: 00000001 gw F EXPORT .hidden foo_alias ; CHECK-SYMS-NEXT: 00000000 gw EXPORT .hidden bar_alias ; CHECK-SYMS-NEXT: 00000001 g F EXPORT .hidden foo diff --git a/test/ObjectYAML/wasm/weak_symbols.yaml b/test/ObjectYAML/wasm/weak_symbols.yaml index c12ef24633a..341146c6c4b 100644 --- a/test/ObjectYAML/wasm/weak_symbols.yaml +++ b/test/ObjectYAML/wasm/weak_symbols.yaml @@ -26,6 +26,14 @@ Sections: - Name: global_export Kind: GLOBAL Index: 0 + - Type: CODE + Functions: + - Index: 0 + Locals: + Body: 00 + - Index: 1 + Locals: + Body: 00 - Type: CUSTOM Name: linking DataSize: 10 diff --git a/test/tools/llvm-nm/wasm/exports.yaml b/test/tools/llvm-nm/wasm/exports.yaml index c9937747a68..06799c45e5e 100644 --- a/test/tools/llvm-nm/wasm/exports.yaml +++ b/test/tools/llvm-nm/wasm/exports.yaml @@ -54,6 +54,23 @@ Sections: - Name: bar Kind: GLOBAL Index: 0x00000003 + - Type: CODE + Functions: + - Index: 1 + Locals: + Body: 00 + - Index: 2 + Locals: + Body: 00 + - Index: 3 + Locals: + Body: 00 + - Index: 4 + Locals: + Body: 00 + - Index: 5 + Locals: + Body: 00 - Type: CUSTOM Name: "linking" DataSize: 0 diff --git a/test/tools/llvm-nm/wasm/weak-symbols.yaml b/test/tools/llvm-nm/wasm/weak-symbols.yaml index 816634f616c..520606532f5 100644 --- a/test/tools/llvm-nm/wasm/weak-symbols.yaml +++ b/test/tools/llvm-nm/wasm/weak-symbols.yaml @@ -54,6 +54,20 @@ Sections: - Name: weak_global_data Kind: GLOBAL Index: 0x00000003 + - Type: CODE + Functions: + - Index: 1 + Locals: + Body: 00 + - Index: 2 + Locals: + Body: 00 + - Index: 3 + Locals: + Body: 00 + - Index: 4 + Locals: + Body: 00 - Type: CUSTOM Name: linking DataSize: 0 diff --git a/test/tools/llvm-objdump/WebAssembly/symbol-table.test b/test/tools/llvm-objdump/WebAssembly/symbol-table.test index 4e46d0d1714..91c227d9d5c 100644 --- a/test/tools/llvm-objdump/WebAssembly/symbol-table.test +++ b/test/tools/llvm-objdump/WebAssembly/symbol-table.test @@ -1,9 +1,6 @@ RUN: llvm-objdump -t %p/../Inputs/trivial.obj.wasm | FileCheck %s CHECK: SYMBOL TABLE: -CHECK-NEXT: 00000000 g F name puts -CHECK-NEXT: 00000001 g F name SomeOtherFunction -CHECK-NEXT: 00000002 g F name main CHECK-NEXT: 00000000 g F IMPORT puts CHECK-NEXT: 00000000 g F IMPORT SomeOtherFunction CHECK-NEXT: 00000002 g F EXPORT main diff --git a/test/tools/llvm-readobj/symbols.test b/test/tools/llvm-readobj/symbols.test index efedd3e6a12..9f1e29f6f31 100644 --- a/test/tools/llvm-readobj/symbols.test +++ b/test/tools/llvm-readobj/symbols.test @@ -74,21 +74,6 @@ ELF-NEXT: } WASM: Symbols [ WASM-NEXT: Symbol { WASM-NEXT: Name: puts -WASM-NEXT: Type: DEBUG_FUNCTION_NAME (0x4) -WASM-NEXT: Flags: 0x0 -WASM-NEXT: } -WASM-NEXT: Symbol { -WASM-NEXT: Name: SomeOtherFunction -WASM-NEXT: Type: DEBUG_FUNCTION_NAME (0x4) -WASM-NEXT: Flags: 0x0 -WASM-NEXT: } -WASM-NEXT: Symbol { -WASM-NEXT: Name: main -WASM-NEXT: Type: DEBUG_FUNCTION_NAME (0x4) -WASM-NEXT: Flags: 0x0 -WASM-NEXT: } -WASM-NEXT: Symbol { -WASM-NEXT: Name: puts WASM-NEXT: Type: FUNCTION_IMPORT (0x0) WASM-NEXT: Flags: 0x0 WASM-NEXT: } diff --git a/tools/llvm-readobj/WasmDumper.cpp b/tools/llvm-readobj/WasmDumper.cpp index 223c1c75246..738b5b5e5cc 100644 --- a/tools/llvm-readobj/WasmDumper.cpp +++ b/tools/llvm-readobj/WasmDumper.cpp @@ -28,7 +28,6 @@ static const EnumEntry WasmSymbolTypes[] = { ENUM_ENTRY(FUNCTION_EXPORT), ENUM_ENTRY(GLOBAL_IMPORT), ENUM_ENTRY(GLOBAL_EXPORT), - ENUM_ENTRY(DEBUG_FUNCTION_NAME), #undef ENUM_ENTRY }; diff --git a/tools/obj2yaml/wasm2yaml.cpp b/tools/obj2yaml/wasm2yaml.cpp index e3577d4ef4f..7ec344a35ba 100644 --- a/tools/obj2yaml/wasm2yaml.cpp +++ b/tools/obj2yaml/wasm2yaml.cpp @@ -53,13 +53,10 @@ std::unique_ptr WasmDumper::dumpCustomSection(const Was std::unique_ptr CustomSec; if (WasmSec.Name == "name") { std::unique_ptr NameSec = make_unique(); - for (const object::SymbolRef& Sym: Obj.symbols()) { - const object::WasmSymbol Symbol = Obj.getWasmSymbol(Sym); - if (Symbol.Type != object::WasmSymbol::SymbolType::DEBUG_FUNCTION_NAME) - continue; + for (const llvm::wasm::WasmFunctionName &Func: Obj.debugNames()) { WasmYAML::NameEntry NameEntry; - NameEntry.Name = Symbol.Name; - NameEntry.Index = Sym.getValue(); + NameEntry.Name = Func.Name; + NameEntry.Index = Func.Index; NameSec->FunctionNames.push_back(NameEntry); } CustomSec = std::move(NameSec);