[PATCH] D80354: [lld][WebAssembly] Do not emit initialization for .bss segments

[Thomas Lively via Phabricator via llvm-commits]

tlively created this revision.
tlively added a reviewer: sbc100.
Herald added subscribers: llvm-commits, sunfish, aheejin, jgravelle-google, dschuff.
Herald added a project: LLVM.
tlively marked an inline comment as done.
tlively added inline comments.


================
Comment at: lld/test/wasm/data-segments.ll:65
+; PASSIVE-NEXT:        Body:            41B4D60041004101FE480200044041B4D6004101427FFE0102001A054180084100410DFC08000041900841004114FC08010041B4D6004102FE17020041B4D600417FFE0002001A0BFC0900FC09010B
+
 ; PASSIVE-NEXT:  - Index:           2
----------------
There used to be 3 data.drop (FC09) instructions at the end, but now you can see that there are only two, as expected.


================
Comment at: lld/test/wasm/data-segments.ll:65
+; PASSIVE-NEXT:        Body:            41B4D60041004101FE480200044041B4D6004101427FFE0102001A054180084100410DFC08000041900841004114FC08010041B4D6004102FE17020041B4D600417FFE0002001A0BFC0900FC09010B
+
 ; PASSIVE-NEXT:  - Index:           2
----------------
tlively wrote:
> There used to be 3 data.drop (FC09) instructions at the end, but now you can see that there are only two, as expected.
So we doulbly dropping a segment before?


================
Comment at: lld/wasm/Writer.cpp:832
           writeUleb128(os, WASM_OPCODE_MEMORY_INIT, "memory.init");
           writeUleb128(os, s->index, "segment index immediate");
           writeU8(os, 0, "memory index immediate");
----------------
So was the crash was happening when there was only a bss segment?  And somehow having more than one segment was masking the crash?  

Should we add a specific test case for that?  An example that would have previously caused and invalid binary would be good.

I'm stil now clear why we didn't see this is more of our emscripten test cases,


This patch fixes a bug where initialization code for .bss segments was
emitted in the memory initialization function even though the .bss
segments were discounted in the datacount section and omitted in the
data section. This was producing invalid binaries due to out-of-bounds
segment indices on the memory.init and data.drop instructions that
were trying to operate on the nonexistent .bss segments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80354

Files:
  lld/test/wasm/data-segments.ll
  lld/wasm/SyntheticSections.cpp
  lld/wasm/SyntheticSections.h
  lld/wasm/Writer.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80354.265413.patch
Type: text/x-patch
Size: 5116 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200521/92859a88/attachment.bin>