Backport "[ELF] Make shouldAddProvideSym return values consistent when demoted to Undefined" #112136
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…to Undefined
Case: `PROVIDE(f1 = bar);` when both `f1` and `bar` are in separate
sections that would be discarded by GC.
Due to `demoteDefined`, `shouldAddProvideSym(f1)` may initially return
false (when Defined) and then return true (been demoted to Undefined).
```
addScriptReferencedSymbolsToSymTable
shouldAddProvideSym(f1): false
// the RHS (bar) is not added to `referencedSymbols` and may be GCed
declareSymbols
shouldAddProvideSym(f1): false
markLive
demoteSymbolsAndComputeIsPreemptible
// demoted f1 to Undefined
processSymbolAssignments
addSymbol
shouldAddProvideSym(f1): true
```
The inconsistency can cause `cmd->expression()` in `addSymbol` to be
evaluated, leading to `symbol not found: bar` errors (since `bar` in the
RHS is not in `referencedSymbols` and is GCed) (llvm#111478).
Fix this by adding a `sym->isUsedInRegularObj` condition, making
`shouldAddProvideSym(f1)` values consistent. In addition, we need a
`sym->exportDynamic` condition to keep provide-shared.s working.
Fixes: ebb326a
Pull Request: llvm#111945
(cherry picked from commit 1c6688a)
|
@llvm/pr-subscribers-lld-elf @llvm/pr-subscribers-lld Author: DianQK (DianQK) ChangesBackport #111945. (cherry picked from commit 1c6688a) Full diff: https://github.com/llvm/llvm-project/pull/112136.diff 2 Files Affected:
diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index 055fa21d44ca6e..d95c5573935ec4 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -1718,6 +1718,13 @@ void LinkerScript::addScriptReferencedSymbolsToSymTable() {
}
bool LinkerScript::shouldAddProvideSym(StringRef symName) {
+ // This function is called before and after garbage collection. To prevent
+ // undefined references from the RHS, the result of this function for a
+ // symbol must be the same for each call. We use isUsedInRegularObj to not
+ // change the return value of a demoted symbol. The exportDynamic condition,
+ // while not so accurate, allows PROVIDE to define a symbol referenced by a
+ // DSO.
Symbol *sym = symtab.find(symName);
- return sym && !sym->isDefined() && !sym->isCommon();
+ return sym && !sym->isDefined() && !sym->isCommon() &&
+ (sym->isUsedInRegularObj || sym->exportDynamic);
}
diff --git a/lld/test/ELF/linkerscript/provide-defined.s b/lld/test/ELF/linkerscript/provide-defined.s
new file mode 100644
index 00000000000000..1d44bef3d4068d
--- /dev/null
+++ b/lld/test/ELF/linkerscript/provide-defined.s
@@ -0,0 +1,36 @@
+# REQUIRES: x86
+## Test the GC behavior when the PROVIDE symbol is defined by a relocatable file.
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64 a.s -o a.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 b.s -o b.o
+# RUN: ld.lld -T a.t --gc-sections a.o b.o -o a
+# RUN: llvm-readelf -s a | FileCheck %s
+
+# CHECK: 1: {{.*}} 0 NOTYPE GLOBAL DEFAULT 1 _start
+# CHECK-NEXT:2: {{.*}} 0 NOTYPE GLOBAL DEFAULT 2 f3
+# CHECK-NOT: {{.}}
+
+#--- a.s
+.global _start, f1, f2, f3, bar
+_start:
+ call f3
+
+.section .text.f1,"ax"; f1:
+.section .text.f2,"ax"; f2: # referenced by another relocatable file
+.section .text.f3,"ax"; f3: # live
+.section .text.bar,"ax"; bar:
+
+.comm comm,4,4
+
+#--- b.s
+ call f2
+
+#--- a.t
+SECTIONS {
+ . = . + SIZEOF_HEADERS;
+ PROVIDE(f1 = bar+1);
+ PROVIDE(f2 = bar+2);
+ PROVIDE(f3 = bar+3);
+ PROVIDE(f4 = comm+4);
+}
|
MaskRay
approved these changes
Oct 13, 2024
|
Mark as draft due to #111945 (comment). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport #111945.
(cherry picked from commit 1c6688a)