[ELF] Keep local symbols when both --emit-relocs and --discard-all ar…

…e specified This fixes a bug as exposed by D77807. Add tests for {--emit-relocs,-r} x {--discard-locals,--discard-all}. They add coverage for previously undertested cases: * STT_SECTION associated to GCed sections (`gc`) * STT_SECTION associated to retained sections (`text`) * STT_SECTION associated to non-SHF_ALLOC sections (`.comment`) * STB_LOCAL in GCed sections (`unused_gc`) Reviewed By: grimar, ikudrin Differential Revision: https://reviews.llvm.org/D78389
llvm · Apr 21, 2020 · 497c76e · 497c76e
1 parent b4fb705
commit 497c76e
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 9 deletions.
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
@@ -609,9 +609,6 @@ static bool isOutputFormatBinary(opt::InputArgList &args) {
 }
 
 static DiscardPolicy getDiscard(opt::InputArgList &args) {
-  if (args.hasArg(OPT_relocatable))
-    return DiscardPolicy::None;
-
   auto *arg =
       args.getLastArg(OPT_discard_all, OPT_discard_locals, OPT_discard_none);
   if (!arg)

diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
@@ -560,8 +560,7 @@ template <class ELFT> void createSyntheticSections() {
 
 // The main function of the writer.
 template <class ELFT> void Writer<ELFT>::run() {
-  if (config->discard != DiscardPolicy::All)
-    copyLocalSymbols();
+  copyLocalSymbols();
 
   if (config->copyRelocs)
     addSectionSymbols();
@@ -642,13 +641,15 @@ static bool shouldKeepInSymtab(const Defined &sym) {
   if (sym.isSection())
     return false;
 
-  if (config->discard == DiscardPolicy::None)
+  // If --emit-reloc or -r is given, all symbols including local ones need to be
+  // copied because they may be referenced by relocations.
+  if (config->copyRelocs)
     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)
+  if (config->discard == DiscardPolicy::None)
     return true;
+  if (config->discard == DiscardPolicy::All)
+    return false;
 
   // In ELF assembly .L symbols are normally discarded by the assembler.
   // If the assembler fails to do so, the linker discards them if

diff --git a/lld/test/ELF/emit-relocs-discard-locals.s b/lld/test/ELF/emit-relocs-discard-locals.s
@@ -0,0 +1,51 @@
+# REQUIRES: x86
+## Test that --emit-relocs keeps local symbols and overrides --discard-{locals,all}.
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 -save-temp-labels %s -o %t.o
+
+# RUN: ld.lld --emit-relocs --discard-locals %t.o -o %tlocal
+# RUN: llvm-readelf -s %tlocal | FileCheck --check-prefixes=SYM,SYM-NOGC %s
+# RUN: llvm-readobj -r %tlocal | FileCheck --check-prefix=REL %s
+## --gc-sections can discard symbols relative to GCed sections (including STT_SECTION).
+# RUN: ld.lld --emit-relocs --discard-locals --gc-sections %t.o -o %tlocal.gc
+# RUN: llvm-readelf -s %tlocal.gc | FileCheck --check-prefix=SYM %s
+# RUN: llvm-readobj -r %tlocal | FileCheck --check-prefix=REL %s
+
+# RUN: ld.lld --emit-relocs --discard-all %t.o -o %tall
+# RUN: llvm-readelf -s %tall | FileCheck --check-prefixes=SYM,SYM-NOGC %s
+# RUN: llvm-readobj -r %tall | FileCheck --check-prefix=REL %s
+
+# SYM:           NOTYPE  LOCAL  DEFAULT {{.*}} .Lunused
+# SYM-NOGC-NEXT: NOTYPE  LOCAL  DEFAULT {{.*}} .Lunused_gc
+# SYM-NEXT:      NOTYPE  LOCAL  DEFAULT {{.*}} .Lused
+# SYM-NEXT:      NOTYPE  LOCAL  DEFAULT {{.*}} unused
+# SYM-NOGC-NEXT: NOTYPE  LOCAL  DEFAULT {{.*}} unused_gc
+# SYM-NEXT:      NOTYPE  LOCAL  DEFAULT {{.*}} used
+# SYM-NEXT:      SECTION LOCAL  DEFAULT {{.*}} .text
+# SYM-NEXT:      SECTION LOCAL  DEFAULT {{.*}} text
+# SYM-NOGC-NEXT: SECTION LOCAL  DEFAULT {{.*}} gc
+# SYM-NEXT:      SECTION LOCAL  DEFAULT {{.*}} .comment
+# SYM-NEXT:      NOTYPE  GLOBAL DEFAULT {{.*}} _start
+
+# REL:      .rela.text {
+# REL-NEXT:   R_X86_64_PLT32 text 0xFFFFFFFFFFFFFFFC
+# REL-NEXT:   R_X86_64_PLT32 .Lused 0xFFFFFFFFFFFFFFFC
+# REL-NEXT:   R_X86_64_PLT32 used 0xFFFFFFFFFFFFFFFC
+# REL-NEXT: }
+
+.globl _start
+_start:
+  call text@plt
+  jmp .Lused@plt
+  call used@plt
+
+.section text,"ax"
+.Lunused:
+.Lused:
+unused:
+used:
+
+.section gc,"ax"
+.Lunused_gc:
+unused_gc:
+  ret
diff --git a/lld/test/ELF/relocatable-discard-locals.s b/lld/test/ELF/relocatable-discard-locals.s
@@ -0,0 +1,47 @@
+# REQUIRES: x86
+## Test that -r keeps local symbols and overrides --discard-{locals,all}.
+## Also see emit-relocs-discard-locals.s
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 -save-temp-labels %s -o %t.o
+
+# RUN: ld.lld -r --discard-locals %t.o -o %tlocal.ro
+# RUN: llvm-readelf -s %tlocal.ro | FileCheck --check-prefixes=SYM,SYM-NOGC %s
+# RUN: llvm-readobj -r %tlocal.ro | FileCheck --check-prefix=REL %s
+
+# RUN: ld.lld -r --discard-all %t.o -o %tall.ro
+# RUN: llvm-readelf -s %tall.ro | FileCheck --check-prefixes=SYM,SYM-NOGC %s
+# RUN: llvm-readobj -r %tall.ro | FileCheck --check-prefix=REL %s
+
+# SYM:           NOTYPE  LOCAL  DEFAULT {{.*}} .Lunused
+# SYM-NOGC-NEXT: NOTYPE  LOCAL  DEFAULT {{.*}} .Lunused_gc
+# SYM-NEXT:      NOTYPE  LOCAL  DEFAULT {{.*}} .Lused
+# SYM-NEXT:      NOTYPE  LOCAL  DEFAULT {{.*}} unused
+# SYM-NOGC-NEXT: NOTYPE  LOCAL  DEFAULT {{.*}} unused_gc
+# SYM-NEXT:      NOTYPE  LOCAL  DEFAULT {{.*}} used
+# SYM-NEXT:      SECTION LOCAL  DEFAULT {{.*}} .text
+# SYM-NEXT:      SECTION LOCAL  DEFAULT {{.*}} text
+# SYM-NOGC-NEXT: SECTION LOCAL  DEFAULT {{.*}} gc
+# SYM-NEXT:      NOTYPE  GLOBAL DEFAULT {{.*}} _start
+
+# REL:      .rela.text {
+# REL-NEXT:   R_X86_64_PLT32 text 0xFFFFFFFFFFFFFFFC
+# REL-NEXT:   R_X86_64_PLT32 .Lused 0xFFFFFFFFFFFFFFFC
+# REL-NEXT:   R_X86_64_PLT32 used 0xFFFFFFFFFFFFFFFC
+# REL-NEXT: }
+
+.globl _start
+_start:
+  call text@plt
+  jmp .Lused@plt
+  call used@plt
+
+.section text,"ax"
+.Lunused:
+.Lused:
+unused:
+used:
+
+.section gc,"ax"
+.Lunused_gc:
+unused_gc:
+  ret