[lld] c650880 - [ELF] Simplify handling of exportDynamic and canBeOmittedFromSymbolTable

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 8 09:33:54 PST 2024


Author: Fangrui Song
Date: 2024-12-08T09:33:48-08:00
New Revision: c6508809585505ffb88f5f724da04bbc058eabf8

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

LOG: [ELF] Simplify handling of exportDynamic and canBeOmittedFromSymbolTable

When computing whether a defined symbol is exported, we set
`exportDynamic` in Defined and CommonSymbol's ctor and merge the bit in
symbol resolution. The complexity is for the LTO special case
canBeOmittedFromSymbolTable, which can be simplified by introducing a
new bit.

We might simplify the state by caching includeInDynsym in exportDynamic
in the future.

Added: 
    

Modified: 
    lld/ELF/InputFiles.cpp
    lld/ELF/Relocations.cpp
    lld/ELF/Symbols.cpp
    lld/ELF/Symbols.h

Removed: 
    


################################################################################
diff  --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 97b3e164bdc3f4..9ff6ed58688cef 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -1742,8 +1742,11 @@ static void createBitcodeSymbol(Ctx &ctx, Symbol *&sym,
   } else {
     Defined newSym(ctx, &f, StringRef(), binding, visibility, type, 0, 0,
                    nullptr);
-    if (objSym.canBeOmittedFromSymbolTable())
-      newSym.exportDynamic = false;
+    // The definition can be omitted if all bitcode definitions satisfy
+    // `canBeOmittedFromSymbolTable()` and isUsedInRegularObj is false.
+    // The latter condition is tested in Symbol::includeInDynsym.
+    sym->ltoCanOmit = objSym.canBeOmittedFromSymbolTable() &&
+                      (!sym->isDefined() || sym->ltoCanOmit);
     sym->resolve(ctx, newSym);
   }
 }

diff  --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 818eef3e2e78aa..b47682857b398c 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -310,7 +310,6 @@ static void replaceWithDefined(Ctx &ctx, Symbol &sym, SectionBase &sec,
       .overwrite(sym);
 
   sym.versionId = old.versionId;
-  sym.exportDynamic = true;
   sym.isUsedInRegularObj = true;
   // A copy relocated alias may need a GOT entry.
   sym.flags.store(old.flags.load(std::memory_order_relaxed) & NEEDS_GOT,

diff  --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index b9069f14ea6e3c..dd530c59c3dc8e 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -278,7 +278,8 @@ bool Symbol::includeInDynsym(Ctx &ctx) const {
     // __pthread_initialize_minimal reference in csu/libc-start.c.
     return !(isUndefWeak() && ctx.arg.noDynamicLinker);
 
-  return exportDynamic;
+  return exportDynamic ||
+         (ctx.arg.exportDynamic && (isUsedInRegularObj || !ltoCanOmit));
 }
 
 // Print out a log message for --trace-symbol.
@@ -375,9 +376,6 @@ bool elf::computeIsPreemptible(Ctx &ctx, const Symbol &sym) {
 // and that's the result of symbol resolution. However, symbols that
 // were not chosen still affect some symbol properties.
 void Symbol::mergeProperties(const Symbol &other) {
-  if (other.exportDynamic)
-    exportDynamic = true;
-
   // DSO symbols do not affect visibility in the output.
   if (!other.isShared() && other.visibility() != STV_DEFAULT) {
     uint8_t v = visibility(), ov = other.visibility();
@@ -564,8 +562,6 @@ void Symbol::checkDuplicate(Ctx &ctx, const Defined &other) const {
 }
 
 void Symbol::resolve(Ctx &ctx, const CommonSymbol &other) {
-  if (other.exportDynamic)
-    exportDynamic = true;
   if (other.visibility() != STV_DEFAULT) {
     uint8_t v = visibility(), ov = other.visibility();
     setVisibility(v == STV_DEFAULT ? ov : std::min(v, ov));
@@ -602,8 +598,6 @@ void Symbol::resolve(Ctx &ctx, const CommonSymbol &other) {
 }
 
 void Symbol::resolve(Ctx &ctx, const Defined &other) {
-  if (other.exportDynamic)
-    exportDynamic = true;
   if (other.visibility() != STV_DEFAULT) {
     uint8_t v = visibility(), ov = other.visibility();
     setVisibility(v == STV_DEFAULT ? ov : std::min(v, ov));

diff  --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 18d078c58fa55c..85a52e98f87a1a 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -130,11 +130,8 @@ class Symbol {
   LLVM_PREFERRED_TYPE(bool)
   uint8_t exportDynamic : 1;
 
-  // True if the symbol is in the --dynamic-list file. A Defined symbol with
-  // protected or default visibility with this property is required to be
-  // exported into .dynsym.
   LLVM_PREFERRED_TYPE(bool)
-  uint8_t inDynamicList : 1;
+  uint8_t ltoCanOmit : 1;
 
   // Used to track if there has been at least one undefined reference to the
   // symbol. For Undefined and SharedSymbol, the binding may change to STB_WEAK
@@ -252,7 +249,7 @@ class Symbol {
          uint8_t stOther, uint8_t type)
       : file(file), nameData(name.data()), nameSize(name.size()), type(type),
         binding(binding), stOther(stOther), symbolKind(k), exportDynamic(false),
-        archSpecificBit(false) {}
+        ltoCanOmit(false), archSpecificBit(false) {}
 
   void overwrite(Symbol &sym, Kind k) const {
     if (sym.traced)
@@ -330,6 +327,12 @@ class Symbol {
   LLVM_PREFERRED_TYPE(bool)
   uint8_t thunkAccessed : 1;
 
+  // True if the symbol is in the --dynamic-list file. A Defined symbol with
+  // protected or default visibility with this property is required to be
+  // exported into .dynsym.
+  LLVM_PREFERRED_TYPE(bool)
+  uint8_t inDynamicList : 1;
+
   void setFlags(uint16_t bits) {
     flags.fetch_or(bits, std::memory_order_relaxed);
   }
@@ -365,7 +368,6 @@ class Defined : public Symbol {
           SectionBase *section)
       : Symbol(DefinedKind, file, name, binding, stOther, type), value(value),
         size(size), section(section) {
-    exportDynamic = ctx.arg.exportDynamic;
   }
   void overwrite(Symbol &sym) const;
 
@@ -403,7 +405,6 @@ class CommonSymbol : public Symbol {
                uint8_t stOther, uint8_t type, uint64_t alignment, uint64_t size)
       : Symbol(CommonKind, file, name, binding, stOther, type),
         alignment(alignment), size(size) {
-    exportDynamic = ctx.arg.exportDynamic;
   }
   void overwrite(Symbol &sym) const {
     Symbol::overwrite(sym, CommonKind);
@@ -447,7 +448,6 @@ class SharedSymbol : public Symbol {
                uint32_t alignment)
       : Symbol(SharedKind, &file, name, binding, stOther, type), value(value),
         size(size), alignment(alignment) {
-    exportDynamic = true;
     dsoProtected = visibility() == llvm::ELF::STV_PROTECTED;
     // GNU ifunc is a mechanism to allow user-supplied functions to
     // resolve PLT slot values at load-time. This is contrary to the


        


More information about the llvm-commits mailing list