To check I understand as I got a bit confused with the three pull requests:
llvm:main ← MaskRay:lld-no-allow-shlib-undefined-hidden
opened 05:56AM - 31 Oct 23 UTC
For a DSO with all DT_NEEDED entries accounted for, if it contains an
undefined … non-weak symbol that shares a name with a non-exported
definition (hidden visibility or localized by a version script), and
there is no DSO definition, we should also report an error. Because the
hidden definition is not exported, it cannot resolve the DSO reference
at runtime.
GNU ld introduced this error-checking in [April
2003](https://sourceware.org/pipermail/binutils/2003-April/026568.html).
The feature is available for executable links but not for -shared, and it is
orthogonal to --no-allow-shlib-undefined. We make the feature part of
--no-allow-shlib-undefined and work with -shared when --no-allow-shlib-undefined
is specified.
A subset of this error-checking is covered by commit
1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40 for --gc-sections discarded
sections. This patch covers non-discarded sections as well.
Internally, I have identified 2 bugs (which would fail with
LD_BIND_NOW=1) covered by commit
1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40
llvm:main ← MaskRay:lld-hidden-preempt-shared
opened 09:49PM - 24 Oct 23 UTC
Commit 1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40 unexpectedly made --no-allow-shl… ib-undefined stronger:
* 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).
When all the conditions are satisfied, the new --no-allow-shlib-undefined code
reports an error.
Technically, the hidden Defined in the executable can be intentional: it can be
meant to remain non-exported and not interact with any dynamic symbols of the same
name that might exist in other DSOs. To allow for such use cases, add a new bit in
Symbols::flags and relax the --no-allow-shlib-undefined check to before commit 1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40.
---
Possible future extension: create a linker option to error about the cases
ignored by this patch, and generalize it to included garbage-collected Defined
and DSO definitions. I have managed to identify several unintended
hidden/default duplicate symbol issues internally.
I haven't yet identified an entirely legitimate use case of a hidden Defined
"preempting" a DSO undef/def. In addition, the option can apply to DSO
definitions as well, not just `requiredSymbols`.
llvm:main ← MaskRay:lld-allow-hidden-symbols-shared-with-dso
opened 05:11AM - 25 Oct 23 UTC
Commit 1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40 unexpectedly strengthened
--no-a… llow-shlib-undefined which will be relaxed by #70130.
This patch adds --no-allow-non-exported-symbols-shared-with-dso to restore
the check and make the check catch more cases:
* report errors in the presence of a DSO definition
* make the check work whether or not --gc-sections discards the symbol
Commit 1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40 has caught several brittle
build issues. For example,
```
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`. This
scenarios is often undesired. In this case, a possible improvement 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.
---
* I wonder whether this option is useful enough to justify a new option.
* Using protected visibility can cause similar multiple definition issues. Is it
useful to generalize this option?
All three pull requests contain #a350e004480dc40f25a18186657b266df5cde584 which will prevent lld from giving an error when there is another shared-definition, but a garbage collected local definition masks it.
[ELF] Enhance --no-allow-shlib-undefined to report non-exported definition by MaskRay · Pull Request #70769 · llvm/llvm-project · GitHub also contains a follow up change that checks whether the local definition is actually exported into the dynamic symbol and, like GNU ld, gives an error message for this case.
[ELF] Add --[no-]allow-non-exported-symbols-shared-with-dso by MaskRay · Pull Request #70163 · llvm/llvm-project · GitHub contains a different follow up change that adds --[no-]allow-non-exported-symbols-shared-with-dso which gives an error message if lld detects a non-exported definition in the executable and a separate shared def of the same symbol, which is a, potentially benign, ODR violation.
Do you want me to concentrate on [ELF] Suppress --no-allow-shlib-undefined diagnostic when a SharedSymbol is overridden by a hidden visibility Defined which is later discarded by MaskRay · Pull Request #70130 · llvm/llvm-project · GitHub first? Then both [ELF] Add --[no-]allow-non-exported-symbols-shared-with-dso by MaskRay · Pull Request #70163 · llvm/llvm-project · GitHub and [ELF] Enhance --no-allow-shlib-undefined to report non-exported definition by MaskRay · Pull Request #70769 · llvm/llvm-project · GitHub can be handled separately.
FWIW I think #70163 looks like the right semanitcs for --no-allow-shlib-undefined. I’m on the fence as to whether --[no-]allow-non-exported-symbols-shared-with-dso will ever get used.