[WebAssembly] call_indirect issues table number relocs

If the reference-types feature is enabled, call_indirect will explicitly
reference its corresponding function table via `TABLE_NUMBER`
relocations against a table symbol.

Also, as before, address-taken functions can also cause the function
table to be created, only with reference-types they additionally cause a
symbol table entry to be emitted.

We abuse the used-in-reloc flag on symbols to indicate which tables
should end up in the symbol table.  We do this because unfortunately
older wasm-ld will carp if it see a table symbol.

Differential Revision: https://reviews.llvm.org/D90948

Author: wingo

lld/test/wasm/compress-relocs.ll

@@ -1,5 +1,5 @@
 ; RUN: llc -filetype=obj %s -o %t.o
-; RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/call-indirect.s -o %t2.o
+; RUN: llvm-mc -mattr=+reference-types -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/call-indirect.s -o %t2.o
 ; RUN: wasm-ld --export-dynamic -o %t.wasm %t2.o %t.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 ; RUN: wasm-ld --export-dynamic -O2 -o %t-opt.wasm %t2.o %t.o
@@ -22,5 +22,5 @@ entry:
 
 ; ERROR: wasm-ld: error: --compress-relocations is incompatible with output debug information. Please pass --strip-debug or --strip-all
 
-; CHECK:    Body:            410028028088808000118080808000001A410028028488808000118180808000001A0B
+; CHECK:    Body:            41002802808880800011808080800080808080001A41002802848880800011818080800080808080001A0B
 ; COMPRESS: Body:            4100280280081100001A4100280284081101001A0B

llvm/include/llvm/MC/MCSymbolWasm.h

