[WebAssembly] Fix offsets for data relocations

[sbc100]

Summary

The code relocations getSectionOffset() returns the offset into the code
section for the data relocations it returns the offset into the data payload
which doesn't take into account the header of the data section.

As an alternative to this approach we could fixup all the FixupSection
fields of the data relocations once we know the header size.

The added test fails without this change (generates an invalid wasm
file).

[sbc100]

• Merge remote-tracking branch 'origin/master' into fix_data_relocations
• Add test

[sbc100]

• update comment

[dschuff]

Don't have a strong opinion but this seems better than fixing up the relocations separately. Maybe the comment on the SectionOffset field of MCSectionWasm should be updated (or some other place) to say that the section offsets don't include headers. Also it says "code section" when it's really more general than that now.

[sbc100]

• simplify

[sbc100]

• Merge remote-tracking branch 'origin/master' into fix_data_relocations
• add offsets
• Merge remote-tracking branch 'origin/master' into fix_data_relocations
• Merge remote-tracking branch 'origin/master' into fix_data_relocations
• use signed addend

[sbc100]

• formatting + comment fix

[sbc100]

In D32513#738751, @dschuff wrote:

Don't have a strong opinion but this seems better than fixing up the relocations separately. Maybe the comment on the SectionOffset field of MCSectionWasm should be updated (or some other place) to say that the section offsets don't include headers. Also it says "code section" when it's really more general than that now.

Done.

[dschuff]

LGTM, this is even better.

lib/MC/WasmObjectWriter.cpp

474 ↗

