Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ELF] PROVIDE: fix spurious "symbol not found" #87530

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Apr 3, 2024

When archive member extraction involving ENTRY happens after
addScriptReferencedSymbolsToSymTable,
addScriptReferencedSymbolsToSymTable may fail to define some PROVIDE
symbols used by ENTRY. This is an edge case that regressed after #84512.
(The interaction with PROVIDE and ENTRY-in-archive was not considered
before).

While here, also ensure that --undefined-glob extracted object files
are parsed before addScriptReferencedSymbolsToSymTable.

Fixes: ebb326a

Created using spr 1.3.5-bogner
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 3, 2024

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: Fangrui Song (MaskRay)

Changes

When archive member extraction involving ENTRY happens after
addScriptReferencedSymbolsToSymTable,
addScriptReferencedSymbolsToSymTable may fail to define some PROVIDE
symbols used by ENTRY. This is an edge case that regressed after #84512.
(The interaction with PROVIDE and ENTRY-in-archive was not considered
before).

Fixes: ebb326a


Full diff: https://github.com/llvm/llvm-project/pull/87530.diff

2 Files Affected:

  • (modified) lld/ELF/Driver.cpp (+10-7)
  • (modified) lld/test/ELF/linkerscript/symbolreferenced.s (+25)
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index b43da7727e22ae..451c0770fb2e13 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2727,13 +2727,6 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
   // Create dynamic sections for dynamic linking and static PIE.
   config->hasDynSymTab = !ctx.sharedFiles.empty() || config->isPic;
 
-  script->addScriptReferencedSymbolsToSymTable();
-
-  // Prevent LTO from removing any definition referenced by -u.
-  for (StringRef name : config->undefined)
-    if (Defined *sym = dyn_cast_or_null<Defined>(symtab.find(name)))
-      sym->isUsedInRegularObj = true;
-
   // If an entry symbol is in a static archive, pull out that file now.
   if (Symbol *sym = symtab.find(config->entry))
     handleUndefined(sym, "--entry");
@@ -2742,6 +2735,16 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
   for (StringRef pat : args::getStrings(args, OPT_undefined_glob))
     handleUndefinedGlob(pat);
 
+  // After potential archive member extraction involving ENTRY and
+  // -u/--undefined-glob, check whether PROVIDE symbols should be defined (the
+  // RHS may refer to definitions in just extracted object files).
+  script->addScriptReferencedSymbolsToSymTable();
+
+  // Prevent LTO from removing any definition referenced by -u.
+  for (StringRef name : config->undefined)
+    if (Defined *sym = dyn_cast_or_null<Defined>(symtab.find(name)))
+      sym->isUsedInRegularObj = true;
+
   // Mark -init and -fini symbols so that the LTO doesn't eliminate them.
   if (Symbol *sym = dyn_cast_or_null<Defined>(symtab.find(config->init)))
     sym->isUsedInRegularObj = true;
diff --git a/lld/test/ELF/linkerscript/symbolreferenced.s b/lld/test/ELF/linkerscript/symbolreferenced.s
index 6f583d20e27641..6848082690837f 100644
--- a/lld/test/ELF/linkerscript/symbolreferenced.s
+++ b/lld/test/ELF/linkerscript/symbolreferenced.s
@@ -50,6 +50,21 @@
 # RUN: not ld.lld -T chain2.t a.o 2>&1 | FileCheck %s --check-prefix=ERR --implicit-check-not=error:
 # ERR-COUNT-3: error: chain2.t:1: symbol not found: undef
 
+## _start in a lazy object file references PROVIDE symbols. We extract _start
+## earlier to avoid spurious "symbol not found" errors.
+# RUN: llvm-mc -filetype=obj -triple=x86_64 undef.s -o undef.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 start.s -o start.o
+# RUN: ld.lld -T chain2.t undef.o --start-lib start.o --end-lib -o lazy
+# RUN: llvm-nm lazy | FileCheck %s --check-prefix=LAZY
+# RUN: ld.lld -e 0 -T chain2.t --undefined-glob '_start*' undef.o --start-lib start.o --end-lib -o lazy
+# RUN: llvm-nm lazy | FileCheck %s --check-prefix=LAZY
+
+# LAZY:      T _start
+# LAZY-NEXT: t f1
+# LAZY-NEXT: T f2
+# LAZY-NEXT: T newsym
+# LAZY-NEXT: T unde
+
 #--- a.s
 .global _start
 _start:
@@ -89,3 +104,13 @@ PROVIDE(newsym = f1);
 PROVIDE(f2 = undef);
 PROVIDE_HIDDEN(f1 = f2);
 PROVIDE(newsym = f1);
+
+#--- undef.s
+.globl undef
+undef: ret
+
+#--- start.s
+.globl _start
+_start: ret
+.data
+.quad newsym

@MaskRay
Copy link
Member Author

MaskRay commented Apr 3, 2024

@partaror

Copy link
Contributor

@partaror partaror left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the quick fix!

Copy link
Collaborator

@statham-arm statham-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked this against the real cases where we were having link failures, and confirmed that it solves those as well as my 3-line reduced examples in #84512.

LGTM. Thanks for the very quick fix!

MaskRay added a commit that referenced this pull request Apr 4, 2024
When archive member extraction involving ENTRY happens after
`addScriptReferencedSymbolsToSymTable`,
`addScriptReferencedSymbolsToSymTable` may fail to define some PROVIDE
symbols used by ENTRY. This is an edge case that regressed after #84512.
(The interaction with PROVIDE and ENTRY-in-archive was not considered
before).

While here, also ensure that --undefined-glob extracted object files
are parsed before `addScriptReferencedSymbolsToSymTable`.

Fixes: ebb326a

Pull Request: #87530
@MaskRay
Copy link
Member Author

MaskRay commented Apr 4, 2024

Landed as dcc45fa.

@MaskRay MaskRay closed this Apr 4, 2024
@MaskRay MaskRay deleted the users/MaskRay/spr/elf-provide-fix-spurious-symbol-not-found branch April 4, 2024 17:27
@vitalybuka
Copy link
Collaborator

This on another ELF related commit breaks https://lab.llvm.org/buildbot/#/builders/5/builds/42341

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants