[PATCH] D111365: [ELF] Demote !isUsedInRegularObj lazy symbol

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: peter.smith, smeenai.
Herald added subscribers: arichardson, emaste.
MaskRay requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

I think D79300 <https://reviews.llvm.org/D79300> has fixed the D51892 <https://reviews.llvm.org/D51892> (`__i686.get_pc_thunk.bx`) issue, so
we can bring back rL330869 <https://reviews.llvm.org/rL330869>.
D79300 <https://reviews.llvm.org/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()`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111365

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


Index: lld/ELF/SyntheticSections.cpp
===================================================================
--- lld/ELF/SyntheticSections.cpp
+++ lld/ELF/SyntheticSections.cpp
@@ -3161,10 +3161,9 @@
 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.
+    write16(buf, s.sym->versionId);
     buf += 2;
   }
 }
Index: lld/ELF/Symbols.h
===================================================================
--- lld/ELF/Symbols.h
+++ lld/ELF/Symbols.h
@@ -163,10 +163,7 @@
 
   // 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)
Index: lld/ELF/Driver.cpp
===================================================================
--- lld/ELF/Driver.cpp
+++ lld/ELF/Driver.cpp
@@ -1838,12 +1838,15 @@
   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;
   }
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D111365.378036.patch
Type: text/x-patch
Size: 2168 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211007/e050787b/attachment.bin>