[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 ↗

(On Diff #206147)

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/trunk/test/wasm/emit-relocs-fpic.s

+# RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj -o %t.o < %s
+# RUN: llc --relocation-model=pic -filetype=obj %p/Inputs/ret32.ll -o %t.ret32.o
+# RUN: wasm-ld -pie --export-all --no-gc-sections --no-entry --emit-relocs -o %t.wasm %t.o %t.ret32.o
+# RUN: obj2yaml %t.wasm | FileCheck %s
+
+load_hidden_data:
+    .functype   load_hidden_data () -> (i32)
+    i32.const   .L.hidden_data@MBREL
+    end_function
+
+.section .rodata.hidden_data,"",@
+.L.hidden_data:
+    .int8 100
+    .size .L.hidden_data, 1
+
+# We just want to make sure that processing this relocation doesn't
+# cause corrupt output. We get most of the way there, by just checking
+# that obj2yaml doesn't fail. Here we just make sure that the relocation
+# survived the trip.
+# CHECK: R_WASM_MEMORY_ADDR_REL_SLEB

lld/trunk/test/wasm/lit.local.cfg

 if 'wasm' not in config.available_features:
   config.unsupported = True
 
-config.suffixes = ['.test', '.yaml', '.ll']
+config.suffixes = ['.test', '.yaml', '.ll', '.s']

lld/trunk/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 (relocTypeHasAddend(Rel.Type))
       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/trunk/include/llvm/BinaryFormat/Wasm.h

 }
 
 inline bool operator!=(const WasmGlobalType &LHS, const WasmGlobalType &RHS) {
   return !(LHS == RHS);
 }
 
 std::string toString(WasmSymbolType type);
 std::string relocTypetoString(uint32_t type);
+bool relocTypeHasAddend(uint32_t type);
 
 } // end namespace wasm
 } // end namespace llvm
 
 #endif

llvm/trunk/lib/BinaryFormat/Wasm.cpp

#define WASM_RELOC(NAME, VALUE)                                                \
   case VALUE:                                                                  \
     return #NAME;
 #include "llvm/BinaryFormat/WasmRelocs.def"
 #undef WASM_RELOC
   default:
     llvm_unreachable("unknown reloc type");
   }
 }
+
+bool llvm::wasm::relocTypeHasAddend(uint32_t 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;
+  }
+}

llvm/trunk/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::relocTypeHasAddend(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)