[lld][WebAssembly] Don't define indirect function table in relocatable output

[sbc100]

Summary

Object files (and the output --relocatable) should never define
__indirect_function_table. It should always be linker synthesized
with the final output executable.

[sbc100]

I verified this fixes the crash we are seeing on the emscripten CI with the use of -r.

[dschuff]

lld/test/wasm/locals-duplicate.test

241

is this elemtype correct? do we just not distinguish right now?

[dschuff]

lld/test/wasm/locals-duplicate.test

241

Looks like the answer is that we only have FUNCREF and EXTERNREF right now. Maybe doesn't matter for this case, I guess that's something we can fix in the future.

[sbc100]

lld/test/wasm/locals-duplicate.test

241

This is correct I think yes. This is the only possible elem type today I think. What else would you expect it to say here?

[dschuff]

lld/test/wasm/locals-duplicate.test

241

Actually nevermind, I was confused: this is the type of the element, not the type of the imported ref. So this is right.

[wingo]

Thank you for the patch and apologies for messing this up!

lld/test/wasm/locals-duplicate.test

 ; RELOC-NEXT:   Version:         0x1
 ; RELOC-NEXT: Sections:
 ; RELOC-NEXT:   - Type:            TYPE
 ; RELOC-NEXT:     Signatures:
 ; RELOC-NEXT:       - Index:           0
 ; RELOC-NEXT:         ParamTypes:      []
 ; RELOC-NEXT:         ReturnTypes:
 ; RELOC-NEXT:           - I32
+; RELOC-NEXT:  - Type:            IMPORT
+; RELOC-NEXT:    Imports:
+; RELOC-NEXT:      - Module:          env
+; RELOC-NEXT:        Field:           __indirect_function_table
+; RELOC-NEXT:        Kind:            TABLE
+; RELOC-NEXT:        Table:
+; RELOC-NEXT:          Index:           0
+; RELOC-NEXT:          ElemType:        FUNCREF
+; RELOC-NEXT:          Limits:
+; RELOC-NEXT:            Initial:         0x3
 ; RELOC-NEXT:   - Type:            FUNCTION
 ; RELOC-NEXT:     FunctionTypes:   [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 ; RELOC-NEXT:                        0, 0 ]
-; RELOC-NEXT:   - Type:            TABLE
-; RELOC-NEXT:     Tables:
-; RELOC-NEXT:       - Index:           0
-; RELOC-NEXT:         ElemType:        FUNCREF
-; RELOC-NEXT:         Limits:
-; RELOC-NEXT:           Flags:           [ HAS_MAX ]
-; RELOC-NEXT:           Initial:         0x7
-; RELOC-NEXT:           Maximum:         0x7
 ; RELOC-NEXT:   - Type:            MEMORY
 ; RELOC-NEXT:     Memories:
 ; RELOC-NEXT:       - Initial:         0x1
 ; RELOC-NEXT:   - Type:            ELEM
 ; RELOC-NEXT:     Segments:
 ; RELOC-NEXT:       - Offset:
 ; RELOC-NEXT:           Opcode:          I32_CONST
 ; RELOC-NEXT:           Value:           1

 ; RELOC-NEXT:       - Index:           7
 ; RELOC-NEXT:         Kind:            FUNCTION
 ; RELOC-NEXT:         Name:            get_func3A
 ; RELOC-NEXT:         Flags:           [  ]
 ; RELOC-NEXT:         Function:        8
 ; RELOC-NEXT:       - Index:           8
 ; RELOC-NEXT:         Kind:            TABLE
 ; RELOC-NEXT:         Name:            __indirect_function_table
-; RELOC-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
+; RELOC-NEXT:         Flags:           [ UNDEFINED, NO_STRIP ]
 ; RELOC-NEXT:         Table:           0
 ; RELOC-NEXT:       - Index:           9
 ; RELOC-NEXT:         Kind:            FUNCTION
 ; RELOC-NEXT:         Name:            colliding_func1
 ; RELOC-NEXT:         Flags:           [  ]
 ; RELOC-NEXT:         Function:        9
 ; RELOC-NEXT:       - Index:           10
 ; RELOC-NEXT:         Kind:            FUNCTION

lld/test/wasm/relocatable.ll

 ; RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/hello.s -o %t.hello.o
 ; RUN: llc -filetype=obj %s -o %t.o
