Revert "[WebAssembly] MC layer writes table symbols to object files"

[sbc100]

Summary

This reverts commit d806618636f8a82bfc3f620e1fad83af4d2a2575.
Review: https://reviews.llvm.org/D92215

We had issues where older version of wasm-ld were crashing if object
files contains this new symbol type. Even if they correctly report the
unknown symbol type they still can't handle the object as input. We
decided that the best stratagy here is to only generate these symbol
types if refernece types is enabled. Without reference types enabled we
should never geneate a table symbol or a TABLE_NUMBER relocation.

This revert is in addition to the one already reverted in
https://reviews.llvm.org/D95005.

The plan is to re-land these in updated form after the llvm 12 branch
point.

[wingo]

@sbc100 @tlively what is the relationship between wasm-ld versions and compiler versions? Surely it's not that any LD should accept anything from any compiler, right?

[sbc100]

In D95420#2522285, @wingo wrote:

@sbc100 @tlively what is the relationship between wasm-ld versions and compiler versions? Surely it's not that any LD should accept anything from any compiler, right?

In general, we should strive to avoid breaking changes to object file format. There can be cases when object files built with a LLVM version X and consumed by LLVM version Y. For example a pre-built library (i.e. libMyGameEngine.a) which is distributed in binary form. Ideally that library should be re-usable across as many LLVM versions as possible.

Occasionally this is not possible. A couple of examples might be:

1. Use of a new feature of wasm that requires new types of symbol/relocation (e.g. ref types)
2. We decide to change the object format metadata layout (e.g. redesign the symbol table)

(1) breaks any object file that opts into using the new features but should not break object thats don't
(2) breaks *all* objects files and we really want to avoid that if we can. This is when we would bump the object file version number

In think what happened with this change is we probably should have done (1) but we inadvertently did (2). At least that is how I see it. @tlively would you agree?

[wingo]

I agree that old object files should always be consumable by new linkers. But the reverse does not follow for me in an obvious way. Is there a bound on how old a wasm-ld is supported for new compilers? Like is it one major version, or six, or all of them?

[sbc100]

In D95420#2522869, @wingo wrote:

I agree that old object files should always be consumable by new linkers. But the reverse does not follow for me in an obvious way. Is there a bound on how old a wasm-ld is supported for new compilers? Like is it one major version, or six, or all of them?

I think its more that we shouldn't break forward compatibility unless we need too. In this case it seems that we can avoid the breakage for those that don't explicitly opt into the new feature (reference types). The approach we chose to take here broke forward comat for *all* object files.. this is something can could consider doing one day, but I would like to minimize the number of times we do this (and reserve that action for something more like a complete re-design of the object format).

[tlively]

Yes, I agree. We advertise a stable object format with the goal of making the linker forward-compatible. The value we get from that is that users with different toolchain versions (for both Emscripten and WASI) can share libraries without worrying about version skew in either direction as long as a least common denominator of features is used.

llvm/lib/MC/WasmObjectWriter.cpp

if (Sym) {
             "symbol '__indirect_function_table' is not a function table");
     } else {
       Sym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(TableName));
       Sym->setFunctionTable();
       // The default function table is synthesized by the linker.
       Sym->setUndefined();
     }
     Sym->setUsedInReloc();
