[lld] 712264b - [ELF] Merge parseSymbolVersion and computeIspreemptible

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2024-12-08T17:50:17-08:00
New Revision: 712264b83c736fac7a7f0d4296f84c1afbd93b1a

URL: https://github.com/llvm/llvm-project/commit/712264b83c736fac7a7f0d4296f84c1afbd93b1a
DIFF: https://github.com/llvm/llvm-project/commit/712264b83c736fac7a7f0d4296f84c1afbd93b1a.diff

LOG: [ELF] Merge parseSymbolVersion and computeIspreemptible

ICF needs isPreemptible, which can be combined with parseSymbolVersion.

Added: 
    

Modified: 
    lld/ELF/Driver.cpp
    lld/ELF/ICF.cpp
    lld/ELF/SymbolTable.cpp
    lld/ELF/Symbols.cpp
    lld/ELF/Symbols.h
    lld/ELF/Writer.cpp

Removed: 
    


################################################################################
diff  --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index bc4b967ccbbbb4..1f90ecccb96f23 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2987,6 +2987,8 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
   if (!ctx.arg.relocatable) {
     llvm::TimeTraceScope timeScope("Process symbol versions");
     ctx.symtab->scanVersionScript();
+
+    parseVersionAndComputeIsPreemptible(ctx);
   }
 
   // Skip the normal linked output if some LTO options are specified.

diff  --git a/lld/ELF/ICF.cpp b/lld/ELF/ICF.cpp
index 80ddc3c32cf5d1..1cdcf6be9d8a93 100644
--- a/lld/ELF/ICF.cpp
+++ b/lld/ELF/ICF.cpp
@@ -461,13 +461,6 @@ static void combineRelocHashes(unsigned cnt, InputSection *isec,
 
 // The main function of ICF.
 template <class ELFT> void ICF<ELFT>::run() {
-  // Compute isPreemptible early. We may add more symbols later, so this loop
-  // cannot be merged with the later computeIsPreemptible() pass which is used
-  // by scanRelocations().
-  if (ctx.arg.hasDynSymTab)
-    for (Symbol *sym : ctx.symtab->getSymbols())
-      sym->isPreemptible = computeIsPreemptible(ctx, *sym);
-
   // Two text sections may have identical content and relocations but 
diff erent
   // LSDA, e.g. the two functions may have catch blocks of 
diff erent types. If a
   // text section is referenced by a .eh_frame FDE with LSDA, it is not

diff  --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index 648da94989d7a4..d5b2200b502279 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -350,13 +350,6 @@ void SymbolTable::scanVersionScript() {
         assignAsterisk(pat, &v, true);
   }
 
-  // Symbol themselves might know their versions because symbols
-  // can contain versions in the form of <name>@<version>.
-  // Let them parse and update their names to exclude version suffix.
-  for (Symbol *sym : symVector)
-    if (sym->hasVersionSuffix)
-      sym->parseSymbolVersion(ctx);
-
   // isPreemptible is false at this point. To correctly compute the binding of a
   // Defined (which is used by includeInDynsym(ctx)), we need to know if it is
   // VER_NDX_LOCAL or not. Compute symbol versions before handling

diff  --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index dd530c59c3dc8e..b19381fe439c79 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -11,6 +11,7 @@
 #include "InputFiles.h"
 #include "InputSection.h"
 #include "OutputSections.h"
+#include "SymbolTable.h"
 #include "SyntheticSections.h"
 #include "Target.h"
 #include "Writer.h"
@@ -345,7 +346,7 @@ bool elf::computeIsPreemptible(Ctx &ctx, const Symbol &sym) {
 
   // Only symbols with default visibility that appear in dynsym can be
   // preempted. Symbols with protected visibility cannot be preempted.
-  if (!sym.includeInDynsym(ctx) || sym.visibility() != STV_DEFAULT)
+  if (sym.visibility() != STV_DEFAULT)
     return false;
 
   // At this point copy relocations have not been created yet, so any
@@ -370,6 +371,20 @@ bool elf::computeIsPreemptible(Ctx &ctx, const Symbol &sym) {
   return true;
 }
 
+void elf::parseVersionAndComputeIsPreemptible(Ctx &ctx) {
+  // Symbol themselves might know their versions because symbols
+  // can contain versions in the form of <name>@<version>.
+  // Let them parse and update their names to exclude version suffix.
+  bool hasDynSymTab = ctx.arg.hasDynSymTab;
+  for (Symbol *sym : ctx.symtab->getSymbols()) {
+    if (sym->hasVersionSuffix)
+      sym->parseSymbolVersion(ctx);
+    if (hasDynSymTab)
+      sym->isPreemptible =
+          sym->includeInDynsym(ctx) && computeIsPreemptible(ctx, *sym);
+  }
+}
+
 // Merge symbol properties.
 //
 // When we have many symbols of the same name, we choose one of them,

diff  --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 85a52e98f87a1a..1a53f3a1e15261 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -527,6 +527,7 @@ void reportDuplicate(Ctx &, const Symbol &sym, const InputFile *newFile,
                      InputSectionBase *errSec, uint64_t errOffset);
 void maybeWarnUnorderableSymbol(Ctx &, const Symbol *sym);
 bool computeIsPreemptible(Ctx &, const Symbol &sym);
+void parseVersionAndComputeIsPreemptible(Ctx &);
 
 } // namespace lld::elf
 

diff  --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 8cd16704ba3ee3..ea9692a2f731b2 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -297,8 +297,11 @@ static void demoteSymbolsAndComputeIsPreemptible(Ctx &ctx) {
       }
     }
 
-    if (ctx.arg.hasDynSymTab)
-      sym->isPreemptible = computeIsPreemptible(ctx, *sym);
+    if (ctx.arg.hasDynSymTab) {
+      sym->exportDynamic = sym->includeInDynsym(ctx);
+      sym->isPreemptible =
+          sym->exportDynamic && computeIsPreemptible(ctx, *sym);
+    }
   }
 }
 
@@ -1888,7 +1891,7 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
       if (ctx.in.symTab)
         ctx.in.symTab->addSymbol(sym);
 
-      if (sym->includeInDynsym(ctx)) {
+      if (sym->exportDynamic) {
         ctx.partitions[sym->partition - 1].dynSymTab->addSymbol(sym);
         if (auto *file = dyn_cast<SharedFile>(sym->file))
           if (file->isNeeded && !sym->isUndefined())