[PATCH] D126950: [lld][WebAssembly] Retain data segments referenced via __start/__stop

[Yuta Saito via Phabricator via llvm-commits]

kateinoigakukun added a comment.

In D126950#3556306 <https://reviews.llvm.org/D126950#3556306>, @sbc100 wrote:

> I'm also fine with `start-stop-gc` being added a followup.

OK, let me do it in a follow-up patch 🙏



================
Comment at: lld/test/wasm/gc-sections-metadata-startstop.s:2
+# RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj -o %t.o %s
+# RUN: wasm-ld %t.o --gc-sections -o %t.wasm
+# RUN: llvm-objdump -d --no-show-raw-insn %t.wasm | FileCheck %s
----------------
sbc100 wrote:
> `--gc-sections` is the default under wasm so you don't need that here.
Ah, I've intentionally added the option to indicate the main testing target is the option, but it's ok to remove indeed.


================
Comment at: lld/wasm/MarkLive.cpp:121
+      if (symtab->find(("__start_" + name).str()) ||
+          symtab->find(("__stop_" + name).str())) {
+        enqueueWholeChunk(segment);
----------------
sbc100 wrote:
> kateinoigakukun wrote:
> > sbc100 wrote:
> > > So, does this mean that if I reference the symbol `__start_data`, then all the normal data segment in my whole program will be included (non will be GC'd)?
> > > 
> > > Is this how ELF works?  It seems like a shame to loose the ability to GC data segments just because i want to know where my data starts and/or stops.  I suppose we have other symbols such as `_edata` which can be used to get the start/end of the static data region without disabling GC?
> > > So, does this mean that if I reference the symbol __start_data, then all the normal data segment in my whole program will be included (non will be GC'd)?
> > 
> > Yes, I agree that's unfortunate situation...
> > 
> > > Is this how ELF works? 
> > 
> > To be precise, according to https://lld.llvm.org/ELF/start-stop-gc, whether `__start`/`__stop` symbols retain all input data segments is controlled by `-z start-stop-gc` / `-z nostart-stop-gc`.
> > 
> > The latest ELF lld defaults to `-z start-stop-gc`, but GNU ld defaults to `-z nostart-stop-gc` to be conservative.
> > 
> > So, how about adding the option in wasm-ld also? If it's reasonable, which behavior should be defaulted?
> Yes, adding start-stop-gc as part of this change makes sense to me.
> 
> So simplify this you could split out of the moving of the getOutputDataSegmentName function amd land that first?  It seems like a good refactoring anyway.
OK, I'll make a separate patch for the refactoring first :+1:


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126950/new/

https://reviews.llvm.org/D126950