[lld] e638d8b - [lld][WebAssembly] -Bsymbolic creates indirect function table if needed

Andy Wingo via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 4 00:30:42 PST 2021


Author: Andy Wingo
Date: 2021-03-04T09:28:21+01:00
New Revision: e638d8b2bc27d620ea26fcc730f7cba29ab82349

URL: https://github.com/llvm/llvm-project/commit/e638d8b2bc27d620ea26fcc730f7cba29ab82349
DIFF: https://github.com/llvm/llvm-project/commit/e638d8b2bc27d620ea26fcc730f7cba29ab82349.diff

LOG: [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

Added: 
    

Modified: 
    lld/test/wasm/bsymbolic.s
    lld/wasm/Driver.cpp
    lld/wasm/SymbolTable.cpp
    lld/wasm/SymbolTable.h
    lld/wasm/SyntheticSections.cpp
    lld/wasm/Writer.cpp

Removed: 
    


################################################################################
diff  --git a/lld/test/wasm/bsymbolic.s b/lld/test/wasm/bsymbolic.s
index 07989fc5f437c..bc2e4e7774069 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 7216e0a5df914..8e145467e43aa 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -795,48 +795,6 @@ static void wrapSymbols(ArrayRef<WrappedSymbol> 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<InputTable>(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<TableSymbol>(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<TableSymbol>(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<const char *> argsArr) {
   WasmOptTable parser;
   opt::InputArgList args = parser.parse(argsArr.slice(1));
@@ -1021,8 +979,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> 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 712f92fca9c72..56438c7b5d998 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<WasmTableType>();
+  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<TableSymbol>(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<InputTable>(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<TableSymbol>(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<TableSymbol>(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 f3510f82fc8a0..36c776be30f59 100644
--- a/lld/wasm/SymbolTable.h
+++ b/lld/wasm/SymbolTable.h
@@ -81,6 +81,8 @@ class SymbolTable {
                             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 @@ class SymbolTable {
                                         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 f4da2ce48c1b3..dc38550d47fa8 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<FunctionSymbol>(sym))
+  if (auto *F = dyn_cast<FunctionSymbol>(sym)) {
+    ensureIndirectFunctionTable();
     out.elemSec->addEntry(F);
+  }
   internalGotSymbols.push_back(sym);
 }
 

diff  --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index a1bd142ce83a1..63e3dac5d2985 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) {


        


More information about the llvm-commits mailing list