[lld] [LLD] [COFF] Make weak aliases to implementations take priority over null pointers (PR #190491)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld

Author: Martin Storsjö (mstorsjo)

<details>
<summary>Changes</summary>

Normally, one uses weak aliases in one out of two ways.

Either one uses weak definitions to let a weak definition work as fallback if a strong definition isn't available (which works with link.exe as well), or as a sort of selectany COMDAT, to let multiple weak definitions coexist, letting the linker pick any of them. (This isn't supported by MS link.exe, but requires the LLD extension option -lld-allow-duplicate-weak, normally implied by the mingw mode.)

Or, one uses weak references, to let a referencing translation unit check at runtime, whether a symbol was found (at link time) or not, optionally using the symbol.

In the latter case, the referencing object file provides a fallback value for the weak symbol, as an absolute null symbol.

Previously, if we had multiple weak externals for the same symbol, we'd pick any (the first one in practice); this is compatible with either of the use cases above.

However if both use cases are combined, then we suddenly have multiple conflicting weak externals for the same symbol; both one (or more) symbols providing a fallback implementation for the symbol, and the referencer that provides an absolute null pointer for the symbol.

In these cases, instead of just picking whichever choice the linker saw first, prefer actual concrete implementations over the absolute null symbol.

For mingw mode, this makes linking a translation unit referencing a symbol declared with `__attribute__((weak))` work consistently if linking it against another translation unit providing that symbol defined with `__attribute__((weak))`, regardless of the other that those two object files are linked.

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


3 Files Affected:

- (modified) lld/COFF/InputFiles.cpp (+9-3) 
- (modified) lld/test/COFF/gnu-weak.test (+3) 
- (added) lld/test/COFF/weak-preference.test (+46) 


``````````diff
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index e797da6effb35..67aec91ec5954 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -88,10 +88,16 @@ static void checkAndSetWeakAlias(SymbolTable &symtab, InputFile *f,
         // Weak aliases as produced by GCC are named in the form
         // .weak.<weaksymbol>.<othersymbol>, where <othersymbol> is the name
         // of another symbol emitted near the weak symbol.
-        // Just use the definition from the first object file that defined
-        // this weak symbol.
-        if (symtab.ctx.config.allowDuplicateWeak)
+        if (symtab.ctx.config.allowDuplicateWeak) {
+          // If the alias we had points at absolute zero, and we get another
+          // weak symbol which isn't absolute, prefer that one.
+          if (isa<DefinedAbsolute>(u->weakAlias) &&
+              dyn_cast<DefinedAbsolute>(u->weakAlias)->getVA() == 0 &&
+              !isa<DefinedAbsolute>(target)) {
+            u->setWeakAlias(target, isAntiDep);
+          }
           return;
+        }
         symtab.reportDuplicate(source, f);
       }
     }
diff --git a/lld/test/COFF/gnu-weak.test b/lld/test/COFF/gnu-weak.test
index 08e59734e0424..3173992f4114a 100644
--- a/lld/test/COFF/gnu-weak.test
+++ b/lld/test/COFF/gnu-weak.test
@@ -9,6 +9,9 @@ GNU ld can handle several definitions of the same weak symbol, and
 unless there is a strong definition of it, it just picks the first
 weak definition encountered.
 
+(LLD extends this, to prefer an actual concrete definition of it,
+over a definition in the form of absolute null.)
+
 For each of the weak definitions, GNU tools produce a regular symbol
 named .weak.<weaksymbol>.<othersymbol>, where the other symbol name is
 another symbol defined close by.
diff --git a/lld/test/COFF/weak-preference.test b/lld/test/COFF/weak-preference.test
new file mode 100644
index 0000000000000..a2363f5dc6265
--- /dev/null
+++ b/lld/test/COFF/weak-preference.test
@@ -0,0 +1,46 @@
+REQUIRES: x86
+RUN: split-file %s %t.dir && cd %t.dir
+
+RUN: llvm-mc -filetype=obj -triple=x86_64-windows weak-ref.s -o weak-ref.obj
+RUN: llvm-mc -filetype=obj -triple=x86_64-windows weak-def.s -o weak-def.obj
+
+RUN: env LLD_IN_TEST=1 not lld-link -entry:entry -subsystem:console -out:test.exe weak-ref.obj weak-def.obj 2>&1 | FileCheck -check-prefix=ERR %s
+
+ERR: error: duplicate symbol: weakfunc
+
+
+Test that we get the same choice for weakfunc (pointing at the definition
+in weak-def.s, not the absolute null fallback provided by weak-ref.s),
+regardless of which object is linked first.
+
+RUN: lld-link -lld-allow-duplicate-weak -entry:entry -subsystem:console -out:test.exe weak-def.obj weak-ref.obj
+RUN: llvm-objdump -s test.exe | FileCheck %s
+RUN: lld-link -lld-allow-duplicate-weak -entry:entry -subsystem:console -out:test.exe weak-ref.obj weak-def.obj
+RUN: llvm-objdump -s test.exe | FileCheck %s
+
+CHECK:      Contents of section .rdata:
+CHECK-NEXT:  140002000 14100040 01000000
+
+#--- weak-ref.s
+	.text
+	.globl	entry
+entry:
+	cmpq	$0, .refptr.weakfunc(%rip)
+	je	.LBB0_1
+	rex64 jmpq	*.refptr.weakfunc(%rip)
+.LBB0_1:
+	retq
+
+	.section	.rdata$.refptr.weakfunc,"dr",discard,.refptr.weakfunc
+	.globl	.refptr.weakfunc
+.refptr.weakfunc:
+	.quad	weakfunc
+
+	.weak	weakfunc
+
+#--- weak-def.s
+	.section	.text$b,"xr"
+	.balign	4
+	.weak	weakfunc
+weakfunc:
+	retq

``````````

</details>


https://github.com/llvm/llvm-project/pull/190491