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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 3 inline comments as done.
MaskRay added inline comments.


================
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
+
----------------
ikudrin wrote:
> Do we really need to check the symbols which are not important to the fix? Unnecessary details make the test fragile.
The STT_SECTION symbols (also STB_LOCAL) are actually an interesting cast which was neglected. I add them for coverage. I am careful and will not intentionally add useless tests.


================
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
----------------
ikudrin wrote:
> The same thoughts about these `0xFFFFFFFFFFFFFFFC` values.
These values don't make tests fragile and have minimum cost (they don't even introduce new lines).

Previously we simply dropped the symbols from relocation records. 0xFFFFFFFFFFFFFFFC is added just in case the addend is replaced by 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78389