From 29e818765e6e2ea6ed0a14ef19cfd278dd631e9d Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Wed, 31 Jan 2024 15:49:22 -0800 Subject: [PATCH 1/5] [Object][Wasm] Move WasmSymbolInfo directly into WasmSymbol (NFC) --- lld/wasm/InputFiles.cpp | 20 ++++++++++---------- llvm/include/llvm/BinaryFormat/Wasm.h | 1 - llvm/include/llvm/Object/Wasm.h | 4 ++-- llvm/lib/Object/WasmObjectFile.cpp | 9 ++------- llvm/tools/obj2yaml/wasm2yaml.cpp | 3 ++- 5 files changed, 16 insertions(+), 21 deletions(-) diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp index f43c39b218787..394ca98d1265a 100644 --- a/lld/wasm/InputFiles.cpp +++ b/lld/wasm/InputFiles.cpp @@ -322,20 +322,20 @@ void ObjFile::addLegacyIndirectFunctionTableIfNeeded( return; } - auto *info = make(); - info->Name = tableImport->Field; - info->Kind = WASM_SYMBOL_TYPE_TABLE; - info->ImportModule = tableImport->Module; - info->ImportName = tableImport->Field; - info->Flags = WASM_SYMBOL_UNDEFINED; - info->Flags |= WASM_SYMBOL_NO_STRIP; - info->ElementIndex = 0; - LLVM_DEBUG(dbgs() << "Synthesizing symbol for table import: " << info->Name + WasmSymbolInfo info; + info.Name = tableImport->Field; + info.Kind = WASM_SYMBOL_TYPE_TABLE; + info.ImportModule = tableImport->Module; + info.ImportName = tableImport->Field; + info.Flags = WASM_SYMBOL_UNDEFINED; + info.Flags |= WASM_SYMBOL_NO_STRIP; + info.ElementIndex = 0; + LLVM_DEBUG(dbgs() << "Synthesizing symbol for table import: " << info.Name << "\n"); const WasmGlobalType *globalType = nullptr; const WasmSignature *signature = nullptr; auto *wasmSym = - make(*info, globalType, &tableImport->Table, signature); + make(std::move(info), globalType, &tableImport->Table, signature); Symbol *sym = createUndefined(*wasmSym, false); // We're only sure it's a TableSymbol if the createUndefined succeeded. if (errorCount()) diff --git a/llvm/include/llvm/BinaryFormat/Wasm.h b/llvm/include/llvm/BinaryFormat/Wasm.h index cd13b7014bfe2..9c1cf68d939a9 100644 --- a/llvm/include/llvm/BinaryFormat/Wasm.h +++ b/llvm/include/llvm/BinaryFormat/Wasm.h @@ -471,7 +471,6 @@ struct WasmLinkingData { uint32_t Version; std::vector InitFunctions; std::vector Comdats; - std::vector SymbolTable; }; struct WasmSignature { diff --git a/llvm/include/llvm/Object/Wasm.h b/llvm/include/llvm/Object/Wasm.h index 927dce882f6ae..1a222e58ef906 100644 --- a/llvm/include/llvm/Object/Wasm.h +++ b/llvm/include/llvm/Object/Wasm.h @@ -34,7 +34,7 @@ namespace object { class WasmSymbol { public: - WasmSymbol(const wasm::WasmSymbolInfo &Info, + WasmSymbol(const wasm::WasmSymbolInfo &&Info, const wasm::WasmGlobalType *GlobalType, const wasm::WasmTableType *TableType, const wasm::WasmSignature *Signature) @@ -43,7 +43,7 @@ class WasmSymbol { assert(!Signature || Signature->Kind != wasm::WasmSignature::Placeholder); } - const wasm::WasmSymbolInfo &Info; + const wasm::WasmSymbolInfo Info; const wasm::WasmGlobalType *GlobalType; const wasm::WasmTableType *TableType; const wasm::WasmSignature *Signature; diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp index 953e7c70b5f6e..636ec484e2d05 100644 --- a/llvm/lib/Object/WasmObjectFile.cpp +++ b/llvm/lib/Object/WasmObjectFile.cpp @@ -642,9 +642,7 @@ Error WasmObjectFile::parseLinkingSectionSymtab(ReadContext &Ctx) { uint32_t Count = readVaruint32(Ctx); // Clear out any symbol information that was derived from the exports // section. - LinkingData.SymbolTable.clear(); Symbols.clear(); - LinkingData.SymbolTable.reserve(Count); Symbols.reserve(Count); StringSet<> SymbolNames; @@ -844,8 +842,7 @@ Error WasmObjectFile::parseLinkingSectionSymtab(ReadContext &Ctx) { return make_error("duplicate symbol name " + Twine(Info.Name), object_error::parse_failed); - LinkingData.SymbolTable.emplace_back(Info); - Symbols.emplace_back(LinkingData.SymbolTable.back(), GlobalType, TableType, + Symbols.emplace_back(std::move(Info), GlobalType, TableType, Signature); LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n"); } @@ -1390,7 +1387,6 @@ Error WasmObjectFile::parseGlobalSection(ReadContext &Ctx) { Error WasmObjectFile::parseExportSection(ReadContext &Ctx) { uint32_t Count = readVaruint32(Ctx); Exports.reserve(Count); - LinkingData.SymbolTable.reserve(Count); Symbols.reserve(Count); for (uint32_t I = 0; I < Count; I++) { wasm::WasmExport Ex; @@ -1455,8 +1451,7 @@ Error WasmObjectFile::parseExportSection(ReadContext &Ctx) { } Exports.push_back(Ex); if (Ex.Kind != wasm::WASM_EXTERNAL_MEMORY) { - LinkingData.SymbolTable.emplace_back(Info); - Symbols.emplace_back(LinkingData.SymbolTable.back(), GlobalType, + Symbols.emplace_back(std::move(Info), GlobalType, TableType, Signature); LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n"); } diff --git a/llvm/tools/obj2yaml/wasm2yaml.cpp b/llvm/tools/obj2yaml/wasm2yaml.cpp index c15d64cef8d65..b2679fb28857f 100644 --- a/llvm/tools/obj2yaml/wasm2yaml.cpp +++ b/llvm/tools/obj2yaml/wasm2yaml.cpp @@ -124,7 +124,8 @@ WasmDumper::dumpCustomSection(const WasmSection &WasmSec) { } uint32_t SymbolIndex = 0; - for (const wasm::WasmSymbolInfo &Symbol : Obj.linkingData().SymbolTable) { + for (const auto &Sym : Obj.symbols()) { + const wasm::WasmSymbolInfo& Symbol = Obj.getWasmSymbol(Sym).Info; WasmYAML::SymbolInfo Info; Info.Index = SymbolIndex++; Info.Kind = static_cast(Symbol.Kind); From d6e17c91fa9d9199a44370416a8fd642edb9caaf Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Thu, 1 Feb 2024 14:28:01 -0800 Subject: [PATCH 2/5] add comments --- llvm/include/llvm/BinaryFormat/Wasm.h | 5 +++++ llvm/include/llvm/Object/Wasm.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/llvm/include/llvm/BinaryFormat/Wasm.h b/llvm/include/llvm/BinaryFormat/Wasm.h index 9c1cf68d939a9..aec6ea0b75779 100644 --- a/llvm/include/llvm/BinaryFormat/Wasm.h +++ b/llvm/include/llvm/BinaryFormat/Wasm.h @@ -467,10 +467,15 @@ struct WasmDebugName { StringRef Name; }; +// Info from the linking metadata section of a wasm object file. struct WasmLinkingData { uint32_t Version; std::vector InitFunctions; std::vector Comdats; + // The linking section also contains a symbol table. This info (represented + // in a WasmSymbolInfo struct) is stored inside the WasmSymbol object instead + // of in this structure; this allows vectors of WasmSymbols and + // WasmLinkingDatas to be reallocated. }; struct WasmSignature { diff --git a/llvm/include/llvm/Object/Wasm.h b/llvm/include/llvm/Object/Wasm.h index 1a222e58ef906..95e42a22f3b78 100644 --- a/llvm/include/llvm/Object/Wasm.h +++ b/llvm/include/llvm/Object/Wasm.h @@ -43,6 +43,8 @@ class WasmSymbol { assert(!Signature || Signature->Kind != wasm::WasmSignature::Placeholder); } + // Symbol info as represented in the symbol's 'syminfo' entry of an object + // file's symbol table. const wasm::WasmSymbolInfo Info; const wasm::WasmGlobalType *GlobalType; const wasm::WasmTableType *TableType; From a45ec69bbb7c6835891f812ad11c5d0b29216e6e Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Thu, 1 Feb 2024 15:07:54 -0800 Subject: [PATCH 3/5] clean up --- lld/wasm/InputFiles.cpp | 2 +- llvm/include/llvm/Object/Wasm.h | 4 ++-- llvm/lib/Object/WasmObjectFile.cpp | 4 ++-- llvm/tools/obj2yaml/wasm2yaml.cpp | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp index 394ca98d1265a..60ee60d9a3aa9 100644 --- a/lld/wasm/InputFiles.cpp +++ b/lld/wasm/InputFiles.cpp @@ -335,7 +335,7 @@ void ObjFile::addLegacyIndirectFunctionTableIfNeeded( const WasmGlobalType *globalType = nullptr; const WasmSignature *signature = nullptr; auto *wasmSym = - make(std::move(info), globalType, &tableImport->Table, signature); + make(info, globalType, &tableImport->Table, signature); Symbol *sym = createUndefined(*wasmSym, false); // We're only sure it's a TableSymbol if the createUndefined succeeded. if (errorCount()) diff --git a/llvm/include/llvm/Object/Wasm.h b/llvm/include/llvm/Object/Wasm.h index 95e42a22f3b78..13d9a17e24c3d 100644 --- a/llvm/include/llvm/Object/Wasm.h +++ b/llvm/include/llvm/Object/Wasm.h @@ -34,7 +34,7 @@ namespace object { class WasmSymbol { public: - WasmSymbol(const wasm::WasmSymbolInfo &&Info, + WasmSymbol(const wasm::WasmSymbolInfo &Info, const wasm::WasmGlobalType *GlobalType, const wasm::WasmTableType *TableType, const wasm::WasmSignature *Signature) @@ -45,7 +45,7 @@ class WasmSymbol { // Symbol info as represented in the symbol's 'syminfo' entry of an object // file's symbol table. - const wasm::WasmSymbolInfo Info; + wasm::WasmSymbolInfo Info; const wasm::WasmGlobalType *GlobalType; const wasm::WasmTableType *TableType; const wasm::WasmSignature *Signature; diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp index 636ec484e2d05..7e10bd8071aee 100644 --- a/llvm/lib/Object/WasmObjectFile.cpp +++ b/llvm/lib/Object/WasmObjectFile.cpp @@ -842,7 +842,7 @@ Error WasmObjectFile::parseLinkingSectionSymtab(ReadContext &Ctx) { return make_error("duplicate symbol name " + Twine(Info.Name), object_error::parse_failed); - Symbols.emplace_back(std::move(Info), GlobalType, TableType, + Symbols.emplace_back(Info, GlobalType, TableType, Signature); LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n"); } @@ -1451,7 +1451,7 @@ Error WasmObjectFile::parseExportSection(ReadContext &Ctx) { } Exports.push_back(Ex); if (Ex.Kind != wasm::WASM_EXTERNAL_MEMORY) { - Symbols.emplace_back(std::move(Info), GlobalType, + Symbols.emplace_back(Info, GlobalType, TableType, Signature); LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n"); } diff --git a/llvm/tools/obj2yaml/wasm2yaml.cpp b/llvm/tools/obj2yaml/wasm2yaml.cpp index b2679fb28857f..397b1fd0ab81f 100644 --- a/llvm/tools/obj2yaml/wasm2yaml.cpp +++ b/llvm/tools/obj2yaml/wasm2yaml.cpp @@ -124,7 +124,7 @@ WasmDumper::dumpCustomSection(const WasmSection &WasmSec) { } uint32_t SymbolIndex = 0; - for (const auto &Sym : Obj.symbols()) { + for (const object::SymbolRef &Sym : Obj.symbols()) { const wasm::WasmSymbolInfo& Symbol = Obj.getWasmSymbol(Sym).Info; WasmYAML::SymbolInfo Info; Info.Index = SymbolIndex++; From b2d746946e4b1d053d183da0242e5fa56b7c1503 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Thu, 1 Feb 2024 15:12:21 -0800 Subject: [PATCH 4/5] clang-format --- llvm/lib/Object/WasmObjectFile.cpp | 6 ++---- llvm/tools/obj2yaml/wasm2yaml.cpp | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp index 7e10bd8071aee..4778562779bb7 100644 --- a/llvm/lib/Object/WasmObjectFile.cpp +++ b/llvm/lib/Object/WasmObjectFile.cpp @@ -842,8 +842,7 @@ Error WasmObjectFile::parseLinkingSectionSymtab(ReadContext &Ctx) { return make_error("duplicate symbol name " + Twine(Info.Name), object_error::parse_failed); - Symbols.emplace_back(Info, GlobalType, TableType, - Signature); + Symbols.emplace_back(Info, GlobalType, TableType, Signature); LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n"); } @@ -1451,8 +1450,7 @@ Error WasmObjectFile::parseExportSection(ReadContext &Ctx) { } Exports.push_back(Ex); if (Ex.Kind != wasm::WASM_EXTERNAL_MEMORY) { - Symbols.emplace_back(Info, GlobalType, - TableType, Signature); + Symbols.emplace_back(Info, GlobalType, TableType, Signature); LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n"); } } diff --git a/llvm/tools/obj2yaml/wasm2yaml.cpp b/llvm/tools/obj2yaml/wasm2yaml.cpp index 397b1fd0ab81f..9aa7f5abe8dd4 100644 --- a/llvm/tools/obj2yaml/wasm2yaml.cpp +++ b/llvm/tools/obj2yaml/wasm2yaml.cpp @@ -125,7 +125,7 @@ WasmDumper::dumpCustomSection(const WasmSection &WasmSec) { uint32_t SymbolIndex = 0; for (const object::SymbolRef &Sym : Obj.symbols()) { - const wasm::WasmSymbolInfo& Symbol = Obj.getWasmSymbol(Sym).Info; + const wasm::WasmSymbolInfo &Symbol = Obj.getWasmSymbol(Sym).Info; WasmYAML::SymbolInfo Info; Info.Index = SymbolIndex++; Info.Kind = static_cast(Symbol.Kind); From a5d53a1b100068e4f3253f775db7b3085f29ff34 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Fri, 2 Feb 2024 10:40:55 -0800 Subject: [PATCH 5/5] review comment --- lld/wasm/InputFiles.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp index 60ee60d9a3aa9..473208a08a812 100644 --- a/lld/wasm/InputFiles.cpp +++ b/lld/wasm/InputFiles.cpp @@ -15,6 +15,7 @@ #include "lld/Common/Args.h" #include "lld/Common/CommonLinkerContext.h" #include "lld/Common/Reproduce.h" +#include "llvm/BinaryFormat/Wasm.h" #include "llvm/Object/Binary.h" #include "llvm/Object/Wasm.h" #include "llvm/Support/Path.h" @@ -327,8 +328,7 @@ void ObjFile::addLegacyIndirectFunctionTableIfNeeded( info.Kind = WASM_SYMBOL_TYPE_TABLE; info.ImportModule = tableImport->Module; info.ImportName = tableImport->Field; - info.Flags = WASM_SYMBOL_UNDEFINED; - info.Flags |= WASM_SYMBOL_NO_STRIP; + info.Flags = WASM_SYMBOL_UNDEFINED | WASM_SYMBOL_NO_STRIP; info.ElementIndex = 0; LLVM_DEBUG(dbgs() << "Synthesizing symbol for table import: " << info.Name << "\n");