[WebAssembly][lld] Preassign table number 0 to indirect function table for MVP inputs

[wingo]

Summary

MVP object files may import at most one table, and if they do, it must
be assigned table number zero in the output, as the references to that
table are not relocatable. Ensure that this is the case, even if some
inputs define other tables.

[wingo]

Looking over this changeset, I see that we need some tests. But also there is a fair amount of code duplication for the "precoloring" logic. I can make it be a part of TableSymbol::setTableNumber and InputTable::setTableNumber; it would seem that that much duplication is necessary. But then the errors would probably be less helpful because there's less context at that point. Thoughts?

[tlively]

In D96001#2541348, @wingo wrote:

Looking over this changeset, I see that we need some tests. But also there is a fair amount of code duplication for the "precoloring" logic. I can make it be a part of TableSymbol::setTableNumber and InputTable::setTableNumber; it would seem that that much duplication is necessary. But then the errors would probably be less helpful because there's less context at that point. Thoughts?

IMO having helpful error messages is well worth some code duplication. Code duplication won't cause users to file the same bug every other week about their links failing, but unhelpful error messages will!

lld/wasm/Driver.cpp

804

Can this dyn_cast ever fail? If not, it would be better to use cast here or even make the parameter a TableSymbol.

lld/wasm/InputFiles.cpp

340

Same question about this dyn_cast.

lld/wasm/SyntheticSections.cpp

118–119

Would it be possible to include the name of an object file that contains the offending non-relocatable table? I've anecdotally found that including file names in similar error messages about unlinkable MVP and atomics objects has dramatically decreased the number of bugs that get filed about it.

lld/wasm/Writer.cpp

597

It would be good to have a short comment here as well about why some tables need to be deferred.

[sbc100]

I'm not familiar with the use of term precoloring. Perhaps I should be.. but maybe we could use something more common like preassignTableNumber rather than precolorTableNumber? Certainly I don't think we want to use the term in the user-facing errors.

lld/wasm/InputFiles.cpp

343

I'm not sure this is a very usefull user-facing error. Can it ever actually happen? Can we improve it or perhaps make it an assert?

345

Can we simplify the code here by asserting that the table number is always zero and that there is only one table.

In fact, perhaps this should be called synthesizeTableSymbol (non-plural) and just return after for first call to addTable?

lld/wasm/SymbolTable.cpp

213 ↗

