[WebAssembly] MC: Fix value of R_WEBASSEMBLY_TABLE_INDEX relocations

[sbc100]

Summary

Previously we were writing the value function index space
value but for these types of relocations we want to be
writing the table element index space value.

Add a test case for these relocation types that fails
without this change.

[sbc100]

Add comments

[sbc100]

• revert part

[sbc100]

• spelling fix

[sbc100]

• check for elem section

[sbc100]

ping?

[jgravelle-google]

Looks fine but I don't understand it really. @sunfish ?

lib/MC/WasmObjectWriter.cpp

638 ↗

(On Diff #101634)

Should probably either maintain the horizontal alignment of these comments, or remove the alignment for both

[sbc100]

• Merge remote-tracking branch 'origin/master' into fix_table_index_reloc
• Merge remote-tracking branch 'origin/master' into fix_table_index_reloc
• Merge remote-tracking branch 'origin/master' into fix_table_index_reloc
• Merge remote-tracking branch 'origin/master' into fix_table_index_reloc
• update comments

[sbc100]

• remove extra newline

[sbc100]

@sunfish, could you that a look? I think this should be fairly non-controversial.

[sunfish]

lib/MC/WasmObjectWriter.cpp

156 ↗

(On Diff #102055)

Just curious: what's behind this change? At a quick glance, MCSymbols's operator<< (ultimately, MCSymbol::print) seems pretty sane.

[sbc100]

lib/MC/WasmObjectWriter.cpp

156 ↗

(On Diff #102055)

Oh, I see. I probably should just deference the pointer (Not really part of this change anyway).

[sbc100]

• revert change to symbol printing

llvm/trunk/lib/MC/WasmObjectWriter.cpp

class WasmObjectWriter : public MCObjectWriter {
   std::vector<WasmRelocationEntry> CodeRelocations;
 
   // Relocations for fixing up references in the data section.
   std::vector<WasmRelocationEntry> DataRelocations;
 
   // Index values to use for fixing up call_indirect type indices.
   // Maps function symbols to the index of the type of the function
   DenseMap<const MCSymbolWasm *, uint32_t> TypeIndices;
-
+  // Maps function symbols to the table element index space. Used
+  // for TABLE_INDEX relocation types (i.e. address taken functions).
+  DenseMap<const MCSymbolWasm *, uint32_t> IndirectSymbolIndices;
+  // Maps function/global symbols to the function/global index space.
   DenseMap<const MCSymbolWasm *, uint32_t> SymbolIndices;
 
   DenseMap<WasmFunctionType, int32_t, WasmFunctionTypeDenseMapInfo>
       FunctionTypeIndices;
 
   // TargetObjectWriter wrappers.
   bool is64Bit() const { return TargetObjectWriter->is64Bit(); }
   unsigned getRelocType(MCContext &Ctx, const MCValue &Target,

 private:
   ~WasmObjectWriter() override;
 
   void reset() override {
     CodeRelocations.clear();
     DataRelocations.clear();
     TypeIndices.clear();
     SymbolIndices.clear();
+    IndirectSymbolIndices.clear();
     FunctionTypeIndices.clear();
     MCObjectWriter::reset();
   }
 
   void writeHeader(const MCAssembler &Asm);
 
   void recordRelocation(MCAssembler &Asm, const MCAsmLayout &Layout,
                         const MCFragment *Fragment, const MCFixup &Fixup,
                         MCValue Target, bool &IsPCRel,
                         uint64_t &FixedValue) override;
 
   void executePostLayoutBinding(MCAssembler &Asm,
                                 const MCAsmLayout &Layout) override;
 
   void writeObject(MCAssembler &Asm, const MCAsmLayout &Layout) override;
 
   void writeValueType(wasm::ValType Ty) {
     encodeSLEB128(int32_t(Ty), getStream());
   }
 
   void writeTypeSection(const SmallVector<WasmFunctionType, 4> &FunctionTypes);
   void writeImportSection(const SmallVector<WasmImport, 4> &Imports);
   void writeFunctionSection(const SmallVector<WasmFunction, 4> &Functions);
-  void writeTableSection(const SmallVector<uint32_t, 4> &TableElems);
+  void writeTableSection(uint32_t NumElements);
   void writeMemorySection(const SmallVector<char, 0> &DataBytes);
   void writeGlobalSection(const SmallVector<WasmGlobal, 4> &Globals);
   void writeExportSection(const SmallVector<WasmExport, 4> &Exports);
   void writeElemSection(const SmallVector<uint32_t, 4> &TableElems);
   void writeCodeSection(const MCAssembler &Asm, const MCAsmLayout &Layout,
                         const SmallVector<WasmFunction, 4> &Functions);
   uint64_t
   writeDataSection(const SmallVector<char, 0> &DataBytes);
static uint32_t ProvisionalValue(const WasmRelocationEntry &RelEntry) {
   uint32_t Value = Address;
 
   return Value;
 }
 
 uint32_t WasmObjectWriter::getRelocationIndexValue(
     const WasmRelocationEntry &RelEntry) {
   switch (RelEntry.Type) {
-  case wasm::R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
   case wasm::R_WEBASSEMBLY_TABLE_INDEX_SLEB:
   case wasm::R_WEBASSEMBLY_TABLE_INDEX_I32:
+    assert(IndirectSymbolIndices.count(RelEntry.Symbol));
+    return IndirectSymbolIndices[RelEntry.Symbol];
+  case wasm::R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
   case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_LEB:
   case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_SLEB:
   case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_I32:
     assert(SymbolIndices.count(RelEntry.Symbol));
     return SymbolIndices[RelEntry.Symbol];
   case wasm::R_WEBASSEMBLY_TYPE_INDEX_LEB:
     assert(TypeIndices.count(RelEntry.Symbol));
     return TypeIndices[RelEntry.Symbol];
void WasmObjectWriter::writeFunctionSection(
 
   encodeULEB128(Functions.size(), getStream());
   for (const WasmFunction &Func : Functions)
     encodeULEB128(Func.Type, getStream());
 
   endSection(Section);
 }
 
-void WasmObjectWriter::writeTableSection(
-    const SmallVector<uint32_t, 4> &TableElems) {
+void WasmObjectWriter::writeTableSection(uint32_t NumElements) {
   // For now, always emit the table section, since indirect calls are not
   // valid without it. In the future, we could perhaps be more clever and omit
   // it if there are no indirect calls.
+
   SectionBookkeeping Section;
   startSection(Section, wasm::WASM_SEC_TABLE);
 
-  // The number of tables, fixed to 1 for now.
-  encodeULEB128(1, getStream());
-
-  encodeSLEB128(wasm::WASM_TYPE_ANYFUNC, getStream());
-
-  encodeULEB128(0, getStream());                 // flags
-  encodeULEB128(TableElems.size(), getStream()); // initial
+  encodeULEB128(1, getStream());                       // The number of tables.
+                                                       // Fixed to 1 for now.
+  encodeSLEB128(wasm::WASM_TYPE_ANYFUNC, getStream()); // Type of table
+  encodeULEB128(0, getStream());                       // flags
+  encodeULEB128(NumElements, getStream());             // initial
 
   endSection(Section);
 }
 
 void WasmObjectWriter::writeMemorySection(
     const SmallVector<char, 0> &DataBytes) {
   // For now, always emit the memory section, since loads and stores are not
   // valid without it. In the future, we could perhaps be more clever and omit
if (WS.isFunction()) {
         SymbolIndices[&WS] = Index;
         Functions.push_back(Func);
       } else {
         // An import; the index was assigned above.
         Index = SymbolIndices.find(&WS)->second;
       }
 
       // If needed, prepare the function to be called indirectly.
-      if (IsAddressTaken.count(&WS))
+      if (IsAddressTaken.count(&WS)) {
+        IndirectSymbolIndices[&WS] = TableElems.size();
         TableElems.push_back(Index);
+      }
     } else {
       if (WS.isTemporary() && !WS.getSize())
         continue;
 
       if (WS.isDefined(false)) {
         if (WS.getOffset() != 0)
           report_fatal_error("data sections must contain one variable each: " +
                              WS.getName());
void WasmObjectWriter::writeObject(MCAssembler &Asm,
   }
 
   // Write out the Wasm header.
   writeHeader(Asm);
 
   writeTypeSection(FunctionTypes);
   writeImportSection(Imports);
   writeFunctionSection(Functions);
-  writeTableSection(TableElems);
+  writeTableSection(TableElems.size());
   writeMemorySection(DataBytes);
   writeGlobalSection(Globals);
   writeExportSection(Exports);
   // TODO: Start Section
   writeElemSection(TableElems);
   writeCodeSection(Asm, Layout, Functions);
   uint64_t DataSectionHeaderSize = writeDataSection(DataBytes);
   writeNameSection(Functions, Imports, NumFuncImports);

llvm/trunk/test/MC/WebAssembly/func-address.ll

+; RUN: llc -mtriple wasm32-unknown-unknown-wasm -O2 -filetype=obj %s -o - | llvm-readobj -r -s -expand-relocs | FileCheck %s
+
+declare i32 @import1()
+declare i32 @import2()
+declare i32 @import3()
+
+; call the imports to make sure they are included in the imports section
+define hidden void @call_imports() #0 {
+entry:
+  %call = call i32 @import1()
+  %call1 = call i32 @import2()
+  ret void
+}
+
+; take the address of the third import.  This should generate a TABLE_INDEX
+; relocation with index of 0 since its the first and only address taken
+; function.
+define hidden void @call_indirect() #0 {
+entry:
+  %adr = alloca i32 ()*, align 4
+  store i32 ()* @import3, i32 ()** %adr, align 4
+  ret void
+}
+
+; CHECK:   Section {
+; CHECK:     Type: ELEM (0x9)
+; CHECK:     Size: 7
+; CHECK:     Offset: 165
+; CHECK:   }
+
+; CHECK: Relocations [
+; CHECK:   Section (9) CODE {
+; CHECK:     Relocation {
+; CHECK:       Type: R_WEBASSEMBLY_FUNCTION_INDEX_LEB (0)
+; CHECK:       Offset: 0x4
+; CHECK:       Index: 0x0
+; CHECK:     }
+; CHECK:     Relocation {
+; CHECK:       Type: R_WEBASSEMBLY_FUNCTION_INDEX_LEB (0)
+; CHECK:       Offset: 0xB
+; CHECK:       Index: 0x1
+; CHECK:     }
+; CHECK:     Relocation {
+; CHECK:       Type: R_WEBASSEMBLY_TABLE_INDEX_SLEB (1)
+; CHECK:       Offset: 0x1A
+; CHECK:       Index: 0x0
+; CHECK:     }
+; CHECK:   }