[llvm-branch-commits] [llvm] 9ad83fd - [WebAssembly] call_indirect causes indirect function table import

Andy Wingo via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Jan 5 02:28:25 PST 2021


Author: Andy Wingo
Date: 2021-01-05T11:09:24+01:00
New Revision: 9ad83fd6dc46330dcdea8af36c435061a31ac0c5

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

LOG: [WebAssembly] call_indirect causes indirect function table import

For wasm-ld table linking work to proceed, object files should indicate
if they use an indirect function table.  In the future this will be done
by the usual symbols and relocations mechanism, but until that support
lands in the linker, the presence of an `__indirect_function_table` in
the object file's import section shows that the object file needs an
indirect function table.

Prior to https://reviews.llvm.org/D91637, this condition was met by all
object files residualizing an `__indirect_function_table` import.

Since https://reviews.llvm.org/D91637, the intention has been that only
those object files needing an indirect function table would have the
`__indirect_function_table` import.  However, we missed the case of
object files which use the table via `call_indirect` but which
themselves do not declare any indirect functions.

This changeset makes it so that when we lower a call to `call_indirect`,
that we ensure that a `__indirect_function_table` symbol is present and
that it will be propagated to the linker.

A followup patch will revise this mechanism to make an explicit link
between `call_indirect` and its associated indirect function table; see
https://reviews.llvm.org/D90948.

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

Added: 
    llvm/test/CodeGen/WebAssembly/call-indirect.ll

Modified: 
    llvm/lib/MC/WasmObjectWriter.cpp
    llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
    llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
    llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
    llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp
    llvm/lib/Target/WebAssembly/WebAssemblyUtilities.h
    llvm/test/MC/WebAssembly/type-index.s

Removed: 
    


################################################################################
diff  --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
index c8d2e5dbdbdc..683678b70ebc 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -405,6 +405,13 @@ 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);
+
   // Build a map of sections to the function that defines them, for use
   // in recordRelocation.
   for (const MCSymbol &S : Asm.symbols()) {

diff  --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index 55f79bafe414..18d7b642e044 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -160,6 +160,24 @@ 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())
+      Ctx.reportError(SMLoc(), "symbol is not a wasm funcref table");
+  } else {
+    Sym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(Name));
+    Sym->setFunctionTable();
+    // The default function table is synthesized by the linker.
+    Sym->setUndefined();
+  }
+  return Sym;
+}
+
 class WebAssemblyAsmParser final : public MCTargetAsmParser {
   MCAsmParser &Parser;
   MCAsmLexer &Lexer;
@@ -531,6 +549,15 @@ 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();
     } else if (Name == "ref.null") {
       ExpectHeapType = true;
     }

diff  --git a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
index 949564e2d542..903a8e2a3e2d 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
@@ -20,12 +20,14 @@
 #include "WebAssemblyMachineFunctionInfo.h"
 #include "WebAssemblySubtarget.h"
 #include "WebAssemblyTargetMachine.h"
+#include "WebAssemblyUtilities.h"
 #include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/CodeGen/FastISel.h"
 #include "llvm/CodeGen/FunctionLoweringInfo.h"
 #include "llvm/CodeGen/MachineConstantPool.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
@@ -878,6 +880,15 @@ bool WebAssemblyFastISel::selectCall(const Instruction *I) {
     // Add placeholders for the type index and immediate flags
     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();
   }
 
   for (unsigned ArgReg : Args)

diff  --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index 20b3e2e07e77..1abc610dac11 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -16,6 +16,7 @@
 #include "WebAssemblyMachineFunctionInfo.h"
 #include "WebAssemblySubtarget.h"
 #include "WebAssemblyTargetMachine.h"
+#include "WebAssemblyUtilities.h"
 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/CodeGen/CallingConvLower.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
@@ -477,6 +478,15 @@ static MachineBasicBlock *LowerCallResults(MachineInstr &CallResults,
   if (IsIndirect) {
     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.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();
   }
 
   for (auto Use : CallParams.uses())

diff  --git a/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp
index bc2bb4fd6935..dd0f6ce03b3d 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp
@@ -15,6 +15,7 @@
 #include "WebAssemblyMachineFunctionInfo.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineLoopInfo.h"
+#include "llvm/MC/MCContext.h"
 using namespace llvm;
 
 const char *const WebAssembly::ClangCallTerminateFn = "__clang_call_terminate";
@@ -96,3 +97,21 @@ const MachineOperand &WebAssembly::getCalleeOp(const MachineInstr &MI) {
     llvm_unreachable("Not a call instruction");
   }
 }
