Fix -emit-reloc against local symbols.

[ruiu]

Summary

Previously, we drop symbols starting with .L from the symbol table, so
if there is a relocation that refers a .L symbol, it ended up
referencing a null -- which happened to be interpreted as an absolute
symbol.

This patch copies all symbols including local ones if -emit-reloc is
given.

Fixes https://bugs.llvm.org/show_bug.cgi?id=41385

[kees]

I can confirm this fixes the Linux kernel relocation visibility problem I saw. Thank you!

[grimar]

lld/trunk/test/ELF/emit-relocs-mergeable2.s

6 ↗

(On Diff #194095)

I would suggest adding the header for these values and a short description of what this patch does.
(I know we are missing the descriptions often, but ideally, I think we might want to add a description for each of
our test. At least that is something I have in my TODO list for LLD and it is really useful sometimes).

[grimar]

lld/trunk/test/ELF/emit-relocs-mergeable2.s

6 ↗

(On Diff #194095)

what this patch does -> what this test checks

[ruiu]

lld/trunk/test/ELF/emit-relocs-mergeable2.s

6 ↗

(On Diff #194095)

Honestly, unlike code, I don't think we need to explain the test in details. If you will need to change this test in the future, for example, it is very likely that you are changing the code and already understand what is expected.

[grimar]

lld/trunk/test/ELF/emit-relocs-mergeable2.s

6 ↗

(On Diff #194095)

I disagree here. It is sometimes hard to realize what the test does. I do not see any reason why we can't add a short description for each.
Perhaps the problem is that I still do not understand what this test do? And then this is what I am talking about: each test should have a clear description.

[MaskRay]

lld/trunk/test/ELF/emit-relocs-mergeable2.s

6 ↗

(On Diff #194095)

I agree with George that a short description will be helpful.

1. help understand the code
2. convenience for people who make layout changes and have to adjust many tests

lld/ELF/Writer.cpp

 
 static bool shouldKeepInSymtab(const Defined &Sym) {
   if (Sym.isSection())
     return false;
 
   if (Config->Discard == DiscardPolicy::None)
     return true;
 
+  // If -emit-reloc is given, all symbols including local ones need to be
+  // copied because they may be referenced by relocations.
+  if (Config->EmitRelocs)
+    return true;
+
   // In ELF assembly .L symbols are normally discarded by the assembler.
   // If the assembler fails to do so, the linker discards them if
   // * --discard-locals is used.
   // * The symbol is in a SHF_MERGE section, which is normally the reason for
   //   the assembler keeping the .L symbol.
   StringRef Name = Sym.getName();
   bool IsLocal = Name.startswith(".L") || Name.empty();
   if (!IsLocal)

lld/test/ELF/emit-relocs-mergeable2.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: ld.lld --emit-relocs %t.o -o %t.exe
+# RUN: llvm-readelf --relocations %t.exe | FileCheck %s
+
+# CHECK: 0000000000201004  000000010000000b R_X86_64_32S 0000000000200120 .Lfoo + 8
+
+.globl  _start
+_start:
+  movq .Lfoo+8, %rax
+.section .rodata.cst16,"aM",@progbits,16
+.Lfoo:
+  .quad 0
+  .quad 0
+