[WebAssembly] Symbol changes #2: Table relocs, LLVM

[ncw]

Summary

Second chunk split out of D41954.

Must be committed simultaneously with LLD change, D42081.

Changes:

• Change the binary format for table relocations. Formerly, if you wanted to reloc against function "2", you'd put "2" in the table, then put the table index in the relocation. Now, you just put "2" in the relocation. For Clang output (or relocatable LLD output) the actual table is completely ignored - the relocation specifies which function.
• Test expectations are updated accordingly. The new relocation values should match the old values from the ELEM section.
• I haven't changed anything about the ELEM section itself. It's now unused, and has the same value it always has. This can freely changed in a future commit, since it now does nothing. (The next commit will change it, so that "relocatable" and non-relocatable Wasm objects have identical ELEM section. There's no need at all for the current artificial differences, so for the dummy values the Clang outputs there, we may as well write out the same values that LLD would.)

Reason for change

• Due to pesky aliases, there can be more Symbols in a wasm file than Functions. I want in a future revision to get rid of the duplicate imports used to work around this problem.
• But if the imports are removed, then it will no longer be legal to put the symbol index in the ELEM table! This is because Wasm ELEM sections contain a vector of funcid, ie the indexes in the ELEM section have to be valid Wasm functions. So we won't be able to put the generalised symbol indexes in there.
• Hence, this change puts the symbol index directly in the relocation itself. This is a necessary change for getting rid of the duplicate imports in the Wasm object files.

[ncw]

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

110 ↗

(On Diff #129883)

Values here have changed from 0, 1 (the table indexes) to the values [ 5, 7 ] at those indexes. Similarly for other test output.

[sbc100]

lib/MC/WasmObjectWriter.cpp

608 ↗

(On Diff #129883)

Can you just use getRelocationIndexValue() here and drop getProvisionalTableIndex() completely?

[ncw]

lib/MC/WasmObjectWriter.cpp

608 ↗

(On Diff #129883)

You're right that getProvisionalTableIndex doesn't do much at the moment...

The reason I kept the existing behaviour is to avoid changing the test output for the ELEM section in this commit. The next review (#3) then modifies getProvisionalTableIndex further. So this is just an intermediate stage to keep each commit in the chain from doing too many changes at once.

[ncw]

lib/MC/WasmObjectWriter.cpp

520 ↗

(On Diff #129883)

NB. This todo is addressed in the next patch in the series, it's not that this patch is incomplete.

[ncw]

Update:

• Straightforward rebase

[ncw]

Updated:

• Another rebase after #4c and #4d landed

llvm/trunk/lib/MC/WasmObjectWriter.cpp

if (Symbol.isVariable()) {
     const MCExpr *Expr = Symbol.getVariableValue();
     auto *Inner = cast<MCSymbolRefExpr>(Expr);
     return cast<MCSymbolWasm>(&Inner->getSymbol());
   }
   return &Symbol;
 }
 
 // Compute a value to write into the code at the location covered
-// by RelEntry. This value isn't used by the static linker, since
-// we have addends; it just serves to make the code more readable
-// and to make standalone wasm modules directly usable.
+// by RelEntry. This value isn't used by the static linker; it just serves
+// to make the object format more readable and more likely to be directly
+// useable.
 uint32_t
 WasmObjectWriter::getProvisionalValue(const WasmRelocationEntry &RelEntry) {
-  const MCSymbolWasm *Sym = ResolveSymbol(*RelEntry.Symbol);
 
+  switch (RelEntry.Type) {
+  case wasm::R_WEBASSEMBLY_TABLE_INDEX_SLEB:
+  case wasm::R_WEBASSEMBLY_TABLE_INDEX_I32:
+    // Provitional value is the indirect symbol index
+    if (!IndirectSymbolIndices.count(RelEntry.Symbol))
+      report_fatal_error("symbol not found in table index space: " +
+                         RelEntry.Symbol->getName());
+    return IndirectSymbolIndices[RelEntry.Symbol];
+  case wasm::R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
+  case wasm::R_WEBASSEMBLY_TYPE_INDEX_LEB:
+  case wasm::R_WEBASSEMBLY_GLOBAL_INDEX_LEB:
+    // Provitional value is function/type/global index itself
+    return getRelocationIndexValue(RelEntry);
+  case wasm::R_WEBASSEMBLY_MEMORY_ADDR_LEB:
+  case wasm::R_WEBASSEMBLY_MEMORY_ADDR_I32:
+  case wasm::R_WEBASSEMBLY_MEMORY_ADDR_SLEB: {
+    // Provitional value is address of the global
+    const MCSymbolWasm *Sym = ResolveSymbol(*RelEntry.Symbol);
     // For undefined symbols, use zero
     if (!Sym->isDefined())
       return 0;
 
     uint32_t GlobalIndex = SymbolIndices[Sym];
     const WasmGlobal& Global = Globals[GlobalIndex - NumGlobalImports];
     uint64_t Address = Global.InitialValue + RelEntry.Addend;
 
     // Ignore overflow. LLVM allows address arithmetic to silently wrap.
-  uint32_t Value = Address;
-
-  return Value;
+    return Address;
+  }
+  default:
+    llvm_unreachable("invalid relocation type");
+  }
 }
 
 static void addData(SmallVectorImpl<char> &DataBytes,
                     MCSectionWasm &DataSection) {
   DEBUG(errs() << "addData: " << DataSection.getSectionName() << "\n");
 
   DataBytes.resize(alignTo(DataBytes.size(), DataSection.getAlignment()));
 
static void addData(SmallVectorImpl<char> &DataBytes,
   // fragment, otherwise the linker cannot map symbols to a unique
   // data segment.  This can be triggered by zero-sized structs
   // See: test/MC/WebAssembly/bss.ll
   if (LastFragmentSize == 0)
     DataBytes.resize(DataBytes.size() + 1);
   DEBUG(dbgs() << "addData -> " << DataBytes.size() << "\n");
 }
 
-uint32_t WasmObjectWriter::getRelocationIndexValue(
-    const WasmRelocationEntry &RelEntry) {
-  switch (RelEntry.Type) {
-  case wasm::R_WEBASSEMBLY_TABLE_INDEX_SLEB:
-  case wasm::R_WEBASSEMBLY_TABLE_INDEX_I32:
-    if (!IndirectSymbolIndices.count(RelEntry.Symbol))
-      report_fatal_error("symbol not found in table index space: " +
-                         RelEntry.Symbol->getName());
-    return IndirectSymbolIndices[RelEntry.Symbol];
-  case wasm::R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
-  case wasm::R_WEBASSEMBLY_GLOBAL_INDEX_LEB:
-  case wasm::R_WEBASSEMBLY_MEMORY_ADDR_LEB:
-  case wasm::R_WEBASSEMBLY_MEMORY_ADDR_SLEB:
-  case wasm::R_WEBASSEMBLY_MEMORY_ADDR_I32:
-    if (!SymbolIndices.count(RelEntry.Symbol))
-      report_fatal_error("symbol not found in function/global index space: " +
-                         RelEntry.Symbol->getName());
-    return SymbolIndices[RelEntry.Symbol];
-  case wasm::R_WEBASSEMBLY_TYPE_INDEX_LEB:
+uint32_t
+WasmObjectWriter::getRelocationIndexValue(const WasmRelocationEntry &RelEntry) {
+  if (RelEntry.Type == wasm::R_WEBASSEMBLY_TYPE_INDEX_LEB) {
     if (!TypeIndices.count(RelEntry.Symbol))
       report_fatal_error("symbol not found in type index space: " +
                          RelEntry.Symbol->getName());
     return TypeIndices[RelEntry.Symbol];
-  default:
-    llvm_unreachable("invalid relocation type");
   }
+
+  if (!SymbolIndices.count(RelEntry.Symbol))
+    report_fatal_error("symbol not found in function/global index space: " +
+                       RelEntry.Symbol->getName());
+  return SymbolIndices[RelEntry.Symbol];
 }
 
 // Apply the portions of the relocation records that we can handle ourselves
 // directly.
 void WasmObjectWriter::applyRelocations(
     ArrayRef<WasmRelocationEntry> Relocations, uint64_t ContentsOffset) {
   raw_pwrite_stream &Stream = getStream();
   for (const WasmRelocationEntry &RelEntry : Relocations) {
     uint64_t Offset = ContentsOffset +
                       RelEntry.FixupSection->getSectionOffset() +
                       RelEntry.Offset;
 
     DEBUG(dbgs() << "applyRelocation: " << RelEntry << "\n");
+    uint32_t Value = getProvisionalValue(RelEntry);
+
     switch (RelEntry.Type) {
-    case wasm::R_WEBASSEMBLY_TABLE_INDEX_SLEB:
     case wasm::R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
     case wasm::R_WEBASSEMBLY_TYPE_INDEX_LEB:
-    case wasm::R_WEBASSEMBLY_GLOBAL_INDEX_LEB: {
-      uint32_t Index = getRelocationIndexValue(RelEntry);
-      WritePatchableSLEB(Stream, Index, Offset);
-      break;
-    }
-    case wasm::R_WEBASSEMBLY_TABLE_INDEX_I32: {
-      uint32_t Index = getRelocationIndexValue(RelEntry);
-      WriteI32(Stream, Index, Offset);
-      break;
-    }
-    case wasm::R_WEBASSEMBLY_MEMORY_ADDR_SLEB: {
-      uint32_t Value = getProvisionalValue(RelEntry);
-      WritePatchableSLEB(Stream, Value, Offset);
-      break;
-    }
-    case wasm::R_WEBASSEMBLY_MEMORY_ADDR_LEB: {
-      uint32_t Value = getProvisionalValue(RelEntry);
+    case wasm::R_WEBASSEMBLY_GLOBAL_INDEX_LEB:
+    case wasm::R_WEBASSEMBLY_MEMORY_ADDR_LEB:
       WritePatchableLEB(Stream, Value, Offset);
       break;
-    }
-    case wasm::R_WEBASSEMBLY_MEMORY_ADDR_I32: {
-      uint32_t Value = getProvisionalValue(RelEntry);
+    case wasm::R_WEBASSEMBLY_TABLE_INDEX_I32:
+    case wasm::R_WEBASSEMBLY_MEMORY_ADDR_I32:
       WriteI32(Stream, Value, Offset);
       break;
-    }
+    case wasm::R_WEBASSEMBLY_TABLE_INDEX_SLEB:
+    case wasm::R_WEBASSEMBLY_MEMORY_ADDR_SLEB:
+      WritePatchableSLEB(Stream, Value, Offset);
+      break;
     default:
       llvm_unreachable("invalid relocation type");
     }
   }
 }
 
 // Write out the portions of the relocation records that the linker will
 // need to handle.

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

 ; CHECK-NEXT:     Segments:
 ; CHECK-NEXT:       - Offset:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           1
 ; CHECK-NEXT:         Functions:       [ 0 ]
 ; CHECK:        - Type:            DATA
 ; CHECK-NEXT:     Relocations:
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_TABLE_INDEX_I32
-; CHECK-NEXT:         Index:           1
+; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Offset:          0x00000006

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

 ; 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: 0x1E
-; CHECK:       Index: 0x1
+; CHECK:       Index: 0x2
 ; CHECK:     }
 ; CHECK:   }

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

 ; CHECK-NEXT:           Value:           1
 ; CHECK-NEXT:         Functions:       [ 5, 7 ]
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Relocations:     
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_FUNCTION_INDEX_LEB
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Offset:          0x00000004
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_TABLE_INDEX_SLEB
-; CHECK-NEXT:         Index:           1
+; CHECK-NEXT:         Index:           5
 ; CHECK-NEXT:         Offset:          0x0000000F
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_MEMORY_ADDR_SLEB
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Offset:          0x00000017
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_FUNCTION_INDEX_LEB
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:         Offset:          0x0000001D
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_FUNCTION_INDEX_LEB
 ; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:         Offset:          0x0000002C
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_TABLE_INDEX_SLEB
-; CHECK-NEXT:         Index:           2
+; CHECK-NEXT:         Index:           7
 ; CHECK-NEXT:         Offset:          0x00000037
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_MEMORY_ADDR_SLEB
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Offset:          0x0000003F
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_FUNCTION_INDEX_LEB
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:         Offset:          0x00000045
 ; CHECK-NEXT:     Functions:       

llvm/trunk/test/MC/WebAssembly/weak-alias.ll

 ; CHECK-NEXT:         Locals:          
 ; CHECK-NEXT:         Body:            410028029080808000118080808000000B
 ; CHECK-NEXT:   - Type:            DATA
 ; CHECK-NEXT:     Relocations:     
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_TABLE_INDEX_I32
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:         Offset:          0x0000000F
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_TABLE_INDEX_I32
-; CHECK-NEXT:         Index:           2
+; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Offset:          0x00000018
 ; CHECK-NEXT:     Segments:        
 ; CHECK-NEXT:       - SectionOffset:   6
 ; CHECK-NEXT:         MemoryIndex:     0
 ; CHECK-NEXT:         Offset:          
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           0
 ; CHECK-NEXT:         Content:         '07000000'