[lld] 255ea48 - [ELF] Merge verdefIndex into versionId. NFC (#72208)

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 16 01:03:58 PST 2023


Author: Fangrui Song
Date: 2023-11-16T01:03:52-08:00
New Revision: 255ea486085fca79d21eb0082594579abdbd89d0

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

LOG: [ELF] Merge verdefIndex into versionId. NFC (#72208)

The two fields are similar.

`versionId` is the Verdef index in the output file. It is set for
`--exclude-id=`, version script patterns, and `sym at ver` symbols.

`verdefIndex` is the Verdef index of a Sharedfile (SharedSymbol or a
copy-relocated Defined), the default value -1 is also used to indicate
that the symbol has not been matched by a version script pattern
(https://reviews.llvm.org/D65716).

It seems confusing to have two fields. Merge them so that we can
allocate one bit for #70130 (suppress --no-allow-shlib-undefined
error in the presence of a DSO definition).

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 8c7f2c8773f2cbc..4b4d7d6db93cd57 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -1546,7 +1546,7 @@ template <class ELFT> void SharedFile::parse() {
           SharedSymbol{*this, name, sym.getBinding(), sym.st_other,
                        sym.getType(), sym.st_value, sym.st_size, alignment});
       if (s->file == this)
-        s->verdefIndex = ver;
+        s->versionId = ver;
     }
 
     // Also add the symbol with the versioned name to handle undefined symbols
@@ -1563,7 +1563,7 @@ template <class ELFT> void SharedFile::parse() {
         SharedSymbol{*this, saver().save(name), sym.getBinding(), sym.st_other,
                      sym.getType(), sym.st_value, sym.st_size, alignment});
     if (s->file == this)
-      s->verdefIndex = idx;
+      s->versionId = idx;
   }
 }
 

diff  --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 6765461805dadb0..fe3d7f419e84aa6 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -309,7 +309,7 @@ static void replaceWithDefined(Symbol &sym, SectionBase &sec, uint64_t value,
           size, &sec)
       .overwrite(sym);
 
-  sym.verdefIndex = old.verdefIndex;
+  sym.versionId = old.versionId;
   sym.exportDynamic = true;
   sym.isUsedInRegularObj = true;
   // A copy relocated alias may need a GOT entry.

diff  --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index fe7edd5b0eb49aa..b3d97e4de779068 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -92,7 +92,6 @@ Symbol *SymbolTable::insert(StringRef name) {
   memset(sym, 0, sizeof(Symbol));
   sym->setName(name);
   sym->partition = 1;
-  sym->verdefIndex = -1;
   sym->versionId = VER_NDX_GLOBAL;
   if (pos != StringRef::npos)
     sym->hasVersionSuffix = true;
@@ -235,10 +234,9 @@ bool SymbolTable::assignExactVersion(SymbolVersion ver, uint16_t versionId,
         sym->getName().contains('@'))
       continue;
 
-    // If the version has not been assigned, verdefIndex is -1. Use an arbitrary
-    // number (0) to indicate the version has been assigned.
-    if (sym->verdefIndex == uint16_t(-1)) {
-      sym->verdefIndex = 0;
+    // If the version has not been assigned, assign versionId to the symbol.
+    if (!sym->versionScriptAssigned) {
+      sym->versionScriptAssigned = true;
       sym->versionId = versionId;
     }
     if (sym->versionId == versionId)
@@ -256,8 +254,8 @@ void SymbolTable::assignWildcardVersion(SymbolVersion ver, uint16_t versionId,
   // so we set a version to a symbol only if no version has been assigned
   // to the symbol. This behavior is compatible with GNU.
   for (Symbol *sym : findAllByVersion(ver, includeNonDefault))
-    if (sym->verdefIndex == uint16_t(-1)) {
-      sym->verdefIndex = 0;
+    if (!sym->versionScriptAssigned) {
+      sym->versionScriptAssigned = true;
       sym->versionId = versionId;
     }
 }

diff  --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index 671eb56f3404c95..734ca6bfcb4034c 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -683,3 +683,13 @@ void Symbol::resolve(const SharedSymbol &other) {
   } else if (traced)
     printTraceSymbol(other, getName());
 }
+
+void Defined::overwrite(Symbol &sym) const {
+  if (isa_and_nonnull<SharedFile>(sym.file))
+    sym.versionId = VER_NDX_GLOBAL;
+  Symbol::overwrite(sym, DefinedKind);
+  auto &s = static_cast<Defined &>(sym);
+  s.value = value;
+  s.size = size;
+  s.section = section;
+}

diff  --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 4addb79d1257914..e34a31e12f99a5e 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -313,11 +313,12 @@ class Symbol {
   uint32_t auxIdx;
   uint32_t dynsymIndex;
 
-  // This field is a index to the symbol's version definition.
-  uint16_t verdefIndex;
-
-  // Version definition index.
+  // If `file` is SharedFile (for SharedSymbol or copy-relocated Defined), this
+  // represents the Verdef index within the input DSO, which will be converted
+  // to a Verneed index in the output. Otherwise, this represents the Verdef
+  // index (VER_NDX_LOCAL, VER_NDX_GLOBAL, or a named version).
   uint16_t versionId;
+  uint8_t versionScriptAssigned : 1;
 
   void setFlags(uint16_t bits) {
     flags.fetch_or(bits, std::memory_order_relaxed);
@@ -355,14 +356,7 @@ class Defined : public Symbol {
         size(size), section(section) {
     exportDynamic = config->exportDynamic;
   }
-  void overwrite(Symbol &sym) const {
-    Symbol::overwrite(sym, DefinedKind);
-    sym.verdefIndex = -1;
-    auto &s = static_cast<Defined &>(sym);
-    s.value = value;
-    s.size = size;
-    s.section = section;
-  }
+  void overwrite(Symbol &sym) const;
 
   static bool classof(const Symbol *s) { return s->isDefined(); }
 

diff  --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 0f7ebf9d7ba840b..2b32eb3a0fe3558 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -3140,10 +3140,8 @@ bool VersionTableSection::isNeeded() const {
 
 void elf::addVerneed(Symbol *ss) {
   auto &file = cast<SharedFile>(*ss->file);
-  if (ss->verdefIndex == VER_NDX_GLOBAL) {
-    ss->versionId = VER_NDX_GLOBAL;
+  if (ss->versionId == VER_NDX_GLOBAL)
     return;
-  }
 
   if (file.vernauxs.empty())
     file.vernauxs.resize(file.verdefs.size());
@@ -3152,10 +3150,10 @@ void elf::addVerneed(Symbol *ss) {
   // already allocated one. The verdef identifiers cover the range
   // [1..getVerDefNum()]; this causes the vernaux identifiers to start from
   // getVerDefNum()+1.
-  if (file.vernauxs[ss->verdefIndex] == 0)
-    file.vernauxs[ss->verdefIndex] = ++SharedFile::vernauxNum + getVerDefNum();
+  if (file.vernauxs[ss->versionId] == 0)
+    file.vernauxs[ss->versionId] = ++SharedFile::vernauxNum + getVerDefNum();
 
-  ss->versionId = file.vernauxs[ss->verdefIndex];
+  ss->versionId = file.vernauxs[ss->versionId];
 }
 
 template <class ELFT>


        


More information about the llvm-commits mailing list