[PATCH] D78389: [ELF] Keep local symbols when both --emit-relocs and --discard-all are specified

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin accepted this revision.
ikudrin added a comment.

LGTM with a couple of nits.



================
Comment at: lld/ELF/Writer.cpp:646
   // copied because they may be referenced by relocations.
-  if (config->emitRelocs)
+  if (config->emitRelocs || config->relocatable)
     return true;
----------------
You may use `config->copyRelocs`, it is the same.


================
Comment at: lld/test/ELF/emit-relocs-discard-locals.s:26-27
+# SYM-NOGC-NEXT: SECTION LOCAL  DEFAULT    3 unreferenced
+# SYM-NEXT:      SECTION LOCAL  DEFAULT    {{.*}} .comment
+# SYM-NEXT:      NOTYPE  GLOBAL DEFAULT    1 _start
+
----------------
Do we really need to check the symbols which are not important to the fix? Unnecessary details make the test fragile.


================
Comment at: lld/test/ELF/emit-relocs-discard-locals.s:30-32
+# REL-NEXT:   R_X86_64_PLT32 text 0xFFFFFFFFFFFFFFFC
+# REL-NEXT:   R_X86_64_PLT32 .Lused 0xFFFFFFFFFFFFFFFC
+# REL-NEXT:   R_X86_64_PLT32 used 0xFFFFFFFFFFFFFFFC
----------------
The same thoughts about these `0xFFFFFFFFFFFFFFFC` values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78389