Backport "[ELF] Make shouldAddProvideSym return values consistent when demoted to Undefined"

[dianqk]

Backport #111945.

(cherry picked from commit 1c6688a)

[llvmbot]

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: DianQK (DianQK)

Changes

Backport #111945.

(cherry picked from commit 1c6688a)

---

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

2 Files Affected:

• (modified) lld/ELF/LinkerScript.cpp (+8-1)
• (added) lld/test/ELF/linkerscript/provide-defined.s (+36)

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);
+}

[dianqk]

Mark as draft due to #111945 (comment).

[dianqk]

Now we also need to backport #112386. Would it be safer to revert ebb326a in release/19.x?

[MaskRay]

Now we also need to backport #112386. Would it be safer to revert ebb326a in release/19.x?

We could revert it, but I do not feel that the PROVIDE regression #111478 is important. I prefer that we do nothing for the release branch.