@@ -18,6 +18,7 @@ class MCSymbolWasm : public MCSymbol {
   bool IsWeak = false;
   bool IsHidden = false;
   bool IsComdat = false;
+  bool OmitFromLinkingSection = false;
   mutable bool IsUsedInInitArray = false;
   mutable bool IsUsedInGOT = false;
   Optional<StringRef> ImportModule;
@@ -75,6 +76,12 @@ class MCSymbolWasm : public MCSymbol {
   bool isComdat() const { return IsComdat; }
   void setComdat(bool isComdat) { IsComdat = isComdat; }
 
+  // wasm-ld understands a finite set of symbol types.  This flag allows the
+  // compiler to avoid emitting symbol table entries that would confuse the
+  // linker, unless the user specifically requests the feature.
+  bool omitFromLinkingSection() const { return OmitFromLinkingSection; }
+  void setOmitFromLinkingSection() { OmitFromLinkingSection = true; }
+
   bool hasImportModule() const { return ImportModule.hasValue(); }
   StringRef getImportModule() const {
     if (ImportModule.hasValue())

llvm/lib/MC/WasmObjectWriter.cpp

@@ -405,12 +405,17 @@ void WasmObjectWriter::writeHeader(const MCAssembler &Asm) {
 
 void WasmObjectWriter::executePostLayoutBinding(MCAssembler &Asm,
                                                 const MCAsmLayout &Layout) {
-  // As a stopgap measure until call_indirect instructions start explicitly
-  // referencing the indirect function table via TABLE_NUMBER relocs, ensure
-  // that the indirect function table import makes it to the output if anything
-  // in the compilation unit has caused it to be present.
-  if (auto *Sym = Asm.getContext().lookupSymbol("__indirect_function_table"))
-    Asm.registerSymbol(*Sym);
+  // Some compilation units require the indirect function table to be present
+  // but don't explicitly reference it.  This is the case for call_indirect
+  // without the reference-types feature, and also function bitcasts in all
+  // cases.  In those cases the __indirect_function_table has the
+  // WASM_SYMBOL_NO_STRIP attribute.  Here we make sure this symbol makes it to
+  // the assembler, if needed.
+  if (auto *Sym = Asm.getContext().lookupSymbol("__indirect_function_table")) {
+    const auto *WasmSym = static_cast<const MCSymbolWasm *>(Sym);
+    if (WasmSym->isNoStrip())
+      Asm.registerSymbol(*Sym);
+  }
 
   // Build a map of sections to the function that defines them, for use
   // in recordRelocation.
@@ -509,21 +514,18 @@ void WasmObjectWriter::recordRelocation(MCAssembler &Asm,
       Type == wasm::R_WASM_TABLE_INDEX_I32 ||
       Type == wasm::R_WASM_TABLE_INDEX_I64) {
     // TABLE_INDEX relocs implicitly use the default indirect function table.
+    // We require the function table to have already been defined.
     auto TableName = "__indirect_function_table";
     MCSymbolWasm *Sym = cast_or_null<MCSymbolWasm>(Ctx.lookupSymbol(TableName));
-    if (Sym) {
-      if (!Sym->isFunctionTable())
-        Ctx.reportError(
-            Fixup.getLoc(),
-            "symbol '__indirect_function_table' is not a function table");
+    if (!Sym || !Sym->isFunctionTable()) {
+      Ctx.reportError(
+          Fixup.getLoc(),
+          "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();
+      // Ensure that __indirect_function_table reaches the output.
+      Sym->setNoStrip();
+      Asm.registerSymbol(*Sym);
     }
-    Sym->setUsedInReloc();
-    Asm.registerSymbol(*Sym);
   }
 
   // Relocation other than R_WASM_TYPE_INDEX_LEB are required to be
@@ -1211,6 +1213,9 @@ static bool isInSymtab(const MCSymbolWasm &Sym) {
   if (Sym.isSection())
     return false;
 
+  if (Sym.omitFromLinkingSection())
+    return false;
+
   return true;
 }
 
@@ -1674,10 +1679,6 @@ uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,
       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;

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

@@ -171,9 +171,6 @@ struct WebAssemblyOperand : public MCParsedAsmOperand {
 
 static MCSymbolWasm *GetOrCreateFunctionTableSymbol(MCContext &Ctx,
                                                     const StringRef &Name) {
-  // FIXME: Duplicates functionality from
-  // MC/WasmObjectWriter::recordRelocation, as well as WebAssemblyCodegen's
-  // WebAssembly:getOrCreateFunctionTableSymbol.
   MCSymbolWasm *Sym = cast_or_null<MCSymbolWasm>(Ctx.lookupSymbol(Name));
   if (Sym) {
     if (!Sym->isFunctionTable())
@@ -223,6 +220,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
   };
   std::vector<NestingType> NestingStack;
 
+  MCSymbolWasm *DefaultFunctionTable = nullptr;
   MCSymbol *LastFunctionLabel = nullptr;
 
 public:
@@ -233,6 +231,15 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
     setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
   }
 
+  void Initialize(MCAsmParser &Parser) override {
+    MCAsmParserExtension::Initialize(Parser);
+
+    DefaultFunctionTable = GetOrCreateFunctionTableSymbol(
+        getContext(), "__indirect_function_table");
+    if (!STI->checkFeatures("+reference-types"))
+      DefaultFunctionTable->setOmitFromLinkingSection();
+  }
+
 #define GET_ASSEMBLER_HEADER
 #include "WebAssemblyGenAsmMatcher.inc"
 
@@ -480,6 +487,39 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
         WebAssemblyOperand::IntOp{static_cast<int64_t>(BT)}));
   }
 
+  bool addFunctionTableOperand(OperandVector &Operands, MCSymbolWasm *Sym,
+                               SMLoc StartLoc, SMLoc EndLoc) {
+    const auto *Val = MCSymbolRefExpr::create(Sym, getContext());
+    Operands.push_back(std::make_unique<WebAssemblyOperand>(
+        WebAssemblyOperand::Symbol, StartLoc, EndLoc,
+        WebAssemblyOperand::SymOp{Val}));
+    return false;
+  }
+
+  bool addFunctionTableOperand(OperandVector &Operands, StringRef TableName,
+                               SMLoc StartLoc, SMLoc EndLoc) {
+    return addFunctionTableOperand(
+        Operands, GetOrCreateFunctionTableSymbol(getContext(), TableName),
+        StartLoc, EndLoc);
+  }
+
+  bool addDefaultFunctionTableOperand(OperandVector &Operands, SMLoc StartLoc,
+                                      SMLoc EndLoc) {
+    if (STI->checkFeatures("+reference-types")) {
+      return addFunctionTableOperand(Operands, DefaultFunctionTable, StartLoc,
+                                     EndLoc);
+    } else {
+      // For the MVP there is at most one table whose number is 0, but we can't
+      // write a table symbol or issue relocations.  Instead we just ensure the
+      // table is live and write a zero.
+      getStreamer().emitSymbolAttribute(DefaultFunctionTable, MCSA_NoDeadStrip);
+      Operands.push_back(std::make_unique<WebAssemblyOperand>(
+          WebAssemblyOperand::Integer, StartLoc, EndLoc,
+          WebAssemblyOperand::IntOp{0}));
+      return false;
+    }
+  }
+
   bool ParseInstruction(ParseInstructionInfo & /*Info*/, StringRef Name,
                         SMLoc NameLoc, OperandVector &Operands) override {
     // Note: Name does NOT point into the sourcecode, but to a local, so
@@ -516,6 +556,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
     bool ExpectBlockType = false;
     bool ExpectFuncType = false;
     bool ExpectHeapType = false;
+    bool ExpectFunctionTable = false;
     if (Name == "block") {
       push(Block);
       ExpectBlockType = true;
@@ -562,15 +603,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
         return true;
     } else if (Name == "call_indirect" || Name == "return_call_indirect") {
       ExpectFuncType = true;
-      // Ensure that the object file has a __indirect_function_table import, as
-      // we call_indirect against it.
-      auto &Ctx = getStreamer().getContext();
-      MCSymbolWasm *Sym =
-          GetOrCreateFunctionTableSymbol(Ctx, "__indirect_function_table");
-      // Until call_indirect emits TABLE_NUMBER relocs against this symbol, mark
-      // it as NO_STRIP so as to ensure that the indirect function table makes
-      // it to linked output.
-      Sym->setNoStrip();
+      ExpectFunctionTable = true;
     } else if (Name == "ref.null") {
       ExpectHeapType = true;
     }
@@ -586,7 +619,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
         return true;
       // Got signature as block type, don't need more
       ExpectBlockType = false;
-      auto &Ctx = getStreamer().getContext();
+      auto &Ctx = getContext();
       // The "true" here will cause this to be a nameless symbol.
       MCSymbol *Sym = Ctx.createTempSymbol("typeindex", true);
       auto *WasmSym = cast<MCSymbolWasm>(Sym);
@@ -598,6 +631,16 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
       Operands.push_back(std::make_unique<WebAssemblyOperand>(
           WebAssemblyOperand::Symbol, Loc.getLoc(), Loc.getEndLoc(),
           WebAssemblyOperand::SymOp{Expr}));
+
+      // Allow additional operands after the signature, notably for
+      // call_indirect against a named table.
+      if (Lexer.isNot(AsmToken::EndOfStatement)) {
+        if (expect(AsmToken::Comma, ","))
+          return true;
+        if (Lexer.is(AsmToken::EndOfStatement)) {
+          return error("Unexpected trailing comma");
+        }
+      }
     }
 
     while (Lexer.isNot(AsmToken::EndOfStatement)) {
@@ -623,6 +666,11 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
               WebAssemblyOperand::Integer, Id.getLoc(), Id.getEndLoc(),
               WebAssemblyOperand::IntOp{static_cast<int64_t>(HeapType)}));
           Parser.Lex();
+        } else if (ExpectFunctionTable) {
+          if (addFunctionTableOperand(Operands, Id.getString(), Id.getLoc(),
+                                      Id.getEndLoc()))
+            return true;
+          Parser.Lex();
         } else {
           // Assume this identifier is a label.
           const MCExpr *Val;
@@ -689,6 +737,12 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
       // Support blocks with no operands as default to void.
       addBlockTypeOperand(Operands, NameLoc, WebAssembly::BlockType::Void);
     }
+    if (ExpectFunctionTable && Operands.size() == 2) {
+      // If call_indirect doesn't specify a target table, supply one.
+      if (addDefaultFunctionTableOperand(Operands, NameLoc,
+                                         SMLoc::getFromPointer(Name.end())))
+        return true;
+    }
     Parser.Lex();
     return false;
   }

llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp

@@ -68,7 +68,7 @@ void WebAssemblyInstPrinter::printInst(const MCInst *MI, uint64_t Address,
     for (auto I = Start, E = MI->getNumOperands(); I < E; ++I) {
       if (MI->getOpcode() == WebAssembly::CALL_INDIRECT &&
           I - Start == NumVariadicDefs) {
-        // Skip type and flags arguments when printing for tests
+        // Skip type and table arguments when printing for tests.
         ++I;
         continue;
       }

llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

@@ -23,6 +23,7 @@
 #include "WebAssemblyMachineFunctionInfo.h"
 #include "WebAssemblyRegisterInfo.h"
 #include "WebAssemblyTargetMachine.h"
+#include "WebAssemblyUtilities.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/BinaryFormat/Wasm.h"
@@ -171,19 +172,25 @@ MCSymbolWasm *WebAssemblyAsmPrinter::getMCSymbolForFunction(
 
 void WebAssemblyAsmPrinter::emitEndOfAsmFile(Module &M) {
   for (auto &It : OutContext.getSymbols()) {
-    // Emit a .globaltype and .eventtype declaration.
+    // Emit .globaltype, .eventtype, or .tabletype declarations.
     auto Sym = cast<MCSymbolWasm>(It.getValue());
     if (Sym->getType() == wasm::WASM_SYMBOL_TYPE_GLOBAL)
       getTargetStreamer()->emitGlobalType(Sym);
     else if (Sym->getType() == wasm::WASM_SYMBOL_TYPE_EVENT)
       getTargetStreamer()->emitEventType(Sym);
+    else if (Sym->getType() == wasm::WASM_SYMBOL_TYPE_TABLE)
+      getTargetStreamer()->emitTableType(Sym);
   }
 
   DenseSet<MCSymbol *> InvokeSymbols;
+  bool HasAddressTakenFunction = false;
   for (const auto &F : M) {
     if (F.isIntrinsic())
       continue;
 
+    if (F.hasAddressTaken())
+      HasAddressTakenFunction = true;
+
     // Emit function type info for all undefined functions
     if (F.isDeclarationForLinker()) {
       SmallVector<MVT, 4> Results;
@@ -242,6 +249,18 @@ void WebAssemblyAsmPrinter::emitEndOfAsmFile(Module &M) {
     }
   }
 
+  // When a function's address is taken, a TABLE_INDEX relocation is emitted
+  // against the function symbol at the use site.  However the relocation
+  // doesn't explicitly refer to the table.  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.
+  if (HasAddressTakenFunction) {
+    MCSymbolWasm *FunctionTable =
+        WebAssembly::getOrCreateFunctionTableSymbol(OutContext, Subtarget);
+    OutStreamer->emitSymbolAttribute(FunctionTable, MCSA_NoDeadStrip);
+  }
+
   for (const auto &G : M.globals()) {
     if (!G.hasInitializer() && G.hasExternalLinkage()) {
       if (G.getValueType()->isSized()) {

llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp

@@ -869,19 +869,20 @@ bool WebAssemblyFastISel::selectCall(const Instruction *I) {
   if (IsDirect) {
     MIB.addGlobalAddress(Func);
   } else {
-    // Add placeholders for the type index and immediate flags
+    // Placehoder for the type index.
     MIB.addImm(0);
-    MIB.addImm(0);
-
-    // Ensure that the object file has a __indirect_function_table import, as we
-    // call_indirect against it.
-    MCSymbolWasm *Sym = WebAssembly::getOrCreateFunctionTableSymbol(
-        MF->getMMI().getContext(), "__indirect_function_table");
-    // Until call_indirect emits TABLE_NUMBER relocs against this symbol, mark
-    // it as NO_STRIP so as to ensure that the indirect function table makes it
-    // to linked output.
-    Sym->setNoStrip();
-
+    // The table into which this call_indirect indexes.
+    MCSymbolWasm *Table = WebAssembly::getOrCreateFunctionTableSymbol(
+        MF->getMMI().getContext(), Subtarget);
+    if (Subtarget->hasReferenceTypes()) {
+      MIB.addSym(Table);
+    } else {
+      // Otherwise for the MVP there is at most one table whose number is 0, but
+      // we can't write a table symbol or issue relocations.  Instead we just
+      // ensure the table is live.
+      Table->setNoStrip();
+      MIB.addImm(0);
+    }
     // See if we must truncate the function pointer.
     // CALL_INDIRECT takes an i32, but in wasm64 we represent function pointers
     // as 64-bit for uniformity with other pointer types.