[lld] 71ec1e5 - [ELF] Demote !isUsedInRegularObj lazy symbol

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2021-10-11T09:46:31-07:00
New Revision: 71ec1e501572ed327c16cb958b93652ea0d74f6f

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

LOG: [ELF] Demote !isUsedInRegularObj lazy symbol

I think D79300 has fixed the D51892 (`__i686.get_pc_thunk.bx`) issue, so
we can bring back rL330869.
D79300 says `would error undefined symbol instead of the more relevant discarded section`
but it doesn't reproduce now.

This avoids a quirk in `isUndefWeak()`.

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D111365

Added: 
    

Modified: 
    lld/ELF/Driver.cpp
    lld/ELF/Symbols.h
    lld/ELF/SyntheticSections.cpp

Removed: 
    


################################################################################
diff  --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 1b9154a190624..9b8b34e14f589 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1838,12 +1838,15 @@ static void demoteSharedSymbols() {
   llvm::TimeTraceScope timeScope("Demote shared symbols");
   for (Symbol *sym : symtab->symbols()) {
     auto *s = dyn_cast<SharedSymbol>(sym);
-    if (!s || s->getFile().isNeeded)
+    if (!((s && !s->getFile().isNeeded) ||
+          (sym->isLazy() && sym->isUsedInRegularObj)))
       continue;
 
-    bool used = s->used;
-    s->replace(Undefined{nullptr, s->getName(), STB_WEAK, s->stOther, s->type});
-    s->used = used;
+    bool used = sym->used;
+    sym->replace(
+        Undefined{nullptr, sym->getName(), STB_WEAK, sym->stOther, sym->type});
+    sym->used = used;
+    sym->versionId = VER_NDX_GLOBAL;
   }
 }
 

diff  --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 4f5cc3f67631e..816d61563021e 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -163,10 +163,7 @@ class Symbol {
 
   // True if this is an undefined weak symbol. This only works once
   // all input files have been added.
-  bool isUndefWeak() const {
-    // See comment on lazy symbols for details.
-    return isWeak() && (isUndefined() || isLazy());
-  }
+  bool isUndefWeak() const { return isWeak() && isUndefined(); }
 
   StringRef getName() const {
     if (nameSize == (uint32_t)-1)

diff  --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 187b2ac90c21a..b3e5ede0ab12f 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -3161,10 +3161,10 @@ size_t VersionTableSection::getSize() const {
 void VersionTableSection::writeTo(uint8_t *buf) {
   buf += 2;
   for (const SymbolTableEntry &s : getPartition().dynSymTab->getSymbols()) {
-    // Use the original versionId for an unfetched lazy symbol (undefined weak),
-    // which must be VER_NDX_GLOBAL (an undefined versioned symbol is an error).
-    write16(buf, s.sym->isLazy() ? static_cast<uint16_t>(VER_NDX_GLOBAL)
-                                 : s.sym->versionId);
+    // For an unfetched 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);
     buf += 2;
   }
 }