[PATCH] D95749: [ELF] Make SHF_GNU_RETAIN sections GC roots

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/MarkLive.cpp:267
+      continue;
+    }
     if (sec->flags & SHF_LINK_ORDER)
----------------
jhenderson wrote:
> Nit: should we have a blank line after this if statement? It's somewhat unrelated to SHF_LINK_ORDER after all.
This version looks equally readable to me...


================
Comment at: lld/test/ELF/gc-sections-retain.s:26
+
+# ERR: error: {{.*}}.o:(.nonalloc): sh_link points to discarded section {{.*}}.o:(.discard)
+
----------------
jhenderson wrote:
> Should we use `FileCheck -DFILE=%t1.o` to check the files in the error message are correct?
I think it is unnecessary. There are dedicates tests checking the file name. And for this test it is difficult to make the filename wrong while keeping the `.o` suffix


================
Comment at: lld/test/ELF/gc-sections-retain.s:37
+
+.ifdef NONALLOC
+.section .discard,"a", at progbits
----------------
jhenderson wrote:
> Maybe it would be simpler to use split-file to create two input files? The ifdef makes the test somewhat less readable to me.
This is to keep `_start` so that there is no warning for missing entry point.
(I think this is equally readable.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95749