[LLD][ELF] Implement --discard-* for cases when -r or --emit-relocs are used.

[ikudrin]

Summary

When discarding local symbols with --discard-all or --discard-locals, the ones which are used in relocations should be preserved. LLD used the simplest approach and just ignored those switches when -r or --emit-relocs was specified.

The patch implements handling the --discard-* switches for the cases when relocations are kept by identifying used local symbols and allowing removing only unused ones. This makes the behavior of LLD compatible with GNU linkers.

[MaskRay]

This makes the behavior of LLD compatible with GNU linkers.

I have verified the patch seems to match the behavior, but can you describe a bit why you need such a behavior?

For -shared --discard-locals --emit-relocs, the patch strips .Lunused . However, .L temporary symbols are usually already discarded by the assembler.

For -shared --discard-all --emit-relocs, this patch appears to be a bug fix. Previously we discard all local symbols.

lld/test/ELF/dont-discard-used-locals.s

1 ↗

(On Diff #256312)

Rename this to discard-locals-* to improve discoverability.

8 ↗

(On Diff #256312)

For the option name, prefer the double-dash form unless the single dash form is prevailing (e.g. -pie -shared )

13 ↗

(On Diff #256312)

No need to use --just-symbol-name. FileCheck allows you to omit the leading t T

33 ↗

(On Diff #256312)

Add -NEXT: if applicable

[MaskRay]

lld/test/ELF/dont-discard-used-locals.s

6 ↗

(On Diff #256312)

llvm-mc -filetype=obj -triple=x86_64 -save-temp-labels %s -o %t.o

54 ↗

(On Diff #256312)

Delete trailing empty line

[ikudrin]

Thank you @MaskRay for reviewing this!

• Renamed the test to discard-locals-preserve-used.s
• Reformulated the description of the test.
• Updated the test according to the review comments.

[ikudrin]

In D77807#1973831, @MaskRay wrote:

This makes the behavior of LLD compatible with GNU linkers.

I have verified the patch seems to match the behavior, but can you describe a bit why you need such a behavior?

Our customers noticed the difference when tried to switch from ld.bfd to lld. They use --discard-all -r and want only useful symbols to survive.

For -shared --discard-all --emit-relocs, this patch appears to be a bug fix. Previously we discard all local symbols.

Thanks. I forgot about that because that was not my direct intention. Maybe it is worth mentioning in the patch's description.

[grimar]

lld/ELF/Writer.cpp

564

Perhaps move it inside copyLocalSymbols()?

At least because this work can be avoided when !in.symTab:

template <class ELFT> void Writer<ELFT>::copyLocalSymbols() {
  if (!in.symTab)
    return;
...

648–649

I think it is better to update the comment, not remove it.

654

Symbol::used is initialized to something in the constructor of the Symbol:

Symbol(Kind k, InputFile *file, StringRefZ name, uint8_t binding,
       uint8_t stOther, uint8_t type)
    :  .... used(!config->gcSections), ....

LLD usually tries to initialize variables to a proper value from the begining.
Given all above I think more consistent would to change the initializer condition right there.
Then you should be able to remove this loop.

lld/test/ELF/discard-locals-preserve-used.s

5 ↗

(On Diff #256516)

Usually REQURES is placed on the first line.

9 ↗

(On Diff #256516)

DISCARD_NONE -> DISCARD-NONE here and below.
(We don't use underbars in check prefixes in LLD tests)

[ikudrin]

• Addressed comments by @grimar. Thanks!

[grimar]

I'd suggest to wait to hear what other reviewers think about the latest changes and my suggestions in general before continuing.

lld/ELF/Symbols.h

242

So, when !gcSections and the symbol is local, it bow sets used to false,
though the previous code set it to true in this case.

Seems this doesn't break anything in the current implementation,
though I wonder if we might want to have config->copyRelocs involved in the condition.
It perhaps would be a bit cleaner way to work with this flag.

Also, perhaps it can be initialized in the constructor body now, since the condition is not trivial.

nit: You can probably use isLocal() instead of checking the binding field directly.

264

The comment needs an update.

[ikudrin]

• Extended the comment for Symbol::used.

lld/ELF/Symbols.h

242

Seems this doesn't break anything in the current implementation, though I wonder if we might want to have config->copyRelocs involved in the condition. It perhaps would be a bit cleaner way to work with this flag.

If we add config->copyRelocs we will also have to add config->discard and the expression will easily become overcomplicated. Long complex expressions are usually hard to understand, so I would prefer this to be as short and simple as possible.

Also, perhaps it can be initialized in the constructor body now, since the condition is not trivial.

As used is a bit field, moving the initialization into the body might confuse the optimizer.

nit: You can probably use isLocal() instead of checking the binding field directly.

Using a method that depends on the state of a class in the member initializer list makes the constructor fragile. Better to avoid.

[MaskRay]

In D77807#1974123, @grimar wrote:

I'd suggest to wait to hear what other reviewers think about the latest changes and my suggestions in general before continuing.

Honestly I am wondering whether a relocation scanning is justified. The only problem I can think of with the current conservative choice for --discard-locals + (-r | --emit-relocs) is that the output is larger. The output contains some otherwise unneeded STB_LOCAL symbols. Note, STB_LOCAL symbols not used by a relocation don't interfere with linking.

For -shared --discard-locals --emit-relocs, the patch strips .Lunused . However, .L temporary symbols are usually already discarded by the assembler.

.L in object files is uncommon so we probably should not worry much about the additional symbols lld retains but GNU linkers discard.

For -shared --discard-all --emit-relocs, this patch appears to be a bug fix. Previously we discard all local symbols.

This discards local symbols as well. My question is why we have these unreferenced local symbols in object files and why we need to save .symtab space for them.

An assembler can convert a local symbol reference to a relocation referencing a STT_SECTION symbol, or resolve the fixup at assembly time. If the intention is to discard the unreferenced local symbols, a better way may be to not generate the local symbols in the object files. Alternatively, generate .L temporary labels which are a special kind of local symbols which usually don't survive in the object file.

At the minimum, the --discard-all --emit-relocs bug fix is still useful. For the additional test, I think we should just replace the existing discard-locals.s with the contents from discard-locals-preserve-used.s

[ikudrin]

In D77807#1974679, @MaskRay wrote:

Honestly I am wondering whether a relocation scanning is justified. The only problem I can think of with the current conservative choice for --discard-locals + (-r | --emit-relocs) is that the output is larger. The output contains some otherwise unneeded STB_LOCAL symbols. Note, STB_LOCAL symbols not used by a relocation don't interfere with linking.

In our case, the output is analyzed with additional tools and these unneeded symbols slow down the process significantly.

This discards local symbols as well. My question is why we have these unreferenced local symbols in object files and why we need to save .symtab space for them.

An assembler can convert a local symbol reference to a relocation referencing a STT_SECTION symbol, or resolve the fixup at assembly time. If the intention is to discard the unreferenced local symbols, a better way may be to not generate the local symbols in the object files. Alternatively, generate .L temporary labels which are a special kind of local symbols which usually don't survive in the object file.

Here is a reduced example of the initial problem I am solving in this patch:

$ cat a.c
static int foo = 0;
int bar() { return foo; }
$ clang -c a.c
$ readelf -rs a.o
Relocation section '.rela.text' at offset 0x150 contains 1 entry:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
000000000007  00040000000b R_X86_64_32S      0000000000000000 .bss + 0
...
Symbol table '.symtab' contains 6 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
...
     2: 0000000000000000     4 OBJECT  LOCAL  DEFAULT    4 foo
...
$ ld.bfd -r a.o --discard-all -o ar.o && nm ar.o
0000000000000000 T bar
$ ld.lld -r a.o --discard-all -o ar.o && nm ar.o
0000000000000000 T bar
0000000000000000 b foo

[ikudrin]

Ping.

[MaskRay]

Implement --discard-* for cases when -r or --emit-relocs are used.

The behavior difference should only be about --discard-all + {-r,--emit-relocs}.

For --discard-locals + {-r,--emit-relocs} (.Lunused is stripped in GNU ld), I am not sure we want to adapt GNU ld's rule.

For --discard-all + {-r,--emit-relocs} (the main purpose of this patch, I think. .Lunused and unused are stripped in GNU ld), I am still uncomfortable with the following change:

- used(!config->gcSections),
+ used(!config->gcSections && binding != llvm::ELF::STB_LOCAL),

and the relocation scanning behavior guarded by the subtle condition:

if (config->copyRelocs && config->discard != DiscardPolicy::None)
  markUsedLocalSymbols<ELFT>();

The --emit-relocs --discard-all bugfix is correct and we should fix that. I took this opportunity to improve test coverage a bit. Created D78389.

[ikudrin]

In D77807#1989361, @MaskRay wrote:

Implement --discard-* for cases when -r or --emit-relocs are used.

The behavior difference should only be about --discard-all + {-r,--emit-relocs}.

For --discard-locals + {-r,--emit-relocs} (.Lunused is stripped in GNU ld), I am not sure we want to adapt GNU ld's rule.

And what should we do instead? Note that without -r or --emit-relocs our behavior matches GNU linkers'.

For --discard-all + {-r,--emit-relocs} (the main purpose of this patch, I think. .Lunused and unused are stripped in GNU ld), I am still uncomfortable with the following change:

- used(!config->gcSections),
+ used(!config->gcSections && binding != llvm::ELF::STB_LOCAL),

Do you suggest to extend the expression and include config->copyRelocs into it, or revert this change and stick with my initial version, which was more localized?

and the relocation scanning behavior guarded by the subtle condition:

if (config->copyRelocs && config->discard != DiscardPolicy::None)
  markUsedLocalSymbols<ELFT>();

The condition is here just to avoid unneeded work. Any suggestions on how to improve that?

[MaskRay]

Can you rebase after D78389? The improved test coverage should help us understand the impact of the used change now.

For tests, you can just modify emit-relocs-discard-locals.s

[MaskRay]

lld/test/ELF/discard-locals-preserve-used.s

39 ↗

(On Diff #256550)

NOT patterns rely on the symbol order. If the symbol order is disrupted, NOT patterns may get stale and don't actually test the (in)existence.

The --check-prefixes approach requires update when the symbol order is changed, but at least it will not miss such updates.

[ikudrin]

• Rebased to the tip.
• Moved the initialization of the field used back to markUsedLocalSymbols(). This makes the handling more local. Made a corresponding note in the comment for that field.
• Dropped the own test in favor of updating tests from D78389.

[MaskRay]

Thanks for the update. The member initializer of used which I was not comfortable is now removed. The code looks pretty good now.

lld/ELF/Writer.cpp

650

Add a comment that when --gc-sections is not specified, this function is needed to set the used bits

679–681

referenced by relocations from live sections.

734

We can also add && !config->gcSections here to avoid work.

Now it is clear why we need --gc-sections tests in *-discard-locals. Symbol::used is always true when --no-gc-sections. When --gc-sections is specified, MarkLive sets used correctly, another relocation scanning (markUsedLocalSymbols) will be unnecessary.

[ikudrin]

• Added/improved comments.
• Added a condition to not execute the scanning if --gc-sections is specified.
• Added testing of the --emit-relocs --discard-all --gc-sections case.

[MaskRay]

lld/test/ELF/emit-relocs-discard-locals.s

27

I think you can still combine the two chunks of CHECK lines.

DICARD-LOCALS-NEXT: for the common part.
DISCARD-LOCALS-NOGC for the additional lines by --no-gc-sections.

It makes the diff stand out.

[ikudrin]

• Combined GC/NOGC checks in the test.

lld/ELF/Writer.cpp

734

I've added the check and the comment to markUsedLocalSymbols<ELFT>().

lld/test/ELF/emit-relocs-discard-locals.s

27

OK, combining gc/nogc variants together looks acceptable. Firstly, I tried to combine all four variants and the result was unreadable, that is why I split them.

[MaskRay]

Thanks!

lld/ELF/Symbols.h

Symbol(Kind k, InputFile *file, StringRefZ name, uint8_t binding,
          uint8_t stOther, uint8_t type)
       : file(file), nameData(name.data), nameSize(name.size), binding(binding),
         type(type), stOther(stOther), symbolKind(k), visibility(stOther & 3),
         isUsedInRegularObj(!file || file->kind() == InputFile::ObjKind),
         exportDynamic(isExportDynamic(k, visibility)), inDynamicList(false),
         canInline(false), referenced(false), traced(false), needsPltAddr(false),
         isInIplt(false), gotInIgot(false), isPreemptible(false),
         used(!config->gcSections), needsTocRestore(false),
         scriptDefined(false) {}
 
 public:
   // True the symbol should point to its PLT entry.
   // For SharedSymbol only.
   uint8_t needsPltAddr : 1;
 
   // True if this symbol is in the Iplt sub-section of the Plt and the Igot
   // sub-section of the .got.plt or .got.
   uint8_t isInIplt : 1;
 
   // True if this symbol needs a GOT entry and its GOT entry is actually in
   // Igot. This will be true only for certain non-preemptible ifuncs.
   uint8_t gotInIgot : 1;
 
   // True if this symbol is preemptible at load time.
   uint8_t isPreemptible : 1;
 
   // True if an undefined or shared symbol is used from a live section.
+  //
+  // NOTE: In Writer.cpp the field is used to mark local defined symbols
+  // which are referenced by relocations when -r or --emit-relocs is given.
   uint8_t used : 1;
 
   // True if a call to this symbol needs to be followed by a restore of the
   // PPC64 toc pointer.
   uint8_t needsTocRestore : 1;
 
   // True if this symbol is defined by a linker script.
   uint8_t scriptDefined : 1;
 

lld/ELF/Writer.cpp

template <class ELFT> void createSyntheticSections() {
   add(in.shStrTab);
   if (in.strTab)
     add(in.strTab);
 }
 
 // The main function of the writer.
 template <class ELFT> void Writer<ELFT>::run() {
   copyLocalSymbols();
 
   if (config->copyRelocs)
     addSectionSymbols();
 
   // Now that we have a complete set of output sections. This function
   // completes section contents. For example, we need to add strings
   // to the string table, and add entries to .got and .plt.
   // finalizeSections does that.
   finalizeSections();
template <class ELFT> void Writer<ELFT>::run() {
   writeBuildId();
   if (errorCount())
     return;
 
   if (auto e = buffer->commit())
     error("failed to write to the output file: " + toString(std::move(e)));
 }
 
+template <class ELFT, class RelTy>
+static void markUsedLocalSymbolsImpl(ObjFile<ELFT> *file,
+                                     llvm::ArrayRef<RelTy> rels) {
+  for (const RelTy &rel : rels) {
+    Symbol &sym = file->getRelocTargetSym(rel);
+    if (sym.isLocal())
+      sym.used = true;
+  }
+}
+
+// The function ensures that the "used" field of local symbols reflects the fact
+// that the symbol is used in a relocation from a live section.
+template <class ELFT> static void markUsedLocalSymbols() {
+  // With --gc-sections, the field is already filled.
+  // See MarkLive<ELFT>::resolveReloc().
+  if (config->gcSections)
+    return;
+  // Without --gc-sections, the field is initialized with "true".
+  // Drop the flag first and then rise for symbols referenced in relocations.
+  for (InputFile *file : objectFiles) {
+    ObjFile<ELFT> *f = cast<ObjFile<ELFT>>(file);
+    for (Symbol *b : f->getLocalSymbols())
+      b->used = false;
+    for (InputSectionBase *s : f->getSections()) {
+      InputSection *isec = dyn_cast_or_null<InputSection>(s);
+      if (!isec)
+        continue;
+      if (isec->type == SHT_REL)
+        markUsedLocalSymbolsImpl(f, isec->getDataAs<typename ELFT::Rel>());
+      else if (isec->type == SHT_RELA)
+        markUsedLocalSymbolsImpl(f, isec->getDataAs<typename ELFT::Rela>());
+    }
+  }
+}
+
 static bool shouldKeepInSymtab(const Defined &sym) {
   if (sym.isSection())
     return false;
 
-  // 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)
+  // If --emit-reloc or -r is given, preserve symbols referenced by relocations
+  // from live sections.
+  if (config->copyRelocs && sym.used)
     return true;
 
   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
   // * --discard-locals is used.
   // * The symbol is in a SHF_MERGE section, which is normally the reason for
static bool includeInSymtab(const Symbol &b) {
   return b.used;
 }
 
 // Local symbols are not in the linker's symbol table. This function scans
 // each object file's symbol table to copy local symbols to the output.
 template <class ELFT> void Writer<ELFT>::copyLocalSymbols() {
   if (!in.symTab)
     return;
+  if (config->copyRelocs && config->discard != DiscardPolicy::None)
+    markUsedLocalSymbols<ELFT>();
   for (InputFile *file : objectFiles) {
     ObjFile<ELFT> *f = cast<ObjFile<ELFT>>(file);
     for (Symbol *b : f->getLocalSymbols()) {
       if (!b->isLocal())
         fatal(toString(f) +
               ": broken object: getLocalSymbols returns a non-local symbol");
       auto *dr = dyn_cast<Defined>(b);
 

lld/test/ELF/emit-relocs-discard-locals.s

 # REQUIRES: x86
-## Test that --emit-relocs keeps local symbols and overrides --discard-{locals,all}.
+## Test that --emit-relocs keeps local symbols which are used in relocations
+## even when --discard-{locals,all} is given.
+
+## There are two separate code paths that mark used local symbols when
+## --gc-sections is specified and when is not. The test checks both.
 
 # 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-readelf -s %tlocal | FileCheck --check-prefix=DISCARD-LOCALS %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-readelf -s %tlocal.gc | FileCheck --check-prefix=DISCARD-LOCALS-GC %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-readelf -s %tall | FileCheck --check-prefix=DISCARD-ALL %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
+# RUN: ld.lld --emit-relocs --discard-all --gc-sections %t.o -o %tall.gc
+# RUN: llvm-readelf -s %tall.gc | FileCheck --check-prefix=DISCARD-ALL-GC %s
+# RUN: llvm-readobj -r %tall.gc | FileCheck --check-prefix=REL %s
+
+## --discard-locals removes unused local symbols which start with ".L"
+# DISCARD-LOCALS:    0: {{0+}} 0 NOTYPE  LOCAL  DEFAULT UND
+# DISCARD-LOCALS-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} .Lused
+# DISCARD-LOCALS-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} unused
+# DISCARD-LOCALS-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} unused_gc
+# DISCARD-LOCALS-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} used
+# DISCARD-LOCALS-NEXT:           SECTION LOCAL  DEFAULT {{.*}} .text
+# DISCARD-LOCALS-NEXT:           SECTION LOCAL  DEFAULT {{.*}} text
+# DISCARD-LOCALS-NEXT:           SECTION LOCAL  DEFAULT {{.*}} gc
+# DISCARD-LOCALS-NEXT:           SECTION LOCAL  DEFAULT {{.*}} .comment
+# DISCARD-LOCALS-NEXT:           NOTYPE  GLOBAL DEFAULT {{.*}} _start
+
+## --gc-sections removes symbols which belong to discarded sections.
+# DISCARD-LOCALS-GC: 0: {{0+}} 0 NOTYPE  LOCAL  DEFAULT UND
+# DISCARD-LOCALS-GC-NEXT:        NOTYPE  LOCAL  DEFAULT {{.*}} .Lused
+# DISCARD-LOCALS-GC-NEXT:        NOTYPE  LOCAL  DEFAULT {{.*}} unused
+# DISCARD-LOCALS-GC-NEXT:        NOTYPE  LOCAL  DEFAULT {{.*}} used
+# DISCARD-LOCALS-GC-NEXT:        SECTION LOCAL  DEFAULT {{.*}} .text
+# DISCARD-LOCALS-GC-NEXT:        SECTION LOCAL  DEFAULT {{.*}} text
+# DISCARD-LOCALS-GC-NEXT:        SECTION LOCAL  DEFAULT {{.*}} .comment
+# DISCARD-LOCALS-GC-NEXT:        NOTYPE  GLOBAL DEFAULT {{.*}} _start
+
+## --discard-all removes all unused regular local symbols.
+# DISCARD-ALL:       0: {{0+}} 0 NOTYPE  LOCAL  DEFAULT UND
+# DISCARD-ALL-NEXT:              NOTYPE  LOCAL  DEFAULT {{.*}} .Lused
+# DISCARD-ALL-NEXT:              NOTYPE  LOCAL  DEFAULT {{.*}} used
+# DISCARD-ALL-NEXT:              SECTION LOCAL  DEFAULT {{.*}} .text
+# DISCARD-ALL-NEXT:              SECTION LOCAL  DEFAULT {{.*}} text
+# DISCARD-ALL-NEXT:              SECTION LOCAL  DEFAULT {{.*}} gc
+# DISCARD-ALL-NEXT:              SECTION LOCAL  DEFAULT {{.*}} .comment
+# DISCARD-ALL-NEXT:              NOTYPE  GLOBAL DEFAULT {{.*}} _start
+
+# DISCARD-ALL-GC:    0: {{0+}} 0 NOTYPE  LOCAL  DEFAULT UND
+# DISCARD-ALL-GC-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} .Lused
+# DISCARD-ALL-GC-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} used
+# DISCARD-ALL-GC-NEXT:           SECTION LOCAL  DEFAULT {{.*}} .text
+# DISCARD-ALL-GC-NEXT:           SECTION LOCAL  DEFAULT {{.*}} text
+# DISCARD-ALL-GC-NEXT:           SECTION LOCAL  DEFAULT {{.*}} .comment
+# DISCARD-ALL-GC-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

lld/test/ELF/relocatable-discard-locals.s

 # REQUIRES: x86
-## Test that -r keeps local symbols and overrides --discard-{locals,all}.
-## Also see emit-relocs-discard-locals.s
+## Test that -r keeps local symbols which are used in relocations even when
+## --discard-{locals,all} is given.
 
 # 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-readelf -s %tlocal.ro | FileCheck --check-prefix=DISCARD-LOCALS %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-readelf -s %tall.ro | FileCheck --check-prefix=DISCARD-ALL %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
+## --discard-locals removes unused local symbols which start with ".L"
+# DISCARD-LOCALS:    0: {{0+}} 0 NOTYPE  LOCAL  DEFAULT UND
+# DISCARD-LOCALS-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} .Lused
+# DISCARD-LOCALS-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} unused
+# DISCARD-LOCALS-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} unused_gc
+# DISCARD-LOCALS-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} used
+# DISCARD-LOCALS-NEXT:           SECTION LOCAL  DEFAULT {{.*}} .text
+# DISCARD-LOCALS-NEXT:           SECTION LOCAL  DEFAULT {{.*}} text
+# DISCARD-LOCALS-NEXT:           SECTION LOCAL  DEFAULT {{.*}} gc
+# DISCARD-LOCALS-NEXT:           NOTYPE  GLOBAL DEFAULT {{.*}} _start
+
+## --discard-all removes all unused regular local symbols.
+# DISCARD-ALL:    0: {{0+}} 0 NOTYPE  LOCAL  DEFAULT UND
+# DISCARD-ALL-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} .Lused
+# DISCARD-ALL-NEXT:           NOTYPE  LOCAL  DEFAULT {{.*}} used
+# DISCARD-ALL-NEXT:           SECTION LOCAL  DEFAULT {{.*}} .text
+# DISCARD-ALL-NEXT:           SECTION LOCAL  DEFAULT {{.*}} text
+# DISCARD-ALL-NEXT:           SECTION LOCAL  DEFAULT {{.*}} gc
+# DISCARD-ALL-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