[PATCH] D102281: [lld][WebAssembly] Allow data symbols to extend past end of segment

[Sam Clegg via Phabricator via llvm-commits]

sbc100 created this revision.
Herald added subscribers: wingo, ecnelises, sunfish, hiraditya, jgravelle-google, dschuff.
sbc100 requested review of this revision.
Herald added subscribers: llvm-commits, aheejin.
Herald added a project: LLVM.

This fixes a bug with string merging which assumes that string symbols
are NULL terminated and therefore can end up trimming unused bytes.
Indeed this is exactly what happens the merge-string.s test.

I verified that this can happen in ELF too given the right conditions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102281

Files:
  lld/test/wasm/merge-string.s
  llvm/lib/Object/WasmObjectFile.cpp


Index: llvm/lib/Object/WasmObjectFile.cpp
===================================================================
--- llvm/lib/Object/WasmObjectFile.cpp
+++ llvm/lib/Object/WasmObjectFile.cpp
@@ -637,9 +637,13 @@
                                                 object_error::parse_failed);
         auto Offset = readVaruint64(Ctx);
         auto Size = readVaruint64(Ctx);
-        if (Offset + Size > DataSegments[Index].Data.Content.size())
-          return make_error<GenericBinaryError>("invalid data symbol offset",
-                                                object_error::parse_failed);
+        size_t SegmentSize = DataSegments[Index].Data.Content.size();
+        if (Offset > SegmentSize)
+          return make_error<GenericBinaryError>(
+              "invalid data symbol offset: `" + Info.Name +
+                  "` (offset: " + Twine(unsigned(Offset)) +
+                  " segment size: " + Twine(unsigned(SegmentSize)) + ")",
+              object_error::parse_failed);
         Info.DataRef = wasm::WasmDataReference{Index, Offset, Size};
       }
       break;
Index: lld/test/wasm/merge-string.s
===================================================================
--- lld/test/wasm/merge-string.s
+++ lld/test/wasm/merge-string.s
@@ -10,6 +10,10 @@
 // RUN: wasm-ld -O0 %t.o -o %t2.wasm --no-gc-sections --no-entry
 // RUN: obj2yaml %t2.wasm | FileCheck --check-prefixes=COMMON,NOMERGE %s
 
+// Check relocatable
+// RUN: wasm-ld -r %t.o -o %t2.o
+// RUN: obj2yaml %t2.o | FileCheck --check-prefixes=RELOC %s
+
         .section .rodata1,"S",@
         .asciz "abc"
 foo:
@@ -18,7 +22,7 @@
 bar:
         .asciz "bc"
         .asciz "bc"
-        .size bar, 4
+        .size bar, 6
 
         .section .rodata_relocs,"",@
 negative_addend:
@@ -74,3 +78,24 @@
 //  COMMON-NEXT:          Value:           1024
 //   MERGE-NEXT:          Content:         '61626300'
 // NOMERGE-NEXT:          Content:         '6162630061626300626300'
+
+
+//      RELOC:  - Type:            DATA
+// RELOC-NEXT:    Relocations:
+// RELOC-NEXT:      - Type:            R_WASM_MEMORY_ADDR_I32
+// RELOC-NEXT:        Index:           0
+// RELOC-NEXT:        Offset:          0xF
+// RELOC-NEXT:        Addend:          -10
+// RELOC-NEXT:    Segments:
+// RELOC-NEXT:      - SectionOffset:   6
+// RELOC-NEXT:        InitFlags:       0
+// RELOC-NEXT:        Offset:
+// RELOC-NEXT:          Opcode:          I32_CONST
+// RELOC-NEXT:          Value:           0
+// RELOC-NEXT:        Content:         '61626300'
+// RELOC-NEXT:      - SectionOffset:   15
+// RELOC-NEXT:        InitFlags:       0
+// RELOC-NEXT:        Offset:
+// RELOC-NEXT:          Opcode:          I32_CONST
+// RELOC-NEXT:          Value:           4
+// RELOC-NEXT:        Content:         F6FFFFFF


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D102281.344574.patch
Type: text/x-patch
Size: 2782 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210511/3c927b97/attachment.bin>