tl;dr Do we want a linker option to catch certain multiple definition issues involving DSO that are less likely benign?
Commit 1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40 unexpectedly made --no-allow-shlib-undefined stronger. When all the three conditions are satisfied, the new --no-allow-shlib-undefined code reports an error.
- There is a DSO undef that can be satisfied by a SharedSymbol.
- The SharedSymbol is overridden by a hidden visibility Defined.
- The hidden visibility Defined can be garbage-collected (it is not part of .dynsym and is not marked as live).
You may read https://github.com/llvm/llvm-project/blob/fa19ef7a10869bf0f8325681be111f7d97b2544e/lld/test/ELF/allow-shlib-undefined.s#L46 to get a sense of the error checking.
However, I am considering restoring the previous behavior (suppressing this error) in #70130.
That said, commit 1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40 does catch some brittle builds.
For example,
% ld.lld @response.txt -y _Znam
...
libfdio.so: reference to _Znam
libclang_rt.asan.so: shared definition of _Znam
libc++.a(stdlib_new_delete.cpp.obj): definition of _Znam
The executable contains a definition from libc++.a, while at run-time, libfdio.soās _Znam reference gets resolved to libclang_rt.asan.so. libclang_rt.asan.so exports _Znam, so when it is linked, the definition is either default or protected visibility, definitely different from the hidden visibility libc++.a.
This scenario implies two different _Znam implementations, which is less likely benign ODR violation. A possible fix is to switch to an asan-instrumented libc++.a that does not define _Znam.
I have also seen problems due to mixing multiple definitions from libgcc.a (hidden visibility) and libclang_rt.builtins.a (default visibility) and relying on archive member extraction rules to work.
Some groups compile relocatable object files with -fvisibility=hidden to allow just static linking. Nevertheless, their system consists of certain shared objects and there is a probability of messing up multiple definition symbols.
- Does it make sense to add an option to catch the brittle case? If yes, lld allow hidden symbols shared with dso by MaskRay Ā· Pull Request #70163 Ā· llvm/llvm-project Ā· GitHub provides the implementation
- Disable the check by default?
- Does it make sense to extend the check to protected visibility?
- Does it make sense to extend the check to default visibility? This would allow us to know preempted symbols for link-time shared objects. However, I suspect that there are a lot of benign violations and in the absence of an ignorelist mechanism, this will not be useful.
If the option covers more than one visibility, it seems challenging to pick a precise option name.
One candidate is --allowed-visibility-for-symbols-shared-with-dso=[none,all,default,default+protected,default+protected+hidden]
If the textual description is difficult to understand, the following test demonstrates the proposed error checking:
# REQUIRES: x86
# 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 weak.s -o weak.o
# RUN: llvm-mc -filetype=obj -triple=x86_64 def.s -o def.o
# RUN: ld.lld -shared -soname=def.so def.o -o def.so
# RUN: ld.lld a.o def.so -o /dev/null
# RUN: ld.lld a.o def.so -o /dev/null --no-allow-hidden-symbols-shared-with-dso --allow-hidden-symbols-shared-with-dso
# RUN: not ld.lld a.o def.so --no-allow-hidden-symbols-shared-with-dso 2>&1 | FileCheck %s --check-prefix=CHECK1
# RUN: not ld.lld --gc-sections weak.o def.so a.o --no-allow-hidden-symbols-shared-with-dso 2>&1 | FileCheck %s --check-prefix=CHECK1
# CHECK1: error: hidden symbol also defined by DSO: foo
# CHECK1-NEXT: >>> defined by {{.*}}a.o
# RUN: llvm-mc -filetype=obj -triple=x86_64 ref.s -o ref.o
# RUN: ld.lld -shared -soname=ref.so ref.o -o ref.so
# RUN: not ld.lld a.o ref.so --no-allow-hidden-symbols-shared-with-dso 2>&1 | FileCheck %s --check-prefix=CHECK2
# CHECK2: error: hidden symbol also referenced by DSO: foo
# CHECK2-NEXT: >>> defined by {{.*}}a.o
## See also allow-shlib-undefined.s.
#--- a.s
.globl _start, foo
_start:
## The check kicks in even if .text.foo is discarded.
.section .text.foo,"ax"
.hidden foo
foo:
#--- weak.s
.weak foo
foo:
#--- def.s
.globl foo
foo:
#--- ref.s
call foo