[WebAssembly] Fix list of relocations with addends in lld

[loladiro]

Summary

The list of relocations with addend in lld was missing R_WASM_MEMORY_ADDR_REL_SLEB,
causing wasm-ld to generate corrupted output. This fixes that problem and while
we're at it pulls the list of such relocations into the Wasm.h header, to avoid
duplicating it in multiple places.

[sbc100]

Thanks!

I guess this means we are also missing a test case for --emit-relocs + -fPIC?

llvm/include/llvm/BinaryFormat/Wasm.h

169

Rather than adding methods to this struct can you implement in the same way that relocTypetoString is below?

Perhaps we can make them methods later, but I'm currently a fan of keeping the struct definitions clean so they can be easily matched with the binary spec.

[loladiro]

Address review comments and add test case.

[sbc100]

lgtm with comment

lld/test/wasm/emit-relocs-fpic.ll

1 ↗

(On Diff #206550)

Can you name this file .s since it contains asm?

(I think this is the first of the .s files tests in lld so you will also need to add .s to the list of suffixes in lld/test/wasm/lit.local.cfg)

[loladiro]

lld/test/wasm/emit-relocs-fpic.ll

1 ↗

(On Diff #206550)

Good catch. Meant to do that, but forgot. Will do and commit.

[loladiro]

There was a small omission in this revision that I fixed up in https://reviews.llvm.org/rG5bb0dcd96ec1.

lld/wasm/InputChunks.cpp

void InputChunk::writeRelocations(raw_ostream &OS) const {
   LLVM_DEBUG(dbgs() << "writeRelocations: " << File->getName()
                     << " offset=" << Twine(Off) << "\n");
 
   for (const WasmRelocation &Rel : Relocations) {
     writeUleb128(OS, Rel.Type, "reloc type");
     writeUleb128(OS, Rel.Offset + Off, "reloc offset");
     writeUleb128(OS, File->calcNewIndex(Rel), "reloc index");
 
-    switch (Rel.Type) {
-    case R_WASM_MEMORY_ADDR_LEB:
-    case R_WASM_MEMORY_ADDR_SLEB:
-    case R_WASM_MEMORY_ADDR_I32:
-    case R_WASM_FUNCTION_OFFSET_I32:
-    case R_WASM_SECTION_OFFSET_I32:
+    if (Rel.hasAddend())
       writeSleb128(OS, File->calcNewAddend(Rel), "reloc addend");
-      break;
-    }
   }
 }
 
 void InputFunction::setFunctionIndex(uint32_t Index) {
   LLVM_DEBUG(dbgs() << "InputFunction::setFunctionIndex: " << getName()
                     << " -> " << Index << "\n");
   assert(!hasFunctionIndex());
   FunctionIndex = Index;

llvm/include/llvm/BinaryFormat/Wasm.h

struct WasmDataReference {
   uint32_t Size;
 };
 
 struct WasmRelocation {
   uint8_t Type;    // The type of the relocation.
   uint32_t Index;  // Index into either symbol or type index space.
   uint64_t Offset; // Offset from the start of the section.
   int64_t Addend;  // A value to add to the symbol.
+  static bool hasAddend(unsigned Type);
+  inline bool hasAddend() const { return hasAddend(Type); };
 };
 
 struct WasmInitFunc {
   uint32_t Priority;
   uint32_t Symbol;
 };
 
 struct WasmSymbolInfo {

 #define WASM_RELOC(name, value) name = value,
 
 enum : unsigned {
 #include "WasmRelocs.def"
 };
 
 #undef WASM_RELOC
 
+inline bool WasmRelocation::hasAddend(unsigned Type) {
+  switch (Type) {
+  case R_WASM_MEMORY_ADDR_LEB:
+  case R_WASM_MEMORY_ADDR_SLEB:
+  case R_WASM_MEMORY_ADDR_REL_SLEB:
+  case R_WASM_MEMORY_ADDR_I32:
+  case R_WASM_FUNCTION_OFFSET_I32:
+  case R_WASM_SECTION_OFFSET_I32:
+    return true;
+  default:
+    return false;
+  }
+}
+
 // Subset of types that a value can have
 enum class ValType {
   I32 = WASM_TYPE_I32,
   I64 = WASM_TYPE_I64,
   F32 = WASM_TYPE_F32,
   F64 = WASM_TYPE_F64,
   V128 = WASM_TYPE_V128,
   EXCEPT_REF = WASM_TYPE_EXCEPT_REF,

llvm/lib/MC/WasmObjectWriter.cpp

struct WasmRelocationEntry {
   const MCSectionWasm *FixupSection; // The section the relocation is targeting.
 
   WasmRelocationEntry(uint64_t Offset, const MCSymbolWasm *Symbol,
                       int64_t Addend, unsigned Type,
                       const MCSectionWasm *FixupSection)
       : Offset(Offset), Symbol(Symbol), Addend(Addend), Type(Type),
         FixupSection(FixupSection) {}
 
-  bool hasAddend() const {
-    switch (Type) {
-    case wasm::R_WASM_MEMORY_ADDR_LEB:
-    case wasm::R_WASM_MEMORY_ADDR_SLEB:
-    case wasm::R_WASM_MEMORY_ADDR_REL_SLEB:
-    case wasm::R_WASM_MEMORY_ADDR_I32:
-    case wasm::R_WASM_FUNCTION_OFFSET_I32:
-    case wasm::R_WASM_SECTION_OFFSET_I32:
-      return true;
-    default:
-      return false;
-    }
-  }
+  bool hasAddend() const { return wasm::WasmRelocation::hasAddend(Type); }
 
   void print(raw_ostream &Out) const {
     Out << wasm::relocTypetoString(Type) << " Off=" << Offset
         << ", Sym=" << *Symbol << ", Addend=" << Addend
         << ", FixupSection=" << FixupSection->getSectionName();
   }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)