(On Diff #97156)

should these be on separate lines to match the rest?

[sbc100]

• Merge remote-tracking branch 'origin/master' into fix_data_relocations
• formatting

[sbc100]

lib/MC/WasmObjectWriter.cpp

474 ↗

(On Diff #97156)

Ran clang format to fix correctly.

llvm/trunk/include/llvm/MC/MCSectionWasm.h

 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
 
 class MCSymbol;
 
 /// This represents a section on wasm.
 class MCSectionWasm final : public MCSection {
+private:
   /// This is the name of the section.  The referenced memory is owned by
   /// TargetLoweringObjectFileWasm's WasmUniqueMap.
   StringRef SectionName;
 
   /// This is the sh_type field of a section, drawn from the enums below.
   unsigned Type;
 
   /// This is the sh_flags field of a section, drawn from the enums below.
   unsigned Flags;
 
   unsigned UniqueID;
 
   const MCSymbolWasm *Group;
 
-  // The offset of the MC function section in the wasm code section.
+  // The offset of the MC function/data section in the wasm code/data section.
+  // For data relocations the offset is relative to start of the data payload
+  // itself and does not include the size of the section header.
   uint64_t SectionOffset;
 
-private:
   friend class MCContext;
   MCSectionWasm(StringRef Section, unsigned type, unsigned flags, SectionKind K,
                 const MCSymbolWasm *group, unsigned UniqueID, MCSymbol *Begin)
       : MCSection(SV_Wasm, K, Begin), SectionName(Section), Type(type),
         Flags(flags), UniqueID(UniqueID), Group(group), SectionOffset(0) {
   }
 
   void setSectionName(StringRef Name) { SectionName = Name; }

llvm/trunk/include/llvm/MC/MCWasmObjectWriter.h

 class MCSymbolWasm;
 class MCValue;
 class raw_pwrite_stream;
 
 // Information about a single relocation.
 struct WasmRelocationEntry {
   uint64_t Offset;            // Where is the relocation.
   const MCSymbolWasm *Symbol; // The symbol to relocate with.
-  uint64_t Addend;            // A value to add to the symbol.
+  int64_t Addend;             // A value to add to the symbol.
   unsigned Type;              // The type of the relocation.
   MCSectionWasm *FixupSection;// The section the relocation is targeting.
 
   WasmRelocationEntry(uint64_t Offset, const MCSymbolWasm *Symbol,
-                      uint64_t Addend, unsigned Type,
+                      int64_t Addend, unsigned Type,
                       MCSectionWasm *FixupSection)
       : Offset(Offset), Symbol(Symbol), Addend(Addend), Type(Type),
         FixupSection(FixupSection) {}
 
   void print(raw_ostream &Out) const {
     Out << "Off=" << Offset << ", Sym=" << Symbol << ", Addend=" << Addend
         << ", Type=" << Type << ", FixupSection=" << FixupSection;
   }

llvm/trunk/include/llvm/Support/Wasm.h

struct WasmElemSegment {
   WasmInitExpr Offset;
   std::vector<uint32_t> Functions;
 };
 
 struct WasmRelocation {
   uint32_t Type;         // The type of the relocation.
   int32_t Index;         // Index into function to global index space.
   uint64_t Offset;       // Offset from the start of the section.
-  uint64_t Addend;       // A value to add to the symbol.
+  int64_t Addend;        // A value to add to the symbol.
 };
 
 enum : unsigned {
   WASM_SEC_CUSTOM = 0,   // Custom / User-defined section
   WASM_SEC_TYPE = 1,     // Function signature declarations
   WASM_SEC_IMPORT = 2,   // Import declarations
   WASM_SEC_FUNCTION = 3, // Function declarations
   WASM_SEC_TABLE = 4,    // Indirect function table and other tables

llvm/trunk/lib/MC/WasmObjectWriter.cpp

for (const WasmRelocationEntry &RelEntry : Relocations) {
     default:
       break;
     }
   }
 }
 
 // Write out the portions of the relocation records that the linker will
 // need to handle.
-static void WriteRelocations(
-    ArrayRef<WasmRelocationEntry> Relocations,
+static void
+WriteRelocations(ArrayRef<WasmRelocationEntry> Relocations,
                  raw_pwrite_stream &Stream,
-    DenseMap<const MCSymbolWasm *, uint32_t> &SymbolIndices)
-{
+                 DenseMap<const MCSymbolWasm *, uint32_t> &SymbolIndices,
+                 uint64_t HeaderSize) {
   for (const WasmRelocationEntry RelEntry : Relocations) {
     encodeULEB128(RelEntry.Type, Stream);
 
     uint64_t Offset = RelEntry.Offset +
-                      RelEntry.FixupSection->getSectionOffset();
+                      RelEntry.FixupSection->getSectionOffset() + HeaderSize;
     uint32_t Index = SymbolIndices[RelEntry.Symbol];
     int64_t Addend = RelEntry.Addend;
 
     switch (RelEntry.Type) {
     case wasm::R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
     case wasm::R_WEBASSEMBLY_TABLE_INDEX_SLEB:
     case wasm::R_WEBASSEMBLY_TABLE_INDEX_I32:
       encodeULEB128(Offset, Stream);
if (!Functions.empty()) {
     // Apply fixups.
     ApplyRelocations(CodeRelocations, getStream(), SymbolIndices,
                      Section.ContentsOffset);
 
     endSection(Section);
   }
 
   // === Data Section ==========================================================
+  uint32_t DataSectionHeaderSize = 0;
   if (!DataBytes.empty()) {
     startSection(Section, wasm::WASM_SEC_DATA);
 
     encodeULEB128(1, getStream()); // count
     encodeULEB128(0, getStream()); // memory index
     write8(wasm::WASM_OPCODE_I32_CONST);
     encodeSLEB128(0, getStream()); // offset
     write8(wasm::WASM_OPCODE_END);
     encodeULEB128(DataBytes.size(), getStream()); // size
+    DataSectionHeaderSize = getStream().tell() - Section.ContentsOffset;
     writeBytes(DataBytes); // data
 
     // Apply fixups.
     ApplyRelocations(DataRelocations, getStream(), SymbolIndices,
-                     Section.ContentsOffset);
+                     Section.ContentsOffset + DataSectionHeaderSize);
 
     endSection(Section);
   }
 
   // === Name Section ==========================================================
   uint32_t TotalFunctions = NumFuncImports + Functions.size();
   if (TotalFunctions != 0) {
     startSection(Section, wasm::WASM_SEC_CUSTOM, "name");
#endif
   // === Code Reloc Section ====================================================
   if (!CodeRelocations.empty()) {
     startSection(Section, wasm::WASM_SEC_CUSTOM, "reloc.CODE");
 
     encodeULEB128(wasm::WASM_SEC_CODE, getStream());
 
     encodeULEB128(CodeRelocations.size() + TypeIndexFixups.size(), getStream());
 
-    WriteRelocations(CodeRelocations, getStream(), SymbolIndices);
+    WriteRelocations(CodeRelocations, getStream(), SymbolIndices, 0);
     WriteTypeRelocations(TypeIndexFixups, TypeIndexFixupTypes, getStream());
 
     endSection(Section);
   }
 
   // === Data Reloc Section ====================================================
   if (!DataRelocations.empty()) {
     startSection(Section, wasm::WASM_SEC_CUSTOM, "reloc.DATA");
 
     encodeULEB128(wasm::WASM_SEC_DATA, getStream());
 
     encodeULEB128(DataRelocations.size(), getStream());
 
-    WriteRelocations(DataRelocations, getStream(), SymbolIndices);
+    WriteRelocations(DataRelocations, getStream(), SymbolIndices,
+                     DataSectionHeaderSize);
 
     endSection(Section);
   }
 
   // === Linking Metadata Section ==============================================
   if (HasStackPointer) {
     startSection(Section, wasm::WASM_SEC_CUSTOM, "linking");
 

llvm/trunk/test/MC/WebAssembly/reloc-data.ll

+; RUN: llc -O0 -mtriple wasm32-unknown-unknown-wasm -filetype=obj %s -o - | llvm-readobj -r -expand-relocs | FileCheck %s
+
+; foo and bar are external and internal symbols.  a and b are pointers
+; initialized to these locations offset by 2 and -2 elements respecitively.
+@foo = external global i32, align 4
+@bar = global i64 7, align 4
+@a = global i32* getelementptr (i32, i32* @foo, i32 2), align 8
+@b = global i64* getelementptr (i64, i64* @bar, i64 -2), align 8
+
+; CHECK: Format: WASM
+; CHECK: Relocations [
+; CHECK:   Section (6) DATA {
+; CHECK:     Relocation {
+; CHECK:       Type: R_WEBASSEMBLY_GLOBAL_ADDR_I32 (5)
+; CHECK:       Offset: 0xE
+; CHECK:       Index: 0x0
+; CHECK:       Addend: 8
+; CHECK:     }
+; CHECK:     Relocation {
+; CHECK:       Type: R_WEBASSEMBLY_GLOBAL_ADDR_I32 (5)
+; CHECK:       Offset: 0x16
+; CHECK:       Index: 0x1
+; CHECK:       Addend: -16
+; CHECK:     }
+; CHECK:   }
+; CHECK: ]