[lld] f10441a - [ELF] Refine includeInDynsym condition

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2025-01-27T22:02:27-08:00
New Revision: f10441ad003236ef3b9e5415a571d2be0c0ce5ce

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

LOG: [ELF] Refine includeInDynsym condition

`includeInDynsym` has a special case for isUndefWeak and
--no-dynamic-linker, which can be removed if we simplify disallow
dynamic symbols for static-pie.

The partition feature reports errors only when a symbol `isExported`.
We need to link in a DSO to trigger the mips error.

Added: 
    

Modified: 
    lld/ELF/Config.h
    lld/ELF/Driver.cpp
    lld/ELF/Symbols.cpp
    lld/ELF/Writer.cpp
    lld/test/ELF/partition-errors.s

Removed: 
    


################################################################################
diff  --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index b2859486d58e93..1d47645edcd83e 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -654,6 +654,8 @@ struct Ctx : CommonLinkerContext {
   std::unique_ptr<llvm::TarWriter> tar;
   // InputFile for linker created symbols with no source location.
   InputFile *internalFile = nullptr;
+  // True if symbols can be exported (isExported) or preemptible.
+  bool hasDynsym = false;
   // True if SHT_LLVM_SYMPART is used.
   std::atomic<bool> hasSympart{false};
   // True if there are TLS IE relocations. Set DF_STATIC_TLS if -shared.

diff  --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 06c93710497ed8..9d0c992c1e8516 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2554,7 +2554,7 @@ void LinkerDriver::compileBitcodeFiles(bool skipLinkedOutput) {
       for (Symbol *sym : obj->getGlobalSymbols()) {
         if (!sym->isDefined())
           continue;
-        if (sym->includeInDynsym(ctx))
+        if (ctx.hasDynsym && sym->includeInDynsym(ctx))
           sym->isExported = true;
         if (sym->hasVersionSuffix)
           sym->parseSymbolVersion(ctx);
@@ -2899,8 +2899,12 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
 
   parseFiles(ctx, files);
 
+  // Dynamic linking is used if there is an input DSO,
+  // or -shared or non-static pie is specified.
+  ctx.hasDynsym = !ctx.sharedFiles.empty() || ctx.arg.shared ||
+                  (ctx.arg.pie && !ctx.arg.noDynamicLinker);
   // Create dynamic sections for dynamic linking and static PIE.
-  ctx.arg.hasDynSymTab = !ctx.sharedFiles.empty() || ctx.arg.isPic;
+  ctx.arg.hasDynSymTab = ctx.hasDynsym || ctx.arg.isPic;
 
   // If an entry symbol is in a static archive, pull out that file now.
   if (Symbol *sym = ctx.symtab->find(ctx.arg.entry))

diff  --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index 497db842bc9be6..b10391c65dfdc3 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -272,11 +272,7 @@ bool Symbol::includeInDynsym(Ctx &ctx) const {
   if (computeBinding(ctx) == STB_LOCAL)
     return false;
   if (!isDefined() && !isCommon())
-    // This should unconditionally return true, unfortunately glibc -static-pie
-    // expects undefined weak symbols not to exist in .dynsym, e.g.
-    // __pthread_mutex_lock reference in _dl_add_to_namespace_list,
-    // __pthread_initialize_minimal reference in csu/libc-start.c.
-    return !(isUndefWeak() && ctx.arg.noDynamicLinker);
+    return true;
 
   return exportDynamic ||
          (ctx.arg.exportDynamic && (isUsedInRegularObj || !ltoCanOmit));
@@ -374,13 +370,14 @@ void elf::parseVersionAndComputeIsPreemptible(Ctx &ctx) {
   // Symbol themselves might know their versions because symbols
   // can contain versions in the form of <name>@<version>.
   // Let them parse and update their names to exclude version suffix.
-  bool hasDynSymTab = ctx.arg.hasDynSymTab;
+  bool hasDynsym = ctx.hasDynsym;
   for (Symbol *sym : ctx.symtab->getSymbols()) {
     if (sym->hasVersionSuffix)
       sym->parseSymbolVersion(ctx);
-    sym->isExported = sym->includeInDynsym(ctx);
-    if (hasDynSymTab)
+    if (hasDynsym) {
+      sym->isExported = sym->includeInDynsym(ctx);
       sym->isPreemptible = sym->isExported && computeIsPreemptible(ctx, *sym);
+    }
   }
 }
 

diff  --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 8ce0b68c9f29cd..535d7871a18ad3 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -282,6 +282,7 @@ static void demoteDefined(Defined &sym, DenseMap<SectionBase *, size_t> &map) {
 static void demoteSymbolsAndComputeIsPreemptible(Ctx &ctx) {
   llvm::TimeTraceScope timeScope("Demote symbols");
   DenseMap<InputFile *, DenseMap<SectionBase *, size_t>> sectionIndexMap;
+  bool hasDynsym = ctx.hasDynsym;
   for (Symbol *sym : ctx.symtab->getSymbols()) {
     if (auto *d = dyn_cast<Defined>(sym)) {
       if (d->section && !d->section->isLive())
@@ -297,9 +298,10 @@ static void demoteSymbolsAndComputeIsPreemptible(Ctx &ctx) {
       }
     }
 
-    sym->isExported = sym->includeInDynsym(ctx);
-    if (ctx.arg.hasDynSymTab)
+    if (hasDynsym) {
+      sym->isExported = sym->includeInDynsym(ctx);
       sym->isPreemptible = sym->isExported && computeIsPreemptible(ctx, *sym);
+    }
   }
 }
 

diff  --git a/lld/test/ELF/partition-errors.s b/lld/test/ELF/partition-errors.s
index 8962ded161f170..476f377640871b 100644
--- a/lld/test/ELF/partition-errors.s
+++ b/lld/test/ELF/partition-errors.s
@@ -12,8 +12,10 @@
 // RUN: not ld.lld --export-dynamic %ts.so %t.o -Tdata=0 2>&1 | FileCheck %s
 // RUN: not ld.lld --export-dynamic %ts.so %t.o -Tbss=0 2>&1 | FileCheck %s
 
+// RUN: llvm-mc %S/Inputs/shared.s -o %ts.o -filetype=obj --triple=mipsel-unknown-linux
+// RUN: ld.lld -shared -soname=ts %ts.o -o %ts.so
 // RUN: llvm-mc -triple=mipsel-unknown-linux -filetype=obj -o %t2.o %s
-// RUN: not ld.lld --export-dynamic %t2.o 2>&1 | FileCheck %s
+// RUN: not ld.lld --export-dynamic %t2.o %ts.so 2>&1 | FileCheck %s
 
 // CHECK: error: {{.*}}.o: partitions cannot be used