-; RUN: wasm-ld -r -o %t.wasm %t.hello.o %t.o
-; RUN: obj2yaml %t.wasm | FileCheck %s
+; RUN: wasm-ld -r -o %t2.o %t.hello.o %t.o
+; RUN: obj2yaml %t2.o | FileCheck %s
+
+; Verify the resulting object can be used as linker input
+; RUN: wasm-ld --allow-undefined -o %t.wasm %t2.o --export-table
 
 target triple = "wasm32-unknown-unknown"
 
 ; Function Attrs: nounwind
 define hidden i32 @my_func() local_unnamed_addr {
 entry:
   %call = tail call i32 @foo_import()
   %call2 = tail call i32 @bar_import()

 define linkonce_odr i32 @func_comdat() comdat {
 entry:
   ret i32 ptrtoint ([3 x i8]* @data_comdat to i32)
 }
 
 ; Test that __attribute__(used) (i.e NO_STRIP) is preserved in the relocated symbol table
 @llvm.used = appending global [1 x i8*] [i8* bitcast (i32 ()* @my_func to i8*)], section "llvm.metadata"
 
+define void @_start() {
+  ret void
+}
+
+
 ; CHECK:      --- !WASM
 ; CHECK-NEXT: FileHeader:
 ; CHECK-NEXT:   Version:         0x1
 ; CHECK-NEXT: Sections:
 ; CHECK-NEXT:   - Type:            TYPE
 ; CHECK-NEXT:     Signatures:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         ParamTypes:

 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           foo_import
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         SigIndex:        1
 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           bar_import
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         SigIndex:        1
-; CHECK-NEXT:   - Type:            FUNCTION
-; CHECK-NEXT:     FunctionTypes:   [ 2, 1, 1 ]
-; CHECK-NEXT:   - Type:            TABLE
-; CHECK-NEXT:     Tables:
-; CHECK-NEXT:       - Index:           0
-; CHECK-NEXT:         ElemType:        FUNCREF
-; CHECK-NEXT:         Limits:
-; CHECK-NEXT:           Flags:           [ HAS_MAX ]
-; CHECK-NEXT:           Initial:         0x4
-; CHECK-NEXT:           Maximum:         0x4
+; CHECK-NEXT:       - Module:          env
+; CHECK-NEXT:         Field:           __indirect_function_table
+; CHECK-NEXT:         Kind:            TABLE
+; CHECK-NEXT:         Table:
+; CHECK-NEXT:           Index:           0
+; CHECK-NEXT:           ElemType:        FUNCREF
+; CHECK-NEXT:           Limits:
+; CHECK-NEXT:             Initial:         0x3
+; CHECK-NEXT:   - Type:            FUNCTION
+; CHECK-NEXT:     FunctionTypes:   [ 2, 1, 1, 2 ]
 ; CHECK-NEXT:   - Type:            MEMORY
 ; CHECK-NEXT:     Memories:
 ; CHECK-NEXT:      - Initial:         0x1
 ; CHECK-NEXT:   - Type:            ELEM
 ; CHECK-NEXT:     Segments:
 ; CHECK-NEXT:       - Offset:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           1

lld/test/wasm/signature-mismatch.ll

 ; RELOC-NEXT:         Kind:            DATA
 ; RELOC-NEXT:         Name:            ret32_address_main
 ; RELOC-NEXT:         Flags:           [  ]
 ; RELOC-NEXT:         Segment:         0
 ; RELOC-NEXT:         Size:            4
 ; RELOC-NEXT:       - Index:           3
 ; RELOC-NEXT:         Kind:            TABLE
 ; RELOC-NEXT:         Name:            __indirect_function_table
-; RELOC-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
+; RELOC-NEXT:         Flags:           [ UNDEFINED, NO_STRIP ]
 ; RELOC-NEXT:         Table:           0
 ; RELOC-NEXT:       - Index:           4
 ; RELOC-NEXT:         Kind:            FUNCTION
 ; RELOC-NEXT:         Name:            call_ret32
 ; RELOC-NEXT:         Flags:           [ ]
 ; RELOC-NEXT:         Function:        3
 ; RELOC-NEXT:       - Index:           5
 ; RELOC-NEXT:         Kind:            DATA

lld/test/wasm/weak-alias.ll

 ; RELOC-NEXT:           - I32
 ; RELOC-NEXT:   - Type:            IMPORT
 ; RELOC-NEXT:     Imports:
 ; RELOC-NEXT:       - Module:          env
 ; RELOC-NEXT:         Field:           __stack_pointer
 ; RELOC-NEXT:         Kind:            GLOBAL
 ; RELOC-NEXT:         GlobalType:      I32
 ; RELOC-NEXT:         GlobalMutable:   true
-; RELOC-NEXT:   - Type:            FUNCTION
-; RELOC-NEXT:     FunctionTypes:   [ 0, 1, 1, 1, 1, 1 ]
-; RELOC-NEXT:   - Type:            TABLE
-; RELOC-NEXT:     Tables:
-; RELOC-NEXT:       - Index:           0
-; RELOC-NEXT:         ElemType:        FUNCREF
-; RELOC-NEXT:         Limits:
-; RELOC-NEXT:           Flags:           [ HAS_MAX ]
-; RELOC-NEXT:           Initial:         0x2
-; RELOC-NEXT:           Maximum:         0x2
+; RELOC-NEXT:       - Module:          env
+; RELOC-NEXT:         Field:           __indirect_function_table
+; RELOC-NEXT:         Kind:            TABLE
+; RELOC-NEXT:         Table:
+; RELOC-NEXT:           Index:           0
+; RELOC-NEXT:           ElemType:        FUNCREF
+; RELOC-NEXT:           Limits:
+; RELOC-NEXT:             Initial:         0x1
+; RELOC-NEXT:   - Type:            FUNCTION
+; RELOC-NEXT:     FunctionTypes:   [ 0, 1, 1, 1, 1, 1 ]
 ; RELOC-NEXT:   - Type:            MEMORY
 ; RELOC-NEXT:     Memories:
 ; RELOC-NEXT:       - Initial:         0x0
 ; RELOC-NEXT:   - Type:            ELEM
 ; RELOC-NEXT:     Segments:
 ; RELOC-NEXT:       - Offset:
 ; RELOC-NEXT:           Opcode:          I32_CONST
 ; RELOC-NEXT:           Value:           1

 ; RELOC-NEXT:       - Index:           7
 ; RELOC-NEXT:         Kind:            FUNCTION
 ; RELOC-NEXT:         Name:            call_direct_ptr
 ; RELOC-NEXT:         Flags:           [  ]
 ; RELOC-NEXT:         Function:        5
 ; RELOC-NEXT:       - Index:           8
 ; RELOC-NEXT:         Kind:            TABLE
 ; RELOC-NEXT:         Name:            __indirect_function_table
-; RELOC-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
+; RELOC-NEXT:         Flags:           [ UNDEFINED, NO_STRIP ]
 ; RELOC-NEXT:         Table:           0
 ; RELOC-NEXT:   - Type:            CUSTOM
 ; RELOC-NEXT:     Name:            name
 ; RELOC-NEXT:     FunctionNames:
 ; RELOC-NEXT:       - Index:           0
 ; RELOC-NEXT:         Name:            _start
 ; RELOC-NEXT:       - Index:           1
 ; RELOC-NEXT:         Name:            direct_fn

lld/wasm/Driver.cpp

static TableSymbol *createUndefinedIndirectFunctionTable(StringRef name) {
   Symbol *sym =
       symtab->addUndefinedTable(name, name, module, flags, nullptr, type);
   sym->markLive();
   sym->forceExport = config->exportTable;
   return cast<TableSymbol>(sym);
 }
 
 static TableSymbol *resolveIndirectFunctionTable() {
-  // Even though we may not need a table, if the user explicitly specified
-  // --import-table or --export-table, ensure a table is residualized.
-  if (config->importTable)
-    return createUndefinedIndirectFunctionTable(functionTableName);
-  if (config->exportTable)
-    return createDefinedIndirectFunctionTable(functionTableName);
-
-  // Otherwise, check to the symtab to find the indirect function table.
-  if (Symbol *sym = symtab->find(functionTableName)) {
-    if (sym->isLive()) {
-      if (auto *t = dyn_cast<TableSymbol>(sym)) {
-        return t->isDefined()
-                   ? t
-                   : createDefinedIndirectFunctionTable(functionTableName);
-      }
-    }
+  Symbol *existingTable = symtab->find(functionTableName);
+  if (existingTable) {
+    if (!isa<TableSymbol>(existingTable)) {
+      error(Twine("reserved symbol must be of type table: `") +
+            functionTableName + "`");
+      return nullptr;
+    }
+    if (existingTable->isDefined()) {
+      error(Twine("reserved symbol must not be defined in input files: `") +
+            functionTableName + "`");
+      return nullptr;
+
+  if (config->importTable) {
+    if (existingTable)
+      return cast<TableSymbol>(existingTable);
+    else
+      return createUndefinedIndirectFunctionTable(functionTableName);
+  } else if ((existingTable && existingTable->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) {
if (entrySym)
     entrySym->setHidden(false);
 
   if (errorCount())
     return;
 
   // Do size optimizations: garbage collection
   markLive();
 
+  if (!config->relocatable) {
     // Provide the indirect funciton table if needed.
     WasmSym::indirectFunctionTable = resolveIndirectFunctionTable();
 
     if (errorCount())
       return;
+  }
 
   // Write the result to the file.
   writeResult();
 }
 
 } // namespace wasm
 } // namespace lld