+
+MCSymbolWasm *
+WebAssembly::getOrCreateFunctionTableSymbol(MCContext &Ctx,
+                                            const StringRef &Name) {
+  // FIXME: Duplicates functionality from
+  // MC/WasmObjectWriter::recordRelocation.
+  MCSymbolWasm *Sym = cast_or_null<MCSymbolWasm>(Ctx.lookupSymbol(Name));
+  if (Sym) {
+    if (!Sym->isFunctionTable())
+      Ctx.reportError(SMLoc(), "symbol is not a wasm funcref table");
+  } else {
+    Sym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(Name));
+    Sym->setFunctionTable();
+    // The default function table is synthesized by the linker.
+    Sym->setUndefined();
+  }
+  return Sym;
+}

diff  --git a/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.h b/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.h
index 8fa794c0b932..5af5c53f94bb 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.h
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.h
@@ -19,6 +19,9 @@ namespace llvm {
 
 class MachineInstr;
 class MachineOperand;
+class MCContext;
+class MCSymbolWasm;
+class StringRef;
 class WebAssemblyFunctionInfo;
 
 namespace WebAssembly {
@@ -37,6 +40,11 @@ extern const char *const PersonalityWrapperFn;
 /// instruction.
 const MachineOperand &getCalleeOp(const MachineInstr &MI);
 
+/// Returns the operand number of a callee, assuming the argument is a call
+/// instruction.
+MCSymbolWasm *getOrCreateFunctionTableSymbol(MCContext &Ctx,
+                                             const StringRef &Name);
+
 } // end namespace WebAssembly
 
 } // end namespace llvm

diff  --git a/llvm/test/CodeGen/WebAssembly/call-indirect.ll b/llvm/test/CodeGen/WebAssembly/call-indirect.ll
new file mode 100644
index 000000000000..3ce74ddfd30e
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/call-indirect.ll
@@ -0,0 +1,28 @@
+; RUN: llc < %s -asm-verbose=false -O2 | FileCheck --check-prefix=CHECK %s
+; RUN: llc < %s -asm-verbose=false -O2 --filetype=obj | obj2yaml | FileCheck --check-prefix=OBJ %s
+
+; Test that compilation units with call_indirect but without any
+; function pointer declarations still get a table.
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+; CHECK-LABEL: call_indirect_void:
+; CHECK-NEXT: .functype call_indirect_void (i32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: call_indirect () -> ()
+; CHECK-NEXT: end_function
+define void @call_indirect_void(void ()* %callee) {
+  call void %callee()
+  ret void
+}
+
+; OBJ:    Imports:
+; OBJ-NEXT:      - Module:          env
+; OBJ-NEXT:        Field:           __linear_memory
+; OBJ-NEXT:        Kind:            MEMORY
+; OBJ-NEXT:        Memory:
+; OBJ-NEXT:          Initial:         0x0
+; OBJ-NEXT:      - Module:          env
+; OBJ-NEXT:        Field:           __indirect_function_table
+; OBJ-NEXT:        Kind:            TABLE

diff  --git a/llvm/test/MC/WebAssembly/type-index.s b/llvm/test/MC/WebAssembly/type-index.s
index 501b71d6d6a8..298e5e2f9de3 100644
--- a/llvm/test/MC/WebAssembly/type-index.s
+++ b/llvm/test/MC/WebAssembly/type-index.s
@@ -38,6 +38,14 @@ test0:
 # BIN-NEXT:         Kind:            MEMORY
 # BIN-NEXT:         Memory:
 # BIN-NEXT:           Initial:         0x0
+# BIN-NEXT:       - Module:          env
+# BIN-NEXT:         Field:           __indirect_function_table
+# BIN-NEXT:         Kind:            TABLE
+# BIN-NEXT:         Table:
+# BIN-NEXT:           Index:           0
+# BIN-NEXT:           ElemType:        FUNCREF
+# BIN-NEXT:           Limits:
+# BIN-NEXT:             Initial:         0x0
 # BIN-NEXT:   - Type:            FUNCTION
 # BIN-NEXT:     FunctionTypes:   [ 0 ]
 # BIN-NEXT:   - Type:            CODE


        


More information about the llvm-branch-commits mailing list