(On Diff #321326)

I think this can be an unconditionally cast<> can't it?

219 ↗

(On Diff #321326)

Similarly, since there is exactly one nonrelocatable table number (zero)... can we simplify this code based on that fact?

[sbc100]

lld/wasm/SyntheticSections.cpp

121

Can this error actually happy? Wouldn't it be a bug in lld if it did? It seems like in order for this to happen there would be a bug in the pre-coloring logic.

[wingo]

Thanks very much for the review!

The "precoloring" name is a bit sloppy -- comes from register allocation FWIW, where you solve the graph coloring problem for the interference graph of register uses. Some nodes require that a register be in a specific place, e.g. %rdi for argument 0 on x86; imposing that constraint is called precoloring. So for tables, we also have a kind of precoloring constraint. But, probably best to just used the term "preassign".

lld/wasm/Driver.cpp

804

I was thinking that it could fail if the existing symbol isn't a table. In that case we would have already issued an error, but control flow could still fall through here. But now I see that we make an early-return in that case below; will fix.

lld/wasm/InputFiles.cpp

340

Unfortunately it is possible that e.g. SymbolTable::createUndefinedTable returns a non-table symbol, if there was an existing symtab entry. We would have issued an error at that point though.

343

This is a sloppy error and I need to reword it; tx for catching.

345

Humm, good question! Old linkers would not propagate "additional" tables from input to output. Also, old compilers wouldn't define a table locally -- the indirect function table would only be imported. So maybe it would be good to avoid adding a new form of acceptable input, in which object files have many table symbols.

If we tighten this restriction, we can issue an error for inputs that

• have no table symbols. and:
  • import any table not named __indirect_function_table, or
  • import more than one table, or
  • define any table

WDYT?

lld/wasm/SymbolTable.cpp

213 ↗

(On Diff #321326)

AFAICS, sadly, no :( Otherwise we could simplify checkTableType as well.

219 ↗

(On Diff #321326)

Sure. I can add a isPreassignedToTableNumberZero method or so. Surely with a better name.

lld/wasm/SyntheticSections.cpp

118–119

Sure.

121

I was thinking that it could happen if you had an MVP input that defined a table locally (not imported), but a reftypes input imported a table. But prompted by your questions I think I see that MVP inputs wouldn't ever define tables, so this error can't happen.

lld/wasm/Writer.cpp

597

Sure.

[wingo]

Address feedback

[sbc100]

lld/wasm/InputFiles.cpp

345

Sounds great!

[wingo]

Fixup casts

[wingo]

Tighten up restrictions on MVP inputs

[wingo]

Add test

[wingo]

So.... this one is ready for review again I think. I haven't been able to test some of the the error cases in synthesizeMVPIndirectFunctionTableIfNeeded because you can't actually make files with those characteristics currently.

[sbc100]

lld/wasm/InputFiles.cpp

351

For this last paragraphs how about:

"Table usage in MVP objects cannot be relocatates so the linker must ensure that this
table gets assigned index zero".

[sbc100]

lld/wasm/Driver.cpp

804

Can these two lines be moved into SymbolTable::addSyntheticTable (which already has a check for an existing symbol). Then you don't need to pass the extra existing arg here, and you don't need to extra existingTable table below?

hmm.. maybe that would make addSyntheticTable a little less tidy, but it seems like logical place... either that or replaceSymbol itself.

In general this notion of preassigned index seems to complicate things in a way that I'm not a fan of. I wish we could find a simpler way to force the indirect table to always have index 0.

[wingo]

lld/wasm/Driver.cpp

804

Right, the whole notion of symbols with preassigned addresses (table numbers in this case) is... unsatisfying. But it does seem to be essential to the constraints we're operating under. I thought that keeping the bits that much about with preassigned addresses local to the parts that deal in tables (and specifically the indirect function table) would limit the "taint".

Probably replaceSymbol is not the place for this to go, as other kinds of symbols rightly don't have a notion of preassigned addresses.

Incidentally, looking again, I don't think the code as it is works, because of the placement new in replaceSymbol :/ Will fix, I guess in addSyntheticTable.

[wingo]

Address feedback; fix placement-new bug

[wingo]

Fix comment

[wingo]

PTAL, I think all comments are addressed. Adding tlively in case he would like to weigh in.

[tlively]

This looks great, thanks @wingo! My only comments are about comments and error message contents. It would also be good to add tests for more of the error messages, especially the one about "Cannot preassign table number 0 to indirect function table..." because I expect that to be a common user-facing issue.

lld/wasm/InputFiles.cpp

333

368–369

I would remove the last sentence here and in the errors below. -mattr=+reference-types is an llc flag but not a clang flag, so it is probably not what the user wants. Also, clang should never produce objects that produce this error no matter what flags are passed to it.

lld/wasm/Symbols.cpp

391

It would be good to add a comment here along the lines of "zero is the only table number that can be preassigned." Otherwise from just the name of the function, it looks strange that other numbers aren't handled gracefully.

lld/wasm/SyntheticSections.cpp

237–238

The indirect function table is imported in Emscripten's dynamic linking scheme, so this isn't necessarily unexpected.

248–250

-mreference-types is the clang flags for enabling this feature.

Or another option would be to be tool- and flag-agnostic by saying "feature 'reference-types'" instead of naming particular flags.

[sbc100]

we when we comes to assigning indexes we do something like this:

lld/wasm/Symbols.h

382

I feel a little bad for pushing back more on the this change, but it still feels a little more invasive than it could be.

Rather than adding these two methods to the table class what do you think about this appoach:

• Add a global config variable that gets set if we have MVP objects in the link. We could call this something like`bool legacyFunctionTable = false`. Or alternatively "forceFunctioTableToBeZero"? Or "functionTableMustBeZero"? I kind of like the idea of a bool here because it clearly indicates that are two different modes.
• We already track the global symbol indirectFunctionTable.
• Then, we when we comes to assigning indexes we do something like this:

if (condig->legacyFunctionTable) {
     placeIndirectFunctionTableFirst()   <-- here we could report errors if the sorting is impossible
}

This sorting and error reporting could be done in the TableSection::assignIndexes().

Hopefully this should avoid adding the preassignTableNumberZero/hasPreassignedTableNumberZero methods and should avoid extra logic to too much extra logic addTable method.

WDYT?

(I guess it would also need to happen in ImportSection::assignIndexes...)

These are mostly cosmetic changes, that actual behaviour LGTM.

[sbc100]

lld/wasm/SyntheticSections.cpp

250

Can we be more succinct here? If you feel its is helpful/necessary to convey all this information than perhaps this verbosity is unavoidable.

Would something shorter such as "object file was not built -mattr=+reference-types enabled. This is incompatible with the use tables imports in <other-object-name>" be enough?

[wingo]

lld/wasm/Symbols.h

382

This can work. It's not quite as minimal as this because tables can also get assigned numbers via the import section. I'll give it a go and see.

lld/wasm/SyntheticSections.cpp

250

We can certainly be more succinct but I don't know how to be as informative. As the comment notes, we don't have a good way to know where the "conflicting" import comes from AFAIU.

[wingo]

Fix error messages and rebase

[wingo]

Instead of tracking unrelocatable uses of the indirect function table on the table symbol, just use a global flag in the config. The writer will take care to handle the indirect function table symbol specially if this flag is set, writing it before any other table.

[wingo]

Addressed feedback; @sbc100 would you mind having another look?

[sbc100]

Nice! I like this much better now. Thanks for making all those changes.

Just a few more nits and I think we can land this.

lld/test/wasm/invalid-mvp-table-use.s

3 ↗

(On Diff #323264)

Can you elaborate on this comment. What makes this an MVP file ? Is it simply the lack of -mattr=+reference-types.

Not part of this PR, but should we have llvm-mc fail here to catch this earlier?

lld/wasm/InputFiles.cpp

380–382

Can we replace this above block with simply:

lld/wasm/InputFiles.h

161

Can we call this addLegacyIndirectFunctionTableSymbolIfNeeded now so matches the config name?

lld/wasm/SyntheticSections.cpp

220

nit: Moving these method back down below writeBody would make the diff easier to read (smaller).

227

Can we combine these two into a single condition to remove one level of testing? In fact can we combine all three maybe?

lld/wasm/Writer.cpp

589

This can be combined into single condition (with no braces).

593

Oh nice so we always import this first, regardless of the config flag.. I like that.

[wingo]

Address feedback

lld/test/wasm/invalid-mvp-table-use.s

3 ↗

(On Diff #323264)

Elaborated. As regards llvm-mc, there will need to be some changes when we finally add (again) support for symtab entries; will follow up then.

lld/wasm/InputFiles.cpp

380–382

Missing comment here, perhaps?

[sbc100]

lld/wasm/InputFiles.cpp

380–382

Sorry, didn't mean to send that.

lld/wasm/Driver.cpp

for (size_t i = 0, e = syms.size(); i != e; ++i)
         syms[i] = s;
   });
 
   // Update pointers in the symbol table.
   for (const WrappedSymbol &w : wrapped)
     symtab->wrap(w.sym, w.real, w.wrap);
 }
 
-static TableSymbol *createDefinedIndirectFunctionTable(StringRef name) {
+static TableSymbol *createDefinedIndirectFunctionTable(StringRef name,
+                                                       TableSymbol *existing) {
   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;
+  // If the indirect function table is used by MVP inputs, make sure it is
+  // assigned table number 0.
+  if (existing && existing->hasPreassignedTableNumberZero())
+    sym->preassignTableNumberZero();
   return sym;
 }
 
 static TableSymbol *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 =
       symtab->addUndefinedTable(name, name, module, flags, nullptr, type);
   sym->markLive();
   sym->forceExport = config->exportTable;
   return cast<TableSymbol>(sym);
 }
 
 static TableSymbol *resolveIndirectFunctionTable() {
-  Symbol *existingTable = symtab->find(functionTableName);
-  if (existingTable) {
-    if (!isa<TableSymbol>(existingTable)) {
+  Symbol *existing = symtab->find(functionTableName);
+  if (existing) {
+    if (!isa<TableSymbol>(existing)) {
       error(Twine("reserved symbol must be of type table: `") +
             functionTableName + "`");
       return nullptr;
     }
-    if (existingTable->isDefined()) {
+    if (existing->isDefined()) {
       error(Twine("reserved symbol must not be defined in input files: `") +
             functionTableName + "`");
       return nullptr;
     }
   }
 
+  TableSymbol *existingTable = cast_or_null<TableSymbol>(existing);
   if (config->importTable) {
     if (existingTable)
-      return cast<TableSymbol>(existingTable);
+      return 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);
+    return createDefinedIndirectFunctionTable(functionTableName, existingTable);
   }
 
   // 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) {

lld/wasm/InputFiles.h

public:
   EventSymbol *getEventSymbol(uint32_t index) const;
   TableSymbol *getTableSymbol(uint32_t index) const;
 
 private:
   Symbol *createDefined(const WasmSymbol &sym);
   Symbol *createUndefined(const WasmSymbol &sym, bool isCalledDirectly);
 
   bool isExcludedByComdat(InputChunk *chunk) const;
-  void synthesizeTableSymbols();
+  void synthesizeMVPIndirectFunctionTableSymbol();
 
   std::unique_ptr<WasmObjectFile> wasmObj;
 };
 
 // .so file.
 class SharedFile : public InputFile {
 public:
   explicit SharedFile(MemoryBufferRef m) : InputFile(SharedKind, m) {}
   static bool classof(const InputFile *f) { return f->kind() == SharedKind; }

lld/wasm/InputFiles.cpp

auto relocsStart = std::lower_bound(relocsNext, relocsEnd,
                                         c->getInputSectionOffset(), relocLess);
     relocsNext = std::lower_bound(
         relocsStart, relocsEnd, c->getInputSectionOffset() + c->getInputSize(),
         relocLess);
     c->setRelocations(ArrayRef<WasmRelocation>(relocsStart, relocsNext));
   }
 }
 
-// Since LLVM 12, we expect that if an input file defines or uses a table, it
-// declares the tables using symbols and records each use with a relocation.
-// This way when the linker combines inputs, it can collate the tables used by
-// the inputs, assigning them distinct table numbers, and renumber all the uses
-// as appropriate.  At the same time, the linker has special logic to build the
+// An object file can have two approaches to tables.  With the reference-types
+// feature enabled, input files that define or use tables declare the tables
+// using symbols, and record each use with a relocation.  This way when the
+// linker combines inputs, it can collate the tables used by the inputs,
+// assigning them distinct table numbers, and renumber all the uses as
+// appropriate.  At the same time, the linker has special logic to build the
 // indirect function table if it is needed.
 //
-// However, object files produced by LLVM 11 and earlier neither write table
-// symbols nor record relocations, and yet still use tables via call_indirect,
-// and via function pointer bitcasts.  We can detect these object files, as they
-// declare tables as imports or define them locally, but don't have table
-// symbols.  synthesizeTableSymbols serves as a shim when loading these older
-// input files, defining the missing symbols to allow the indirect function
+// However, MVP object files (those that target WebAssembly 1.0, the "minimum
+// viable product" version of WebAssembly) neither write table symbols nor
+// record relocations.  These files can have at most one table, the indirect
+// function table used by call_indirect and which is the address space for
+// function points.  If this table is present, it is always an import.  If we
+// have a file with a table import but no table symbols, it is an MVP object
+// file.  synthesizeIndirectFunctionTableSymbol serves as a shim when loading
+// these input files, defining the missing symbol to allow the indirect function
 // table to be built.
 //
-// Table uses in these older files won't be relocated, as they have no
-// relocations.  In practice this isn't a problem, as these object files
-// typically just declare a single table named __indirect_function_table and
-// having table number 0, so relocation would be idempotent anyway.
-void ObjFile::synthesizeTableSymbols() {
-  uint32_t tableNumber = 0;
-  const WasmGlobalType *globalType = nullptr;
-  const WasmEventType *eventType = nullptr;
-  const WasmSignature *signature = nullptr;
-  if (wasmObj->getNumImportedTables()) {
-    for (const auto &import : wasmObj->imports()) {
-      if (import.Kind == WASM_EXTERNAL_TABLE) {
-        auto *info = make<WasmSymbolInfo>();
-        info->Name = import.Field;
-        info->Kind = WASM_SYMBOL_TYPE_TABLE;
-        info->ImportModule = import.Module;
-        info->ImportName = import.Field;
-        info->Flags = WASM_SYMBOL_UNDEFINED;
-        info->Flags |= WASM_SYMBOL_NO_STRIP;
-        info->ElementIndex = tableNumber++;
-        LLVM_DEBUG(dbgs() << "Synthesizing symbol for table import: "
-                          << info->Name << "\n");
-        auto *wasmSym = make<WasmSymbol>(*info, globalType, &import.Table,
-                                         eventType, signature);
-        symbols.push_back(createUndefined(*wasmSym, false));
-        // Because there are no TABLE_NUMBER relocs in this case, we can't
-        // compute accurate liveness info; instead, just mark the symbol as
-        // always live.
-        symbols.back()->markLive();
-      }
+// Table uses in these MVP inputs won't be relocated, as they have no
+// relocations.  The linker must ensure that they have the same numbers in the
+// output as in the input.
+void ObjFile::synthesizeMVPIndirectFunctionTableSymbol() {
+  if (errorCount())
+    return;
+
+  assert(wasmObj->getNumImportedTables() == 1);
+
+  bool haveTable = false;
+      if (haveTable) {
+        error(toString(this) +
+              ": multiple table imports, but no "
+              "corresponding symbol-table entries.  Recompile this input with "
+              "-mattr=+reference-types.");
+        continue;
+      }
+      haveTable = true;
+
+      info->ElementIndex = 0;
+      const WasmGlobalType *globalType = nullptr;
+      const WasmEventType *eventType = nullptr;
+      const WasmSignature *signature = nullptr;
+      Symbol *sym = createUndefined(*wasmSym, false);
+      // We're only sure it's a TableSymbol if the createUndefined succeeded.
+      if (errorCount())
+        continue;
+      symbols.push_back(sym);
+      // Because there are no TABLE_NUMBER relocs, we can't compute accurate
+      // liveness info; instead, just mark the symbol as always live.
+      sym->markLive();
+      // We assume that this compilation unit has unrelocatable references to
+      // this table, so preassign this table's table number.
+      cast<TableSymbol>(sym)->preassignTableNumberZero();
     }
   }
-  for (const auto &table : tables) {
-    auto *info = make<llvm::wasm::WasmSymbolInfo>();
-    // Empty name.
-    info->Kind = WASM_SYMBOL_TYPE_TABLE;
-    info->Flags = WASM_SYMBOL_BINDING_LOCAL;
-    info->Flags |= WASM_SYMBOL_VISIBILITY_HIDDEN;
-    info->Flags |= WASM_SYMBOL_NO_STRIP;
-    info->ElementIndex = tableNumber++;
-    LLVM_DEBUG(dbgs() << "Synthesizing symbol for table definition: "
-                      << info->Name << "\n");
-    auto *wasmSym = make<WasmSymbol>(*info, globalType, &table->getType(),
-                                     eventType, signature);
-    symbols.push_back(createDefined(*wasmSym));
-    // Mark live, for the same reasons as for imported tables.
-    symbols.back()->markLive();
+
+  // An MVP input has no table definitions.
+  if (!tables.empty()) {
   }
 }
 
 void ObjFile::parse(bool ignoreComdats) {
   // Parse a memory buffer as a wasm file.
   LLVM_DEBUG(dbgs() << "Parsing object: " << toString(this) << "\n");
   std::unique_ptr<Binary> bin = CHECK(createBinary(mb), toString(this));
 
for (const WasmGlobal &g : wasmObj->globals())
     globals.emplace_back(make<InputGlobal>(g, this));
 
   // Populate `Events`.
   for (const WasmEvent &e : wasmObj->events())
     events.emplace_back(make<InputEvent>(types[e.Type.SigIndex], e, this));
 
   // Populate `Symbols` based on the symbols in the object.
   symbols.reserve(wasmObj->getNumberOfSymbols());
-  bool haveTableSymbol = false;
+  uint32_t tableSymbolCount = 0;
   for (const SymbolRef &sym : wasmObj->symbols()) {
     const WasmSymbol &wasmSym = wasmObj->getWasmSymbol(sym.getRawDataRefImpl());
     if (wasmSym.isTypeTable())
-      haveTableSymbol = true;
+      tableSymbolCount++ = true;
     if (wasmSym.isDefined()) {
       // createDefined may fail if the symbol is comdat excluded in which case
       // we fall back to creating an undefined symbol
       if (Symbol *d = createDefined(wasmSym)) {
         symbols.push_back(d);
         continue;
       }
     }
     size_t idx = symbols.size();
     symbols.push_back(createUndefined(wasmSym, isCalledDirectly[idx]));
   }
 
-  // As a stopgap measure while implementing table support, if the object file
-  // has table definitions or imports but no table symbols, synthesize symbols
-  // for those tables.  Mark as NO_STRIP to ensure they reach the output file,
-  // even if there are no TABLE_NUMBER relocs against them.
-  if (!haveTableSymbol)
-    synthesizeTableSymbols();
+  uint32_t tableCount = wasmObj->getNumImportedTables() + tables.count();
+
+  if (tableCount != tableSymbolCount) {
+    if (tableSymbolCount == 0) {
+      // An MVP object file can have up to one table import, for the indirect
+      // function table, but will have no table definitions.
+      if (tables.count()) {
+        error(toString(this) +
+              ": unexpected table definition(s) without corresponding "
+              "symbol-table entries.  Recompile this input with "
+              "-mattr=+reference-types.");
+      } else if (tableCount != 1) {
+        error(toString(this) +
+              ": multiple table imports, but no corresponding symbol-table "
+              "entries.  Recompile this input with -mattr=+reference-types.");
+      } else {
+        synthesizeMVPIndirectFunctionTableSymbol();
+      }
+    else
 }
 
 bool ObjFile::isExcludedByComdat(InputChunk *chunk) const {
   uint32_t c = chunk->getComdat();
   if (c == UINT32_MAX)
     return false;
   return !keptComdats[c];
 }
 
 FunctionSymbol *ObjFile::getFunctionSymbol(uint32_t index) const {
   return cast<FunctionSymbol>(symbols[index]);
 }
 
 GlobalSymbol *ObjFile::getGlobalSymbol(uint32_t index) const {
   return cast<GlobalSymbol>(symbols[index]);
 }
 
 EventSymbol *ObjFile::getEventSymbol(uint32_t index) const {
   return cast<EventSymbol>(symbols[index]);
 }
 
 TableSymbol *ObjFile::getTableSymbol(uint32_t index) const {
   return cast<TableSymbol>(symbols[index]);
 }
 
 SectionSymbol *ObjFile::getSectionSymbol(uint32_t index) const {
   return cast<SectionSymbol>(symbols[index]);
 }
 
 DataSymbol *ObjFile::getDataSymbol(uint32_t index) const {
   return cast<DataSymbol>(symbols[index]);
 }
 
 Symbol *ObjFile::createDefined(const WasmSymbol &sym) {
   StringRef name = sym.Info.Name;
   uint32_t flags = sym.Info.Flags;
 
   switch (sym.Info.Kind) {
   case WASM_SYMBOL_TYPE_FUNCTION: {
     InputFunction *func =
         functions[sym.Info.ElementIndex - wasmObj->getNumImportedFunctions()];
     if (sym.isBindingLocal())
       return make<DefinedFunction>(name, flags, this, func);
     if (func->discarded)
       return nullptr;
     return symtab->addDefinedFunction(name, flags, this, func);
case WASM_SYMBOL_TYPE_TABLE: {
     if (sym.isBindingLocal())
       return make<DefinedTable>(name, flags, this, table);
     return symtab->addDefinedTable(name, flags, this, table);
   }
   }
   llvm_unreachable("unknown symbol kind");
 }
 
 Symbol *ObjFile::createUndefined(const WasmSymbol &sym, bool isCalledDirectly) {
   StringRef name = sym.Info.Name;
   uint32_t flags = sym.Info.Flags | WASM_SYMBOL_UNDEFINED;
 
   switch (sym.Info.Kind) {
   case WASM_SYMBOL_TYPE_FUNCTION:
     if (sym.isBindingLocal())
       return make<UndefinedFunction>(name, sym.Info.ImportName,
                                      sym.Info.ImportModule, flags, this,
                                      sym.Signature, isCalledDirectly);
     return symtab->addUndefinedFunction(name, sym.Info.ImportName,
                                         sym.Info.ImportModule, flags, this,
                                         sym.Signature, isCalledDirectly);
   case WASM_SYMBOL_TYPE_DATA:
     if (sym.isBindingLocal())
       return make<UndefinedData>(name, flags, this);
     return symtab->addUndefinedData(name, flags, this);
   case WASM_SYMBOL_TYPE_GLOBAL:
     if (sym.isBindingLocal())
       return make<UndefinedGlobal>(name, sym.Info.ImportName,
                                    sym.Info.ImportModule, flags, this,
                                    sym.GlobalType);
     return symtab->addUndefinedGlobal(name, sym.Info.ImportName,
                                       sym.Info.ImportModule, flags, this,
                                       sym.GlobalType);
   case WASM_SYMBOL_TYPE_TABLE:
     if (sym.isBindingLocal())
       return make<UndefinedTable>(name, sym.Info.ImportName,
                                   sym.Info.ImportModule, flags, this,
                                   sym.TableType);
     return symtab->addUndefinedTable(name, sym.Info.ImportName,
                                      sym.Info.ImportModule, flags, this,
                                      sym.TableType);
   case WASM_SYMBOL_TYPE_SECTION:
     llvm_unreachable("section symbols cannot be undefined");
   }
   llvm_unreachable("unknown symbol kind");
 }
 
 void ArchiveFile::parse() {
   // Parse a MemoryBufferRef as an archive file.
   LLVM_DEBUG(dbgs() << "Parsing library: " << toString(this) << "\n");
   file = CHECK(Archive::create(mb), toString(this));
 
   // Read the symbol table to construct Lazy symbols.
   int count = 0;
   for (const Archive::Symbol &sym : file->symbols()) {
     symtab->addLazy(this, &sym);
     ++count;
   }
   LLVM_DEBUG(dbgs() << "Read " << count << " symbols\n");
 }
 
 void ArchiveFile::addMember(const Archive::Symbol *sym) {
   const Archive::Child &c =
       CHECK(sym->getMember(),
             "could not get the member for symbol " + sym->getName());
 
   // Don't try to load the same member twice (this can happen when members
   // mutually reference each other).
   if (!seen.insert(c.getChildOffset()).second)
     return;
 
   LLVM_DEBUG(dbgs() << "loading lazy: " << sym->getName() << "\n");
   LLVM_DEBUG(dbgs() << "from archive: " << toString(this) << "\n");
 
   MemoryBufferRef mb =
       CHECK(c.getMemoryBufferRef(),
             "could not get the buffer for the member defining symbol " +
                 sym->getName());
 
   InputFile *obj = createObjectFile(mb, getName());
   symtab->addFile(obj);
 }
 
 static uint8_t mapVisibility(GlobalValue::VisibilityTypes gvVisibility) {
   switch (gvVisibility) {
   case GlobalValue::DefaultVisibility:
     return WASM_SYMBOL_VISIBILITY_DEFAULT;
   case GlobalValue::HiddenVisibility:
   case GlobalValue::ProtectedVisibility:
     return WASM_SYMBOL_VISIBILITY_HIDDEN;
   }
   llvm_unreachable("unknown visibility");
 }
 
 static Symbol *createBitcodeSymbol(const std::vector<bool> &keptComdats,
                                    const lto::InputFile::Symbol &objSym,
                                    BitcodeFile &f) {
   StringRef name = saver.save(objSym.getName());
 
   uint32_t flags = objSym.isWeak() ? WASM_SYMBOL_BINDING_WEAK : 0;
   flags |= mapVisibility(objSym.getVisibility());
 
   int c = objSym.getComdatIndex();
   bool excludedByComdat = c != -1 && !keptComdats[c];
 
   if (objSym.isUndefined() || excludedByComdat) {
     flags |= WASM_SYMBOL_UNDEFINED;
     if (objSym.isExecutable())
       return symtab->addUndefinedFunction(name, None, None, flags, &f, nullptr,
                                           true);
     return symtab->addUndefinedData(name, flags, &f);
   }
 
   if (objSym.isExecutable())
     return symtab->addDefinedFunction(name, flags, &f, nullptr);
   return symtab->addDefinedData(name, flags, &f, nullptr, 0, 0);
 }
 
 bool BitcodeFile::doneLTO = false;
 
 void BitcodeFile::parse() {
   if (doneLTO) {
     error(toString(this) + ": attempt to add bitcode file after LTO.");
     return;
   }
 
   obj = check(lto::InputFile::create(MemoryBufferRef(
       mb.getBuffer(), saver.save(archiveName + mb.getBufferIdentifier()))));
   Triple t(obj->getTargetTriple());
   if (!t.isWasm()) {
     error(toString(this) + ": machine type must be wasm32 or wasm64");
     return;
   }
   checkArch(t.getArch());
   std::vector<bool> keptComdats;
   for (StringRef s : obj->getComdatTable())
     keptComdats.push_back(symtab->addComdat(s));
 
   for (const lto::InputFile::Symbol &objSym : obj->symbols())
     symbols.push_back(createBitcodeSymbol(keptComdats, objSym, *this));
 }
 
 } // namespace wasm
 } // namespace lld

lld/wasm/Symbols.h

public:
   const WasmTableType *getTableType() const { return tableType; }
   void setLimits(const WasmLimits &limits);
 
   // Get/set the table number
   uint32_t getTableNumber() const;
   void setTableNumber(uint32_t number);
   bool hasTableNumber() const;
 
+  void preassignTableNumberZero();
+  bool hasPreassignedTableNumberZero() const;
+
 protected:
   TableSymbol(StringRef name, Kind k, uint32_t flags, InputFile *f,
               const WasmTableType *type)
       : Symbol(name, k, flags, f), tableType(type) {}
 
   const WasmTableType *tableType;
   uint32_t tableNumber = INVALID_INDEX;
 };

lld/wasm/Symbols.cpp

 uint32_t TableSymbol::getTableNumber() const {
   if (const auto *t = dyn_cast<DefinedTable>(this))
     return t->table->getTableNumber();
   assert(tableNumber != INVALID_INDEX);
   return tableNumber;
 }
 
 void TableSymbol::setTableNumber(uint32_t number) {
+  if (const auto *t = dyn_cast<DefinedTable>(this))
+    return t->table->setTableNumber(number);
   LLVM_DEBUG(dbgs() << "setTableNumber " << name << " -> " << number << "\n");
   assert(tableNumber == INVALID_INDEX);
   tableNumber = number;
 }
 
 bool TableSymbol::hasTableNumber() const {
   if (const auto *t = dyn_cast<DefinedTable>(this))
     return t->table->hasTableNumber();
   return tableNumber != INVALID_INDEX;
 }
 
+void TableSymbol::preassignTableNumberZero() {
+  if (!hasPreassignedTableNumberZero())
+    setTableNumber(0);
+}
+
+bool TableSymbol::hasPreassignedTableNumberZero() const {
+  if (hasTableNumber()) {
+    assert(getTableNumber() == 0);
+    return true;
+  }
+  return false;
+}
+
 DefinedTable::DefinedTable(StringRef name, uint32_t flags, InputFile *file,
                            InputTable *table)
     : TableSymbol(name, DefinedTableKind, flags, file,
                   table ? &table->getType() : nullptr),
       table(table) {}
 
 const OutputSectionSymbol *SectionSymbol::getOutputSectionSymbol() const {
   assert(section->outputSec && section->outputSec->sectionSym);

lld/wasm/SyntheticSections.cpp

 }
 
 void ImportSection::addImport(Symbol *sym) {
   assert(!isSealed);
   importedSymbols.emplace_back(sym);
   if (auto *f = dyn_cast<FunctionSymbol>(sym))
     f->setFunctionIndex(numImportedFunctions++);
   else if (auto *g = dyn_cast<GlobalSymbol>(sym))
     g->setGlobalIndex(numImportedGlobals++);
   else if (auto *e = dyn_cast<EventSymbol>(sym))
     e->setEventIndex(numImportedEvents++);
+  else {
+    // It is always possible to preserve preassigned table number 0 for imported
+    // tables, as imported tables are always numbered from 0.
+    TableSymbol *t = cast<TableSymbol>(sym);
+    if (t->hasPreassignedTableNumberZero())
+      assert(numImportedTables == 0);
     else
-    cast<TableSymbol>(sym)->setTableNumber(numImportedTables++);
+      t->setTableNumber(numImportedTables);
+    numImportedTables++;
+  }
 }
 
 void ImportSection::writeBody() {
   raw_ostream &os = bodyOutputStream;
 
   writeUleb128(os, getNumImports(), "import count");
 
   if (config->importMemory) {
void FunctionSection::addFunction(InputFunction *func) {
   if (!func->live)
     return;
   uint32_t functionIndex =
       out.importSec->getNumImportedFunctions() + inputFunctions.size();
   inputFunctions.emplace_back(func);
   func->setFunctionIndex(functionIndex);
 }
 
 void TableSection::writeBody() {
   raw_ostream &os = bodyOutputStream;
 
   writeUleb128(os, inputTables.size(), "table count");
   for (const InputTable *table : inputTables)
     writeTableType(os, table->getType());
 }
 
 void TableSection::addTable(InputTable *table) {
   if (!table->live)
     return;
   uint32_t tableNumber =
       out.importSec->getNumImportedTables() + inputTables.size();
   inputTables.push_back(table);
+  if (table->hasTableNumber()) {
+    // If a reftypes input has imported tables, any indirect function table
+    // referenced by an MVP input and preassigned to table number 0 will no
+    // longer work as the imported tables get the lowest table numbers (unless
+    // for some reason the indirect function table is itself imported).
+    assert(table->getTableNumber() == 0);
+    if (tableNumber != 0) {
+      // Of the declarations of __indirect_function_table in the various inputs,
+      // we don't know which one is responsible for preassigning the table
+      // number.  Probably here we should grovel the imports to determine which
+      // input file imported a table.
+      error(toString(table->file) + ": " + table->getName() +
+            ": Cannot "
+            "preassign table number 0 to indirect function table, because "
+            "object files compiled with -mattr=+reference-types declared "
+            "table imports.  Suggest recompiling all inputs with "
+            "-mattr=+reference-types.");
+    }
+  } else {
     table->setTableNumber(tableNumber);
   }
+}
 
 void MemorySection::writeBody() {
   raw_ostream &os = bodyOutputStream;
 
   bool hasMax = maxMemoryPages != 0 || config->sharedMemory;
   writeUleb128(os, 1, "memory count");
   unsigned flags = 0;
   if (hasMax)

lld/wasm/Writer.cpp

if (auto *f = dyn_cast<UndefinedFunction>(sym))
     return f->importName.hasValue();
   if (auto *t = dyn_cast<UndefinedTable>(sym))
     return t->importName.hasValue();
 
   return false;
 }
 
 void Writer::calculateImports() {
+  std::vector<Symbol *> deferredTables;
   for (Symbol *sym : symtab->getSymbols()) {
     if (!sym->isUndefined())
       continue;
     if (sym->isWeak() && !config->relocatable)
       continue;
     if (!sym->isLive())
       continue;
     if (!sym->isUsedInRegularObj)
       continue;
-    if (shouldImport(sym)) {
-      LLVM_DEBUG(dbgs() << "import: " << sym->getName() << "\n");
-      out.importSec->addImport(sym);
-    }
+    if (!shouldImport(sym))
+      continue;
+    if (auto *ts = dyn_cast<TableSymbol>(sym)) {
+      // Ensure that an indirect function table with a preassigned table number
+      // of 0 gets written as the first table import.
+      if (!ts->hasPreassignedTableNumberZero()) {
+        deferredTables.push_back(sym);
+        continue;
+      }
+    }
+  for (Symbol *sym : deferredTables) {
+    LLVM_DEBUG(dbgs() << "import: " << sym->getName() << "\n");
+    out.importSec->addImport(sym);
   }
 }
 
 void Writer::calculateExports() {
   if (config->relocatable)
     return;
 
   if (!config->relocatable && !config->importMemory)