[flang-commits] [lld] [llvm] [clang] [mlir] [flang] [ELF] Merge exportDynamic into versionId (PR #71272)

Fangrui Song via flang-commits flang-commits at lists.llvm.org
Sat Nov 4 16:04:10 PDT 2023


https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/71272

>From e6a9f8c08ddab444f581a18c0d3c2ad242448e7c Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Fri, 3 Nov 2023 14:48:13 -0700
Subject: [PATCH 1/2] [ELF] Merge exportDynamic into versionId

For a Defined/Common symbol with a false `inDynamicList`, both
`versionId==VER_NDX_LOCAL` and `!exportDynamic` indicate that the symbol
should not be exported, which means that the two fields have overlapping
purposes. We can merge them together by reserving `versionId==-1` to
indicate a symbol that is not exported:

* -1 (initial): not exported, binding unchanged
* 0: VER_NDX_LOCAL, not exported, binding changed to STB_LOCAL
* 1: VER_NDX_GLOBAL, exported, binding unchanged
* others: verdef index, exported, binding unchanged

-1 and 0 are similar, but -1 does not change the binding to STB_LOCAL.

The saved bit can be use for another purpose, e.g. whether a symbol has
a DSO definition (#70130).

--version-script is almost never used for executables. If used, a minor behavior
change is that a version pattern that is not `local:` will export the matched
symbols.
---
 lld/ELF/Config.h                     |  1 +
 lld/ELF/Driver.cpp                   |  9 ++++--
 lld/ELF/InputFiles.cpp               |  4 +--
 lld/ELF/LTO.cpp                      |  6 ++--
 lld/ELF/Relocations.cpp              |  4 +--
 lld/ELF/SymbolTable.cpp              |  2 +-
 lld/ELF/Symbols.cpp                  | 16 +++++------
 lld/ELF/Symbols.h                    | 42 +++++++++++++++++-----------
 lld/ELF/SyntheticSections.cpp        |  3 +-
 lld/test/ELF/symver-non-default.s    |  6 ++--
 lld/test/ELF/version-script-symver.s |  4 +--
 11 files changed, 55 insertions(+), 42 deletions(-)

diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 56229334f9a44ae..06fef790d1e5439 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -225,6 +225,7 @@ struct Config {
   bool cref;
   llvm::SmallVector<std::pair<llvm::GlobPattern, uint64_t>, 0>
       deadRelocInNonAlloc;
+  uint16_t defaultVersionId;
   bool demangle = true;
   bool dependentLibraries;
   bool disableVerify;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 6290880c43d3b95..555a396e1eccbb0 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1683,6 +1683,9 @@ static void readConfigs(opt::InputArgList &args) {
     }
   }
 
+  config->defaultVersionId =
+      config->exportDynamic ? ELF::VER_NDX_GLOBAL : nonExported;
+
   assert(config->versionDefinitions.empty());
   config->versionDefinitions.push_back(
       {"local", (uint16_t)VER_NDX_LOCAL, {}, {}});
@@ -2518,9 +2521,9 @@ static void combineVersionedSymbol(Symbol &sym,
     sym.symbolKind = Symbol::PlaceholderKind;
     sym.isUsedInRegularObj = false;
   } else if (auto *sym1 = dyn_cast<Defined>(&sym)) {
-    if (sym2->versionId > VER_NDX_GLOBAL
-            ? config->versionDefinitions[sym2->versionId].name == suffix1 + 1
-            : sym1->section == sym2->section && sym1->value == sym2->value) {
+    if (sym2->versionId == VER_NDX_GLOBAL || sym2->versionId == nonExported
+            ? sym1->section == sym2->section && sym1->value == sym2->value
+            : config->versionDefinitions[sym2->versionId].name == suffix1 + 1) {
       // Due to an assembler design flaw, if foo is defined, .symver foo,
       // foo at v1 defines both foo and foo at v1. Unless foo is bound to a
       // different version, GNU ld makes foo at v1 canonical and eliminates
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index a0d4be8ff9885b0..1ac94e179688fee 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -1520,7 +1520,7 @@ template <class ELFT> void SharedFile::parse() {
       }
       Symbol *s = symtab.addSymbol(
           Undefined{this, name, sym.getBinding(), sym.st_other, sym.getType()});
-      s->exportDynamic = true;
+      s->versionId = VER_NDX_GLOBAL;
       if (s->isUndefined() && sym.getBinding() != STB_WEAK &&
           config->unresolvedSymbolsInShlib != UnresolvedPolicy::Ignore)
         requiredSymbols.push_back(s);
@@ -1698,7 +1698,7 @@ createBitcodeSymbol(Symbol *&sym, const std::vector<bool> &keptComdats,
   } else {
     Defined newSym(&f, StringRef(), binding, visibility, type, 0, 0, nullptr);
     if (objSym.canBeOmittedFromSymbolTable())
-      newSym.exportDynamic = false;
+      newSym.versionId = nonExported;
     sym->resolve(newSym);
   }
 }
diff --git a/lld/ELF/LTO.cpp b/lld/ELF/LTO.cpp
index 504c12aac6c5696..e534f159e5e42be 100644
--- a/lld/ELF/LTO.cpp
+++ b/lld/ELF/LTO.cpp
@@ -245,9 +245,9 @@ void BitcodeCompiler::add(BitcodeFile &f) {
                             usedStartStop.count(objSym.getSectionName());
     // Identify symbols exported dynamically, and that therefore could be
     // referenced by a shared library not visible to the linker.
-    r.ExportDynamic =
-        sym->computeBinding() != STB_LOCAL &&
-        (config->exportDynamic || sym->exportDynamic || sym->inDynamicList);
+    r.ExportDynamic = sym->computeBinding() != STB_LOCAL &&
+                      (config->exportDynamic || sym->versionId != nonExported ||
+                       sym->inDynamicList);
     const auto *dr = dyn_cast<Defined>(sym);
     r.FinalDefinitionInLinkageUnit =
         (isExec || sym->visibility() != STV_DEFAULT) && dr &&
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 6765461805dadb0..cbf6c42cd2d9b8d 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -310,7 +310,7 @@ static void replaceWithDefined(Symbol &sym, SectionBase &sec, uint64_t value,
       .overwrite(sym);
 
   sym.verdefIndex = old.verdefIndex;
-  sym.exportDynamic = true;
+  sym.exportIfNonExported();
   sym.isUsedInRegularObj = true;
   // A copy relocated alias may need a GOT entry.
   sym.flags.store(old.flags.load(std::memory_order_relaxed) & NEEDS_GOT,
@@ -1068,7 +1068,7 @@ void RelocationScanner::processAux(RelExpr expr, RelType type, uint64_t offset,
   // direct relocation on through.
   if (LLVM_UNLIKELY(isIfunc) && config->zIfuncNoplt) {
     std::lock_guard<std::mutex> lock(relocMutex);
-    sym.exportDynamic = true;
+    sym.exportIfNonExported();
     mainPart->relaDyn->addSymbolReloc(type, *sec, offset, sym, addend, type);
     return;
   }
diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index fe7edd5b0eb49aa..1d8459c0db96857 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -93,7 +93,7 @@ Symbol *SymbolTable::insert(StringRef name) {
   sym->setName(name);
   sym->partition = 1;
   sym->verdefIndex = -1;
-  sym->versionId = VER_NDX_GLOBAL;
+  sym->versionId = nonExported;
   if (pos != StringRef::npos)
     sym->hasVersionSuffix = true;
   return sym;
diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index 671eb56f3404c95..0972a77fecfc32a 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -287,7 +287,7 @@ bool Symbol::includeInDynsym() const {
     // __pthread_initialize_minimal reference in csu/libc-start.c.
     return !(isUndefWeak() && config->noDynamicLinker);
 
-  return exportDynamic || inDynamicList;
+  return versionId != nonExported || inDynamicList;
 }
 
 // Print out a log message for --trace-symbol.
@@ -384,8 +384,8 @@ bool elf::computeIsPreemptible(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;
+  if (versionId == nonExported)
+    versionId = other.versionId;
 
   // DSO symbols do not affect visibility in the output.
   if (!other.isShared() && other.visibility() != STV_DEFAULT) {
@@ -575,8 +575,8 @@ void Symbol::checkDuplicate(const Defined &other) const {
 }
 
 void Symbol::resolve(const CommonSymbol &other) {
-  if (other.exportDynamic)
-    exportDynamic = true;
+  if (versionId == nonExported)
+    versionId = other.versionId;
   if (other.visibility() != STV_DEFAULT) {
     uint8_t v = visibility(), ov = other.visibility();
     setVisibility(v == STV_DEFAULT ? ov : std::min(v, ov));
@@ -613,8 +613,8 @@ void Symbol::resolve(const CommonSymbol &other) {
 }
 
 void Symbol::resolve(const Defined &other) {
-  if (other.exportDynamic)
-    exportDynamic = true;
+  if (versionId == nonExported)
+    versionId = other.versionId;
   if (other.visibility() != STV_DEFAULT) {
     uint8_t v = visibility(), ov = other.visibility();
     setVisibility(v == STV_DEFAULT ? ov : std::min(v, ov));
@@ -663,7 +663,7 @@ void Symbol::resolve(const LazyObject &other) {
 }
 
 void Symbol::resolve(const SharedSymbol &other) {
-  exportDynamic = true;
+  exportIfNonExported();
   if (isPlaceholder()) {
     other.overwrite(*this);
     return;
diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 4addb79d1257914..29eccb29fdd23da 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -67,6 +67,9 @@ struct SymbolAux {
 
 LLVM_LIBRARY_VISIBILITY extern SmallVector<SymbolAux, 0> symAux;
 
+// A versionId value similar to VER_NDX_LOCAL, but the binding is not changed.
+constexpr uint16_t nonExported = uint16_t(-1);
+
 // The base class for real symbol classes.
 class Symbol {
 public:
@@ -129,17 +132,6 @@ class Symbol {
   // which are referenced by relocations when -r or --emit-relocs is given.
   uint8_t used : 1;
 
-  // Used by a Defined symbol with protected or default visibility, to record
-  // whether it is required to be exported into .dynsym. This is set when any of
-  // the following conditions hold:
-  //
-  // - If there is an interposable symbol from a DSO. Note: We also do this for
-  //   STV_PROTECTED symbols which can't be interposed (to match BFD behavior).
-  // - If -shared or --export-dynamic is specified, any symbol in an object
-  //   file/bitcode sets this property, unless suppressed by LTO
-  //   canBeOmittedFromSymbolTable().
-  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.
@@ -254,7 +246,7 @@ class Symbol {
   Symbol(Kind k, InputFile *file, StringRef name, uint8_t binding,
          uint8_t stOther, uint8_t type)
       : file(file), nameData(name.data()), nameSize(name.size()), type(type),
-        binding(binding), stOther(stOther), symbolKind(k), exportDynamic(false),
+        binding(binding), stOther(stOther), symbolKind(k),
         archSpecificBit(false) {}
 
   void overwrite(Symbol &sym, Kind k) const {
@@ -316,9 +308,25 @@ class Symbol {
   // This field is a index to the symbol's version definition.
   uint16_t verdefIndex;
 
-  // Version definition index.
-  uint16_t versionId;
+  // Used by a Defined symbol with protected or default visibility, to record
+  // the verdef index and whether the symbol is exported into .dynsym.
+  // * -1 (initial): not exported, binding unchanged
+  // * 0: VER_NDX_LOCAL, not exported, binding changed to STB_LOCAL
+  // * 1: VER_NDX_GLOBAL, exported, binding unchanged
+  // * others: verdef index, exported, binding unchanged
+  //
+  // -1 transits to 1 if any of the following conditions hold:
+  // * If there is an interposable symbol from a DSO. Note: We also do this for
+  //   STV_PROTECTED symbols which can't be interposed (to match BFD behavior).
+  // * If -shared or --export-dynamic is specified, any symbol in an object
+  //   file/bitcode sets this property, unless suppressed by LTO
+  //   canBeOmittedFromSymbolTable().
+  uint16_t versionId = nonExported;
 
+  void exportIfNonExported() {
+    if (versionId == nonExported)
+      versionId = llvm::ELF::VER_NDX_GLOBAL;
+  }
   void setFlags(uint16_t bits) {
     flags.fetch_or(bits, std::memory_order_relaxed);
   }
@@ -353,7 +361,7 @@ class Defined : public Symbol {
           uint8_t type, uint64_t value, uint64_t size, SectionBase *section)
       : Symbol(DefinedKind, file, name, binding, stOther, type), value(value),
         size(size), section(section) {
-    exportDynamic = config->exportDynamic;
+    versionId = config->defaultVersionId;
   }
   void overwrite(Symbol &sym) const {
     Symbol::overwrite(sym, DefinedKind);
@@ -398,7 +406,7 @@ 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 = config->exportDynamic;
+    versionId = config->defaultVersionId;
   }
   void overwrite(Symbol &sym) const {
     Symbol::overwrite(sym, CommonKind);
@@ -442,7 +450,7 @@ class SharedSymbol : public Symbol {
                uint32_t alignment)
       : Symbol(SharedKind, &file, name, binding, stOther, type), value(value),
         size(size), alignment(alignment) {
-    exportDynamic = true;
+    versionId = llvm::ELF::VER_NDX_GLOBAL;
     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
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 0f7ebf9d7ba840b..11c39743883fea3 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -3128,7 +3128,8 @@ void VersionTableSection::writeTo(uint8_t *buf) {
     // For an unextracted lazy symbol (undefined weak), it must have been
     // converted to Undefined and have VER_NDX_GLOBAL version here.
     assert(!s.sym->isLazy());
-    write16(buf, s.sym->versionId);
+    write16(buf, s.sym->versionId == nonExported ? VER_NDX_GLOBAL
+                                                 : s.sym->versionId);
     buf += 2;
   }
 }
diff --git a/lld/test/ELF/symver-non-default.s b/lld/test/ELF/symver-non-default.s
index 0fe50367147e613..1947733b37123e6 100644
--- a/lld/test/ELF/symver-non-default.s
+++ b/lld/test/ELF/symver-non-default.s
@@ -20,7 +20,8 @@
 # RUN: ld.lld -pie --version-script=%t/ver1 %t/def1.o %t/ref.o -o %t1
 # RUN: llvm-readelf -s %t1 | FileCheck %s --check-prefix=EXE1
 
-# EXE1:         Symbol table '.dynsym' contains 1 entries:
+# EXE1:         Symbol table '.dynsym' contains 2 entries:
+# EXE:          1: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo at v1
 # EXE1:         Symbol table '.symtab' contains 3 entries:
 # EXE1:         2: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo{{$}}
 
@@ -49,7 +50,8 @@
 # RUN: ld.lld -pie --version-script=%t/ver1 %t/def2.o %t/def3.o %t/ref.o -o %t3
 # RUN: llvm-readelf -s %t3 | FileCheck %s --check-prefix=EXE3
 
-# EXE3:         Symbol table '.dynsym' contains 1 entries:
+# EXE3:         Symbol table '.dynsym' contains 2 entries:
+# EXE3:         1: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo at v1
 # EXE3:         Symbol table '.symtab' contains 4 entries:
 # EXE3:         2: {{.*}} NOTYPE GLOBAL DEFAULT [[#SEC:]] foo{{$}}
 # EXE3-NEXT:    3: {{.*}} NOTYPE GLOBAL DEFAULT [[#SEC]] foo{{$}}
diff --git a/lld/test/ELF/version-script-symver.s b/lld/test/ELF/version-script-symver.s
index db7c6f434ff4e57..798d9dbc5d7f144 100644
--- a/lld/test/ELF/version-script-symver.s
+++ b/lld/test/ELF/version-script-symver.s
@@ -42,9 +42,7 @@
 # RUN: ld.lld --version-script %t4.script -pie --export-dynamic %t.o -o %t4
 # RUN: llvm-readelf --dyn-syms %t4 | FileCheck --check-prefix=MIX2 %s
 # RUN: ld.lld --version-script %t4.script -pie %t.o -o %t4
-# RUN: llvm-readelf --dyn-syms %t4 | FileCheck --check-prefix=EXE %s
-
-# EXE: Symbol table '.dynsym' contains 1 entries:
+# RUN: llvm-readelf --dyn-syms %t4 | FileCheck --check-prefix=MIX2 %s
 
 # RUN: ld.lld --version-script %t4.script -shared %t.o %tref.o -o %t5.so
 # RUN: llvm-readelf -r %t5.so | FileCheck --check-prefix=RELOC %s

>From 20f988ea7bea7f215dd89ab7846e92fe0a479261 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Sat, 4 Nov 2023 16:03:32 -0700
Subject: [PATCH 2/2] Symbol::resolve(const SharedSymbol .*:  strength
 reduction to versionId = llvm::ELF::VER_NDX_GLOBAL

---
 lld/ELF/Symbols.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index 0972a77fecfc32a..18ea4cec88393a9 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -663,7 +663,7 @@ void Symbol::resolve(const LazyObject &other) {
 }
 
 void Symbol::resolve(const SharedSymbol &other) {
-  exportIfNonExported();
+  versionId = llvm::ELF::VER_NDX_GLOBAL;
   if (isPlaceholder()) {
     other.overwrite(*this);
     return;



More information about the flang-commits mailing list