-    // Any time we have a TABLE_INDEX relocation against a function symbol, we
-    // need to ensure that table itself is part of the final output too.  In the
-    // future we may want to define a new kind of reloc against both the
-    // function and the table, so that the linker can see that the function
-    // symbol keeps the table alive, but for now manually mark the table as
-    // live.
-    Sym->setNoStrip();
     Asm.registerSymbol(*Sym);
   }
 
   // Relocation other than R_WASM_TYPE_INDEX_LEB are required to be
   // against a named symbol.
   if (Type != wasm::R_WASM_TYPE_INDEX_LEB) {
     if (SymA->getName().empty())
       report_fatal_error("relocations against un-named temporaries are not yet "
uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,
 
   // Finally, populate the symbol table itself, in its "natural" order.
   for (const MCSymbol &S : Asm.symbols()) {
     const auto &WS = static_cast<const MCSymbolWasm &>(S);
     if (!isInSymtab(WS)) {
       WS.setIndex(InvalidIndex);
       continue;
     }
+    if (WS.isTable() && WS.getName() == "__indirect_function_table") {
+      // For the moment, don't emit table symbols -- wasm-ld can't handle them.
+      continue;
+    }
     LLVM_DEBUG(dbgs() << "adding to symtab: " << WS << "\n");
 
     uint32_t Flags = 0;
     if (WS.isWeak())
       Flags |= wasm::WASM_SYMBOL_BINDING_WEAK;
     if (WS.isHidden())
       Flags |= wasm::WASM_SYMBOL_VISIBILITY_HIDDEN;
     if (!WS.isExternal() && WS.isDefined())

llvm/test/MC/WebAssembly/debug-info.ll

 ; CHECK-NEXT:  Section {
 ; CHECK-NEXT:    Type: CUSTOM (0x0)
 ; CHECK-NEXT:    Size: 57
 ; CHECK-NEXT:    Offset: 656
 ; CHECK-NEXT:    Name: .debug_line
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:  Section {
 ; CHECK-NEXT:    Type: CUSTOM (0x0)
-; CHECK-NEXT:    Size: 95
+; CHECK-NEXT:    Size: 91
 ; CHECK-NEXT:    Offset: 731
 ; CHECK-NEXT:    Name: linking
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:  Section {
 ; CHECK-NEXT:    Type: CUSTOM (0x0)
 ; CHECK-NEXT:    Size: 9
-; CHECK-NEXT:    Offset: 840
+; CHECK-NEXT:    Offset: 836
 ; CHECK-NEXT:    Name: reloc.DATA
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:  Section {
 ; CHECK-NEXT:    Type: CUSTOM (0x0)
 ; CHECK-NEXT:    Size: 61
-; CHECK-NEXT:    Offset: 866
+; CHECK-NEXT:    Offset: 862
 ; CHECK-NEXT:    Name: reloc..debug_info
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:  Section {
 ; CHECK-NEXT:    Type: CUSTOM (0x0)
 ; CHECK-NEXT:    Size: 6
-; CHECK-NEXT:    Offset: 951
+; CHECK-NEXT:    Offset: 947
 ; CHECK-NEXT:    Name: reloc..debug_pubnames
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:  Section {
 ; CHECK-NEXT:    Type: CUSTOM (0x0)
 ; CHECK-NEXT:    Size: 6
-; CHECK-NEXT:    Offset: 985
+; CHECK-NEXT:    Offset: 981
 ; CHECK-NEXT:    Name: reloc..debug_pubtypes
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:  Section {
 ; CHECK-NEXT:    Type: CUSTOM (0x0)
 ; CHECK-NEXT:    Size: 6
-; CHECK-NEXT:    Offset: 1019
+; CHECK-NEXT:    Offset: 1015
 ; CHECK-NEXT:    Name: reloc..debug_line
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:  Section {
 ; CHECK-NEXT:    Type: CUSTOM (0x0)
 ; CHECK-NEXT:    Size: 77
-; CHECK-NEXT:    Offset: 1049
+; CHECK-NEXT:    Offset: 1045
 ; CHECK-NEXT:    Name: producers
 ; CHECK-NEXT:  }
 ; CHECK-NEXT:]
 ; CHECK-NEXT:Relocations [
 ; CHECK-NEXT:  Section (7) DATA {
 ; CHECK-NEXT:    0x6 R_WASM_MEMORY_ADDR_I32 myextern 0
 ; CHECK-NEXT:    0xF R_WASM_TABLE_INDEX_I32 f2
 ; CHECK-NEXT:  }

 ; CHECK-NEXT:  Symbol {
 ; CHECK-NEXT:    Name: .debug_line
 ; CHECK-NEXT:    Type: SECTION (0x3)
 ; CHECK-NEXT:    Flags [ (0x2)
 ; CHECK-NEXT:      BINDING_LOCAL (0x2)
 ; CHECK-NEXT:    ]
 ; CHECK-NEXT:    ElementIndex: 0xC
 ; CHECK-NEXT:  }
-; CHECK-NEXT:  Symbol {
-; CHECK-NEXT:    Name: __indirect_function_table
-; CHECK-NEXT:    Type: TABLE (0x5)
-; CHECK-NEXT:    Flags [ (0x90)
-; CHECK-NEXT:      NO_STRIP (0x80)
-; CHECK-NEXT:      UNDEFINED (0x10)
-; CHECK-NEXT:    ]
-; CHECK-NEXT:    ImportModule: env
-; CHECK-NEXT:    ElementIndex: 0x0
-; CHECK-NEXT:  }
 ; CHECK-NEXT:]
 
 ; generated from the following C code using: clang --target=wasm32 -g -O0 -S -emit-llvm test.c
 ; extern int myextern;
 ; void f2(void) { return; }
 ;
 ; int* foo = &myextern;
 ; void (*ptr2)(void) = f2;

llvm/test/MC/WebAssembly/debug-info64.ll

 ; CHECK-NEXT:   Section {
 ; CHECK-NEXT:     Type: CUSTOM (0x0)
 ; CHECK-NEXT:     Size: 61
 ; CHECK-NEXT:     Offset: 680
 ; CHECK-NEXT:     Name: .debug_line
 ; CHECK-NEXT:   }
 ; CHECK-NEXT:   Section {
 ; CHECK-NEXT:     Type: CUSTOM (0x0)
-; CHECK-NEXT:     Size: 95
+; CHECK-NEXT:     Size: 91
 ; CHECK-NEXT:     Offset: 759
 ; CHECK-NEXT:     Name: linking
 ; CHECK-NEXT:   }
 ; CHECK-NEXT:   Section {
 ; CHECK-NEXT:     Type: CUSTOM (0x0)
 ; CHECK-NEXT:     Size: 9
-; CHECK-NEXT:     Offset: 868
+; CHECK-NEXT:     Offset: 864
 ; CHECK-NEXT:     Name: reloc.DATA
 ; CHECK-NEXT:   }
 ; CHECK-NEXT:   Section {
 ; CHECK-NEXT:     Type: CUSTOM (0x0)
 ; CHECK-NEXT:     Size: 61
-; CHECK-NEXT:     Offset: 894
+; CHECK-NEXT:     Offset: 890
 ; CHECK-NEXT:     Name: reloc..debug_info
 ; CHECK-NEXT:   }
 ; CHECK-NEXT:   Section {
 ; CHECK-NEXT:     Type: CUSTOM (0x0)
 ; CHECK-NEXT:     Size: 6
-; CHECK-NEXT:     Offset: 979
+; CHECK-NEXT:     Offset: 975
 ; CHECK-NEXT:     Name: reloc..debug_pubnames
 ; CHECK-NEXT:   }
 ; CHECK-NEXT:   Section {
 ; CHECK-NEXT:     Type: CUSTOM (0x0)
 ; CHECK-NEXT:     Size: 6
-; CHECK-NEXT:     Offset: 1013
+; CHECK-NEXT:     Offset: 1009
 ; CHECK-NEXT:     Name: reloc..debug_pubtypes
 ; CHECK-NEXT:   }
 ; CHECK-NEXT:   Section {
 ; CHECK-NEXT:     Type: CUSTOM (0x0)
 ; CHECK-NEXT:     Size: 6
-; CHECK-NEXT:     Offset: 1047
+; CHECK-NEXT:     Offset: 1043
 ; CHECK-NEXT:     Name: reloc..debug_line
 ; CHECK-NEXT:   }
 ; CHECK-NEXT:   Section {
 ; CHECK-NEXT:     Type: CUSTOM (0x0)
 ; CHECK-NEXT:     Size: 77
-; CHECK-NEXT:     Offset: 1077
+; CHECK-NEXT:     Offset: 1073
 ; CHECK-NEXT:     Name: producers
 ; CHECK-NEXT:   }
 ; CHECK-NEXT: ]
 ; CHECK-NEXT: Relocations [
 ; CHECK-NEXT:   Section (7) DATA {
 ; CHECK-NEXT:     0x6 R_WASM_MEMORY_ADDR_I64 myextern 0
 ; CHECK-NEXT:     0x13 R_WASM_TABLE_INDEX_I64 f2
 ; CHECK-NEXT:   }

 ; CHECK-NEXT:   Symbol {
 ; CHECK-NEXT:     Name: .debug_line
 ; CHECK-NEXT:     Type: SECTION (0x3)
 ; CHECK-NEXT:     Flags [ (0x2)
 ; CHECK-NEXT:       BINDING_LOCAL (0x2)
 ; CHECK-NEXT:     ]
 ; CHECK-NEXT:     ElementIndex: 0xC
 ; CHECK-NEXT:   }
-; CHECK-NEXT:   Symbol {
-; CHECK-NEXT:     Name: __indirect_function_table
-; CHECK-NEXT:     Type: TABLE (0x5)
-; CHECK-NEXT:     Flags [ (0x90)
-; CHECK-NEXT:       NO_STRIP (0x80)
-; CHECK-NEXT:       UNDEFINED (0x10)
-; CHECK-NEXT:     ]
-; CHECK-NEXT:     ImportModule: env
-; CHECK-NEXT:     ElementIndex: 0x0
-; CHECK-NEXT:   }
 ; CHECK-NEXT: ]
 
 ; generated from the following C code using: clang --target=wasm64 -g -O0 -S -emit-llvm test.c
 ; extern int myextern;
 ; void f2(void) { return; }
 ;
 ; int* foo = &myextern;
 ; void (*ptr2)(void) = f2;

llvm/test/MC/WebAssembly/function-alias.ll

 ; CHECK-NEXT:   }
 ; CHECK-NEXT:   Symbol {
 ; CHECK-NEXT:     Name: bar
 ; CHECK-NEXT:     Type: FUNCTION (0x0)
 ; CHECK-NEXT:     Flags [ (0x0)
 ; CHECK-NEXT:     ]
 ; CHECK-NEXT:     ElementIndex: 0x0
 ; CHECK-NEXT:   }
-; CHECK-NEXT:   Symbol {
-; CHECK-NEXT:     Name: __indirect_function_table
-; CHECK-NEXT:     Type: TABLE (0x5)
-; CHECK-NEXT:     Flags [ (0x90)
-; CHECK-NEXT:       NO_STRIP (0x80)
-; CHECK-NEXT:       UNDEFINED (0x10)
-; CHECK-NEXT:     ]
-; CHECK-NEXT:     ImportModule: env
-; CHECK-NEXT:     ElementIndex: 0x0
-; CHECK-NEXT:   }
 ; CHECK-NEXT: ]

llvm/test/MC/WebAssembly/global-ctor-dtor.ll

 ; CHECK-NEXT:         Name:            func1
 ; CHECK-NEXT:         Flags:           [ UNDEFINED ]
 ; CHECK-NEXT:         Function:        3
 ; CHECK-NEXT:       - Index:           10
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Name:            func0
 ; CHECK-NEXT:         Flags:           [ UNDEFINED ]
 ; CHECK-NEXT:         Function:        4
-; CHECK-NEXT:       - Index:           11
-; CHECK-NEXT:         Kind:            TABLE
-; CHECK-NEXT:         Name:            __indirect_function_table
-; CHECK-NEXT:         Flags:           [ UNDEFINED, NO_STRIP ]
-; CHECK-NEXT:         Table:           0
 ; CHECK-NEXT:     SegmentInfo:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            .data.global1
 ; CHECK-NEXT:         Alignment:       3
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT:     InitFunctions:
 ; CHECK-NEXT:       - Priority: 42
 ; CHECK-NEXT:         Symbol: 9
 ; CHECK-NEXT:       - Priority: 42
 ; CHECK-NEXT:         Symbol: 2
 ; CHECK-NEXT:       - Priority: 65535
 ; CHECK-NEXT:         Symbol: 10
 ; CHECK-NEXT:       - Priority: 65535
 ; CHECK-NEXT:         Symbol: 7
 ; CHECK-NEXT: ...

llvm/test/MC/WebAssembly/reloc-pic.s

 # CHECK-NEXT:         Kind:            DATA
 # CHECK-NEXT:         Name:            __table_base
 # CHECK-NEXT:         Flags:           [ UNDEFINED ]
 # CHECK-NEXT:       - Index:           9
 # CHECK-NEXT:         Kind:            FUNCTION
 # CHECK-NEXT:         Name:            hidden_func
 # CHECK-NEXT:         Flags:           [ BINDING_LOCAL ]
 # CHECK-NEXT:         Function:        5
-# CHECK-NEXT:       - Index:           10
-# CHECK-NEXT:         Kind:            TABLE
-# CHECK-NEXT:         Name:            __indirect_function_table
-# CHECK-NEXT:         Flags:           [ UNDEFINED, NO_STRIP ]
-# CHECK-NEXT:         Table:           0
 # CHECK-NEXT:     SegmentInfo:
 # CHECK-NEXT:       - Index:           0
 # CHECK-NEXT:         Name:            .rodata.hidden_data
 # CHECK-NEXT:         Alignment:       0
 # CHECK-NEXT:         Flags:           [  ]
 # CHECK-NEXT: ...

llvm/test/MC/WebAssembly/type-index.s

 # BIN-NEXT:     Name:            linking
 # BIN-NEXT:     Version:         2
 # BIN-NEXT:     SymbolTable:
 # BIN-NEXT:       - Index:           0
 # BIN-NEXT:         Kind:            FUNCTION
 # BIN-NEXT:         Name:            test0
 # BIN-NEXT:         Flags:           [ BINDING_LOCAL ]
 # BIN-NEXT:         Function:        0
-# BIN-NEXT:       - Index:           1
-# BIN-NEXT:         Kind:            TABLE
-# BIN-NEXT:         Name:            __indirect_function_table
-# BIN-NEXT:         Flags:           [ UNDEFINED, NO_STRIP ]
-# BIN-NEXT:         Table:           0
 # BIN-NEXT: ...

llvm/test/MC/WebAssembly/weak-alias.s

 # CHECK-NEXT:         Segment:         0
 # CHECK-NEXT:         Size:            4
 # CHECK-NEXT:       - Index:           9
 # CHECK-NEXT:         Kind:            DATA
 # CHECK-NEXT:         Name:            bar_alias
 # CHECK-NEXT:         Flags:           [ BINDING_WEAK, VISIBILITY_HIDDEN, NO_STRIP ]
 # CHECK-NEXT:         Segment:         0
 # CHECK-NEXT:         Size:            4
-# CHECK-NEXT:       - Index:           10
-# CHECK-NEXT:         Kind:            TABLE
-# CHECK-NEXT:         Name:            __indirect_function_table
-# CHECK-NEXT:         Flags:           [ UNDEFINED, NO_STRIP ]
-# CHECK-NEXT:         Table:           0
 # CHECK-NEXT:     SegmentInfo:
 # CHECK-NEXT:       - Index:           0
 # CHECK-NEXT:         Name:            .data.bar
 # CHECK-NEXT:         Alignment:       3
 # CHECK-NEXT:         Flags:           [ ]
 # CHECK-NEXT:       - Index:           1
 # CHECK-NEXT:         Name:            .data.direct_address
 # CHECK-NEXT:         Alignment:       3