[lld] Revert "[ELF] Merge verdefIndex into versionId. NFC" #72208 (PR #72484)

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 15 23:13:38 PST 2023


https://github.com/MaskRay created https://github.com/llvm/llvm-project/pull/72484

Reverts llvm/llvm-project#72208

If a unversioned Defined preempts a versioned DSO definition, the version ID will not be reset.

>From 2e889e7e28dacf3ba10d84f8addef0c5f8c46788 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Wed, 15 Nov 2023 23:12:17 -0800
Subject: [PATCH] Revert "[ELF] Merge verdefIndex into versionId. NFC (#72208)"

This reverts commit 667ea2ca408bbfb29f6d138b6c758542e329b2c5.
---
 lld/ELF/InputFiles.cpp        |  4 ++--
 lld/ELF/Relocations.cpp       |  1 +
 lld/ELF/SymbolTable.cpp       | 12 +++++++-----
 lld/ELF/Symbols.h             | 10 +++++-----
 lld/ELF/SyntheticSections.cpp | 10 ++++++----
 5 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 4b4d7d6db93cd57..8c7f2c8773f2cbc 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->versionId = ver;
+        s->verdefIndex = 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->versionId = idx;
+      s->verdefIndex = idx;
   }
 }
 
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index d4685ce70ece8cb..6765461805dadb0 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -309,6 +309,7 @@ static void replaceWithDefined(Symbol &sym, SectionBase &sec, uint64_t value,
           size, &sec)
       .overwrite(sym);
 
+  sym.verdefIndex = old.verdefIndex;
   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 b3d97e4de779068..fe7edd5b0eb49aa 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -92,6 +92,7 @@ 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;
@@ -234,9 +235,10 @@ bool SymbolTable::assignExactVersion(SymbolVersion ver, uint16_t versionId,
         sym->getName().contains('@'))
       continue;
 
-    // If the version has not been assigned, assign versionId to the symbol.
-    if (!sym->versionScriptAssigned) {
-      sym->versionScriptAssigned = true;
+    // 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;
       sym->versionId = versionId;
     }
     if (sym->versionId == versionId)
@@ -254,8 +256,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->versionScriptAssigned) {
-      sym->versionScriptAssigned = true;
+    if (sym->verdefIndex == uint16_t(-1)) {
+      sym->verdefIndex = 0;
       sym->versionId = versionId;
     }
 }
diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 0c79efccb548680..4addb79d1257914 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -313,12 +313,11 @@ class Symbol {
   uint32_t auxIdx;
   uint32_t dynsymIndex;
 
-  // For a Defined symbol, this represents the Verdef index (VER_NDX_LOCAL,
-  // VER_NDX_GLOBAL, or a named version). For a SharedSymbol, this represents
-  // the Verdef index within the input DSO, which will be converted to a Verneed
-  // index in the output.
+  // This field is a index to the symbol's version definition.
+  uint16_t verdefIndex;
+
+  // Version definition index.
   uint16_t versionId;
-  uint8_t versionScriptAssigned : 1;
 
   void setFlags(uint16_t bits) {
     flags.fetch_or(bits, std::memory_order_relaxed);
@@ -358,6 +357,7 @@ class Defined : public Symbol {
   }
   void overwrite(Symbol &sym) const {
     Symbol::overwrite(sym, DefinedKind);
+    sym.verdefIndex = -1;
     auto &s = static_cast<Defined &>(sym);
     s.value = value;
     s.size = size;
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 2b32eb3a0fe3558..0f7ebf9d7ba840b 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -3140,8 +3140,10 @@ bool VersionTableSection::isNeeded() const {
 
 void elf::addVerneed(Symbol *ss) {
   auto &file = cast<SharedFile>(*ss->file);
-  if (ss->versionId == VER_NDX_GLOBAL)
+  if (ss->verdefIndex == VER_NDX_GLOBAL) {
+    ss->versionId = VER_NDX_GLOBAL;
     return;
+  }
 
   if (file.vernauxs.empty())
     file.vernauxs.resize(file.verdefs.size());
@@ -3150,10 +3152,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->versionId] == 0)
-    file.vernauxs[ss->versionId] = ++SharedFile::vernauxNum + getVerDefNum();
+  if (file.vernauxs[ss->verdefIndex] == 0)
+    file.vernauxs[ss->verdefIndex] = ++SharedFile::vernauxNum + getVerDefNum();
 
-  ss->versionId = file.vernauxs[ss->versionId];
+  ss->versionId = file.vernauxs[ss->verdefIndex];
 }
 
 template <class ELFT>



More information about the llvm-commits mailing list