[ELF] Merge copyLocalSymbols and demoteLocalSymbolsInDiscardedSections (#69425)

Follow-up to #69295: In `Writer<ELFT>::run`, the symbol passes are
flexible:
they can be placed almost everywhere before `scanRelocations`, with a
constraint
that the `computeIsPreemptible` pass must be invoked for linker-defined
non-local symbols.

Merge copyLocalSymbols and demoteLocalSymbolsInDiscardedSections to
simplify
code:

* Demoting local symbols can be made unconditional, not constrainted to
/DISCARD/ uses due to performance concerns
* `includeInSymtab` can be made faster
* Make symbol passes close to each other
* Decrease data cache misses due to saving an iteration over local
symbols

There is no speedup, likely due to the unconditional `dr->section`
access in `demoteAndCopyLocalSymbols`.

`gc-sections-tls.s` no longer reports an error because the TLS symbol is
converted to an Undefined.

Author: MaskRay

lld/ELF/LinkerScript.cpp

@@ -613,7 +613,6 @@ void LinkerScript::processSectionCommands() {
         discard(*s);
       discardSynthetic(*osec);
       osec->commands.clear();
-      seenDiscard = true;
       return false;
     }
 

lld/ELF/LinkerScript.h

@@ -356,7 +356,6 @@ class LinkerScript final {
 
   bool hasSectionsCommand = false;
   bool seenDataAlign = false;
-  bool seenDiscard = false;
   bool seenRelroEnd = false;
   bool errorOnMissingSection = false;
   std::string backwardDotErr;

lld/ELF/Writer.cpp

@@ -53,7 +53,6 @@ template <class ELFT> class Writer {
   void run();
 
 private:
-  void copyLocalSymbols();
   void addSectionSymbols();
   void sortSections();
   void resolveShfLinkOrder();
@@ -292,18 +291,6 @@ static void demoteSymbolsAndComputeIsPreemptible() {
   }
 }
 
-static void demoteLocalSymbolsInDiscardedSections() {
-  llvm::TimeTraceScope timeScope("Demote local symbols");
-  parallelForEach(ctx.objectFiles, [&](ELFFileBase *file) {
-    DenseMap<SectionBase *, size_t> sectionIndexMap;
-    for (Symbol *sym : file->getLocalSymbols()) {
-      Defined *d = dyn_cast<Defined>(sym);
-      if (d && d->section && !d->section->isLive())
-        demoteDefined(*d, sectionIndexMap);
-    }
-  });
-}
-
 // Fully static executables don't support MTE globals at this point in time, as
 // we currently rely on:
 //   - A dynamic loader to process relocations, and
@@ -598,11 +585,6 @@ template <class ELFT> void elf::createSyntheticSections() {
 
 // 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.
@@ -751,31 +733,33 @@ bool lld::elf::includeInSymtab(const Symbol &b) {
     SectionBase *sec = d->section;
     if (!sec)
       return true;
+    assert(sec->isLive());
 
     if (auto *s = dyn_cast<MergeInputSection>(sec))
       return s->getSectionPiece(d->value).live;
-    return sec->isLive();
+    return true;
   }
   return b.used || !config->gcSections;
 }
 
-// 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;
+// Scan local symbols to:
+//
+// - demote symbols defined relative to /DISCARD/ discarded input sections so
+//   that relocations referencing them will lead to errors.
+// - copy eligible symbols to .symTab
+static void demoteAndCopyLocalSymbols() {
   llvm::TimeTraceScope timeScope("Add local symbols");
-  if (config->copyRelocs && config->discard != DiscardPolicy::None)
-    markUsedLocalSymbols<ELFT>();
   for (ELFFileBase *file : ctx.objectFiles) {
+    DenseMap<SectionBase *, size_t> sectionIndexMap;
     for (Symbol *b : file->getLocalSymbols()) {
       assert(b->isLocal() && "should have been caught in initializeSymbols()");
       auto *dr = dyn_cast<Defined>(b);
-
-      // No reason to keep local undefined symbol in symtab.
       if (!dr)
         continue;
-      if (includeInSymtab(*b) && shouldKeepInSymtab(*dr))
+
+      if (dr->section && !dr->section->isLive())
+        demoteDefined(*dr, sectionIndexMap);
+      else if (in.symTab && includeInSymtab(*b) && shouldKeepInSymtab(*dr))
         in.symTab->addSymbol(b);
     }
   }
@@ -1991,12 +1975,13 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
   }
 
   demoteSymbolsAndComputeIsPreemptible();
-  // Also demote local symbols defined relative to discarded input sections so
-  // that relocations referencing them will lead to errors. To avoid unneeded
-  // work, we only do this when /DISCARD/ is seen, but this demotation also
-  // applies to --gc-sections discarded sections.
-  if (script->seenDiscard)
-    demoteLocalSymbolsInDiscardedSections();
+
+  if (config->copyRelocs && config->discard != DiscardPolicy::None)
+    markUsedLocalSymbols<ELFT>();
+  demoteAndCopyLocalSymbols();
+
+  if (config->copyRelocs)
+    addSectionSymbols();
 
   // Change values of linker-script-defined symbols from placeholders (assigned
   // by declareSymbols) to actual definitions.

lld/test/ELF/gc-sections-tls.s

@@ -1,31 +1,25 @@
 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
 
-## Relocation in a non .debug_* referencing a discarded TLS symbol is invalid.
-## If we happen to have no PT_TLS, we will emit an error.
-# RUN: not ld.lld %t.o --gc-sections -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR
-
-# ERR: error: {{.*}}.o has an STT_TLS symbol but doesn't have an SHF_TLS section
-
-## TODO As a corner case, when /DISCARD/ is present, demoteLocalSymbolsInDiscardedSections
-## demotes tls and the error is not triggered.
-# RUN: echo 'SECTIONS { /DISCARD/ : {} }' > %t.lds
-# RUN: ld.lld %t.o --gc-sections -T %t.lds -o /dev/null
+## When a TLS section is discarded, we will resolve the relocation in a non-SHF_ALLOC
+## section to the addend. Technically, we can emit an error in this case as the
+## relocation type is not TLS.
+# RUN: ld.lld %t.o --gc-sections -o %t
+# RUN: llvm-readelf -x .noalloc %t | FileCheck %s
 
-## If we happen to have a PT_TLS, we will resolve the relocation to
-## an arbitrary value (current implementation uses a negative value).
 # RUN: echo '.section .tbss,"awT"; .globl root; root: .long 0' | \
 # RUN:   llvm-mc -filetype=obj -triple=x86_64 - -o %t1.o
 # RUN: ld.lld --gc-sections -u root %t.o %t1.o -o %t
 # RUN: llvm-readelf -x .noalloc %t | FileCheck %s
 
 # CHECK:      Hex dump of section '.noalloc':
-# CHECK-NEXT: 0x00000000 {{[0-9a-f]+}} ffffffff
+# CHECK-NEXT: 0x00000000 00800000 00000000
 
 .globl _start
 _start:
 
 .section .tbss,"awT",@nobits
+  .long 0
 tls:
   .long 0