From e638d8b2bc27d620ea26fcc730f7cba29ab82349 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Wed, 3 Mar 2021 11:13:25 +0100 Subject: [PATCH] [lld][WebAssembly] -Bsymbolic creates indirect function table if needed It can be that while processing relocations, we realize that in the end we need an indirect function table. Ensure that one is present, in that case, to avoid writing invalid object files. Fixes https://bugs.llvm.org/show_bug.cgi?id=49397. Differential Revision: https://reviews.llvm.org/D97843 --- lld/test/wasm/bsymbolic.s | 66 +++++++++++++++++++++++++++++++++++------- lld/wasm/Driver.cpp | 47 ++---------------------------- lld/wasm/SymbolTable.cpp | 65 +++++++++++++++++++++++++++++++++++++++++ lld/wasm/SymbolTable.h | 5 ++++ lld/wasm/SyntheticSections.cpp | 10 ++++++- lld/wasm/Writer.cpp | 9 ++++++ 6 files changed, 146 insertions(+), 56 deletions(-) diff --git a/lld/test/wasm/bsymbolic.s b/lld/test/wasm/bsymbolic.s index 07989fc..bc2e4e7 100644 --- a/lld/test/wasm/bsymbolic.s +++ b/lld/test/wasm/bsymbolic.s @@ -8,17 +8,34 @@ // RUN: wasm-ld --experimental-pic -shared -Bsymbolic %t.o -o %t1.so // RUN: obj2yaml %t1.so | FileCheck -check-prefix=SYMBOLIC %s -// NOOPTION - Type: IMPORT -// NOOPTION: - Module: GOT.func -// NOOPTION-NEXT: Field: foo -// NOOPTION-NEXT: Kind: GLOBAL -// NOOPTION-NEXT: GlobalType: I32 -// NOOPTION-NEXT: GlobalMutable: true -// NOOPTION-NEXT: - Module: GOT.mem -// NOOPTION-NEXT: Field: bar -// NOOPTION-NEXT: Kind: GLOBAL -// NOOPTION-NEXT: GlobalType: I32 -// NOOPTION-NEXT: GlobalMutable: true +// NOOPTION: - Type: IMPORT +// NOOPTION-NEXT: Imports: +// NOOPTION-NEXT: - Module: env +// NOOPTION-NEXT: Field: memory +// NOOPTION-NEXT: Kind: MEMORY +// NOOPTION-NEXT: Memory: +// NOOPTION-NEXT: Initial: 0x1 +// NOOPTION-NEXT: - Module: env +// NOOPTION-NEXT: Field: __memory_base +// NOOPTION-NEXT: Kind: GLOBAL +// NOOPTION-NEXT: GlobalType: I32 +// NOOPTION-NEXT: GlobalMutable: false +// NOOPTION-NEXT: - Module: env +// NOOPTION-NEXT: Field: __table_base +// NOOPTION-NEXT: Kind: GLOBAL +// NOOPTION-NEXT: GlobalType: I32 +// NOOPTION-NEXT: GlobalMutable: false +// NOOPTION-NEXT: - Module: GOT.func +// NOOPTION-NEXT: Field: foo +// NOOPTION-NEXT: Kind: GLOBAL +// NOOPTION-NEXT: GlobalType: I32 +// NOOPTION-NEXT: GlobalMutable: true +// NOOPTION-NEXT: - Module: GOT.mem +// NOOPTION-NEXT: Field: bar +// NOOPTION-NEXT: Kind: GLOBAL +// NOOPTION-NEXT: GlobalType: I32 +// NOOPTION-NEXT: GlobalMutable: true +// NOOPTION-NEXT: - Type: FUNCTION // NOOPTION: - Type: GLOBAL // NOOPTION-NEXT: Globals: @@ -33,6 +50,33 @@ // SYMBOLIC-NOT: - Module: GOT.mem // SYMBOLIC-NOT: - Module: GOT.func +// SYMBOLIC: - Type: IMPORT +// SYMBOLIC-NEXT: Imports: +// SYMBOLIC-NEXT: - Module: env +// SYMBOLIC-NEXT: Field: memory +// SYMBOLIC-NEXT: Kind: MEMORY +// SYMBOLIC-NEXT: Memory: +// SYMBOLIC-NEXT: Initial: 0x1 +// SYMBOLIC-NEXT: - Module: env +// SYMBOLIC-NEXT: Field: __memory_base +// SYMBOLIC-NEXT: Kind: GLOBAL +// SYMBOLIC-NEXT: GlobalType: I32 +// SYMBOLIC-NEXT: GlobalMutable: false +// SYMBOLIC-NEXT: - Module: env +// SYMBOLIC-NEXT: Field: __table_base +// SYMBOLIC-NEXT: Kind: GLOBAL +// SYMBOLIC-NEXT: GlobalType: I32 +// SYMBOLIC-NEXT: GlobalMutable: false +// SYMBOLIC-NEXT: - Module: env +// SYMBOLIC-NEXT: Field: __indirect_function_table +// SYMBOLIC-NEXT: Kind: TABLE +// SYMBOLIC-NEXT: Table: +// SYMBOLIC-NEXT: Index: 0 +// SYMBOLIC-NEXT: ElemType: FUNCREF +// SYMBOLIC-NEXT: Limits: +// SYMBOLIC-NEXT: Initial: 0x1 +// SYMBOLIC-NEXT: - Type: FUNCTION + // SYMBOLIC: - Type: GLOBAL // SYMBOLIC-NEXT: Globals: // SYMBOLIC-NEXT: - Index: 2 diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp index 7216e0a..8e14546 100644 --- a/lld/wasm/Driver.cpp +++ b/lld/wasm/Driver.cpp @@ -795,48 +795,6 @@ static void wrapSymbols(ArrayRef wrapped) { symtab->wrap(w.sym, w.real, w.wrap); } -static TableSymbol *createDefinedIndirectFunctionTable(StringRef name) { - const uint32_t invalidIndex = -1; - WasmLimits limits{0, 0, 0}; // Set by the writer. - WasmTableType type{uint8_t(ValType::FUNCREF), limits}; - WasmTable desc{invalidIndex, type, name}; - InputTable *table = make(desc, nullptr); - uint32_t flags = config->exportTable ? 0 : WASM_SYMBOL_VISIBILITY_HIDDEN; - TableSymbol *sym = symtab->addSyntheticTable(name, flags, table); - sym->markLive(); - sym->forceExport = config->exportTable; - return sym; -} - -static TableSymbol *resolveIndirectFunctionTable() { - Symbol *existing = symtab->find(functionTableName); - if (existing) { - if (!isa(existing)) { - error(Twine("reserved symbol must be of type table: `") + - functionTableName + "`"); - return nullptr; - } - if (existing->isDefined()) { - error(Twine("reserved symbol must not be defined in input files: `") + - functionTableName + "`"); - return nullptr; - } - } - - if (config->importTable) { - return cast_or_null(existing); - } else if ((existing && existing->isLive()) || config->exportTable) { - // A defined table is required. Either because the user request an exported - // table or because the table symbol is already live. The existing table is - // guaranteed to be undefined due to the check above. - return createDefinedIndirectFunctionTable(functionTableName); - } - - // An indirect function table will only be present in the symbol table if - // needed by a reloc; if we get here, we don't need one. - return nullptr; -} - void LinkerDriver::linkerMain(ArrayRef argsArr) { WasmOptTable parser; opt::InputArgList args = parser.parse(argsArr.slice(1)); @@ -1021,8 +979,9 @@ void LinkerDriver::linkerMain(ArrayRef argsArr) { // Do size optimizations: garbage collection markLive(); - // Provide the indirect funciton table if needed. - WasmSym::indirectFunctionTable = resolveIndirectFunctionTable(); + // Provide the indirect function table if needed. + WasmSym::indirectFunctionTable = + symtab->resolveIndirectFunctionTable(/*required =*/false); if (errorCount()) return; diff --git a/lld/wasm/SymbolTable.cpp b/lld/wasm/SymbolTable.cpp index 712f92f..56438c7 100644 --- a/lld/wasm/SymbolTable.cpp +++ b/lld/wasm/SymbolTable.cpp @@ -628,6 +628,71 @@ Symbol *SymbolTable::addUndefinedTable(StringRef name, return s; } +TableSymbol *SymbolTable::createUndefinedIndirectFunctionTable(StringRef name) { + WasmLimits limits{0, 0, 0}; // Set by the writer. + WasmTableType *type = make(); + type->ElemType = uint8_t(ValType::FUNCREF); + type->Limits = limits; + StringRef module(defaultModule); + uint32_t flags = config->exportTable ? 0 : WASM_SYMBOL_VISIBILITY_HIDDEN; + flags |= WASM_SYMBOL_UNDEFINED; + Symbol *sym = addUndefinedTable(name, name, module, flags, nullptr, type); + sym->markLive(); + sym->forceExport = config->exportTable; + return cast(sym); +} + +TableSymbol *SymbolTable::createDefinedIndirectFunctionTable(StringRef name) { + const uint32_t invalidIndex = -1; + WasmLimits limits{0, 0, 0}; // Set by the writer. + WasmTableType type{uint8_t(ValType::FUNCREF), limits}; + WasmTable desc{invalidIndex, type, name}; + InputTable *table = make(desc, nullptr); + uint32_t flags = config->exportTable ? 0 : WASM_SYMBOL_VISIBILITY_HIDDEN; + TableSymbol *sym = addSyntheticTable(name, flags, table); + sym->markLive(); + sym->forceExport = config->exportTable; + return sym; +} + +// Whether or not we need an indirect function table is usually a function of +// whether an input declares a need for it. However sometimes it's possible for +// no input to need the indirect function table, but then a late +// addInternalGOTEntry causes a function to be allocated an address. In that +// case address we synthesize a definition at the last minute. +TableSymbol *SymbolTable::resolveIndirectFunctionTable(bool required) { + Symbol *existing = find(functionTableName); + if (existing) { + if (!isa(existing)) { + error(Twine("reserved symbol must be of type table: `") + + functionTableName + "`"); + return nullptr; + } + if (existing->isDefined()) { + error(Twine("reserved symbol must not be defined in input files: `") + + functionTableName + "`"); + return nullptr; + } + } + + if (config->importTable) { + if (existing) + return cast(existing); + if (required) + return createUndefinedIndirectFunctionTable(functionTableName); + } else if ((existing && existing->isLive()) || config->exportTable || + required) { + // A defined table is required. Either because the user request an exported + // table or because the table symbol is already live. The existing table is + // guaranteed to be undefined due to the check above. + return createDefinedIndirectFunctionTable(functionTableName); + } + + // An indirect function table will only be present in the symbol table if + // needed by a reloc; if we get here, we don't need one. + return nullptr; +} + void SymbolTable::addLazy(ArchiveFile *file, const Archive::Symbol *sym) { LLVM_DEBUG(dbgs() << "addLazy: " << sym->getName() << "\n"); StringRef name = sym->getName(); diff --git a/lld/wasm/SymbolTable.h b/lld/wasm/SymbolTable.h index f3510f8..36c776b 100644 --- a/lld/wasm/SymbolTable.h +++ b/lld/wasm/SymbolTable.h @@ -81,6 +81,8 @@ public: uint32_t flags, InputFile *file, const WasmTableType *type); + TableSymbol *resolveIndirectFunctionTable(bool required); + void addLazy(ArchiveFile *f, const llvm::object::Archive::Symbol *sym); bool addComdat(StringRef name); @@ -116,6 +118,9 @@ private: StringRef debugName); void replaceWithUndefined(Symbol *sym); + TableSymbol *createDefinedIndirectFunctionTable(StringRef name); + TableSymbol *createUndefinedIndirectFunctionTable(StringRef name); + // Maps symbol names to index into the symVector. -1 means that symbols // is to not yet in the vector but it should have tracing enabled if it is // ever added. diff --git a/lld/wasm/SyntheticSections.cpp b/lld/wasm/SyntheticSections.cpp index f4da2ce..dc38550 100644 --- a/lld/wasm/SyntheticSections.cpp +++ b/lld/wasm/SyntheticSections.cpp @@ -296,6 +296,12 @@ void GlobalSection::assignIndexes() { isSealed = true; } +static void ensureIndirectFunctionTable() { + if (!WasmSym::indirectFunctionTable) + WasmSym::indirectFunctionTable = + symtab->resolveIndirectFunctionTable(/*required =*/true); +} + void GlobalSection::addInternalGOTEntry(Symbol *sym) { assert(!isSealed); if (sym->requiresGOT) @@ -303,8 +309,10 @@ void GlobalSection::addInternalGOTEntry(Symbol *sym) { LLVM_DEBUG(dbgs() << "addInternalGOTEntry: " << sym->getName() << " " << toString(sym->kind()) << "\n"); sym->requiresGOT = true; - if (auto *F = dyn_cast(sym)) + if (auto *F = dyn_cast(sym)) { + ensureIndirectFunctionTable(); out.elemSec->addEntry(F); + } internalGotSymbols.push_back(sym); } diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp index a1bd142..63e3dac 100644 --- a/lld/wasm/Writer.cpp +++ b/lld/wasm/Writer.cpp @@ -739,6 +739,15 @@ static void finalizeIndirectFunctionTable() { if (!WasmSym::indirectFunctionTable) return; + if (shouldImport(WasmSym::indirectFunctionTable) && + !WasmSym::indirectFunctionTable->hasTableNumber()) { + // Processing -Bsymbolic relocations resulted in a late requirement that the + // indirect function table be present, and we are running in --import-table + // mode. Add the table now to the imports section. Otherwise it will be + // added to the tables section later in assignIndexes. + out.importSec->addImport(WasmSym::indirectFunctionTable); + } + uint32_t tableSize = config->tableBase + out.elemSec->numEntries(); WasmLimits limits = {0, tableSize, 0}; if (WasmSym::indirectFunctionTable->isDefined() && !config->growableTable) { -- 2.7.4