# Relocatable file definitions shared with DSO

**URL:** <https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391>\
**Category:** LLD\
**Created:** [October 25, 2023, 5:35am UTC](https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391 "2023-10-25T05:35:17Z")\
**Posts on this page:** 10\
**Page:** 1

<div class="post-metadata">

**Author:** ![MaskRay](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/maskray/32/81_2.png) [@MaskRay](https://discourse.llvm.org/u/MaskRay)\
**Post date:** [October 25, 2023, 5:35am UTC](https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391/1 "2023-10-25T05:35:17Z")

</div>

tl;dr Do we want a linker option to catch certain multiple definition issues involving DSO that are less likely benign?

[Commit 1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40](https://github.com/llvm/llvm-project/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](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](https://github.com/llvm/llvm-project/pull/70130).

That said, [commit 1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40](https://github.com/llvm/llvm-project/commit/1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40) does catch some brittle builds.

For example,

```auto
% 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](https://github.com/llvm/llvm-project/pull/70163) 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:

```auto
# 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

```

---

<div class="post-metadata">

**Author:** ![smithp35](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/smithp35/32/2545_2.png) [@smithp35](https://discourse.llvm.org/u/smithp35)\
**Post date:** [October 25, 2023, 2:06pm UTC](https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391/2 "2023-10-25T14:06:04Z")

</div>

Just to make sure I understand, I think there are two separate problems that may not cleanly overlap with the options we have now, or proposed. I’m starting with the assumption that a dynamic linker won’t see any hidden visibility definitions.

The first is the behavior of `--no-allow-shlib-undefined` in the presence of hidden visibility definitions. For this I think we want to give an error message when a dynamic linker would give an error message. I think this means that:

- If there is no default visibility symbol for a shared library reference from other shared-libraries or the executable then linker should give an error message.
- If there is a default visibility definition in another shared-library we shouldn’t give an error message for `--no-allow-shlib-undefined` if that symbol is preempted by a hidden visibility symbol (see 2nd problem).
- This should be independent of garbage collection.

The second problem is when a hidden visibility symbol in the executable preempts a shared definition. This will mean that the shared libraries and executable see different definitions which is an ODR violation that may be benign. In this case garbage collection (or at least the symbol liveness) does matter, because if the preempting definition is not used there is no ODR violation.

My intuition is that this should apply to non-default visibility symbols (protected, hidden and internal). I don’t think that fine grain control will be needed. Perhaps --no-allow-shlib-odr-violation is a better name?

I think starting with the check disabled is the easiest way to introduce this. A lot of projects need to link with GNU ld and LLD so unless GNU ld adopts a similarly named option then disabling it could be difficult for some projects.

Extending the check to default is potentially interesting, although if I’ve understood correctly, that will just say which symbols have been preempted. That is probably a separate “remark” level diagnostic.

---

<div class="post-metadata">

**Author:** ![jyknight](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/jyknight/32/1253_2.png) [@jyknight](https://discourse.llvm.org/u/jyknight)\
**Post date:** [October 25, 2023, 6:15pm UTC](https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391/3 "2023-10-25T18:15:32Z")

</div>

I think the fundamental issue here is that no-allow-shlib-undefined diagnostic (both before and after the recent change) is working on the basis of the _static_ symbol resolution of the current link. It’s understandable that it does so, since that’s what the linker is typically working on, but it’s not actually correct to use that for diagnosing symbol-lookup issues in shared libraries you’re linking against.

E.g. you get false-negatives if a shared-lib actually _does_ require a symbol defined by the binary (no other shared lib defines it), but the symbol in the binary was marked hidden.

---

<div class="post-metadata">

**Author:** ![MaskRay](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/maskray/32/81_2.png) [@MaskRay](https://discourse.llvm.org/u/MaskRay)\
**Post date:** [October 26, 2023, 8:19am UTC](https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391/4 "2023-10-26T08:19:04Z")

</div>

Thanks for the comments!

> [@smithp35](#):
>
> The first is the behavior of `--no-allow-shlib-undefined` in the presence of hidden visibility definitions. For this I think we want to give an error message when a dynamic linker would give an error message. I think this means that:
> 
> - If there is no default visibility symbol for a shared library reference from other shared-libraries or the executable then linker should give an error message.
> - If there is a default visibility definition in another shared-library we shouldn’t give an error message for `--no-allow-shlib-undefined` if that symbol is preempted by a hidden visibility symbol (see 2nd problem).
> - This should be independent of garbage collection.

Yes. This is the desired behavior of `--no-allow-shlib-undefined`.

> [@smithp35](#):
>
> The second problem is when a hidden visibility symbol in the executable preempts a shared definition. This will mean that the shared libraries and executable see different definitions which is an ODR violation that may be benign. In this case garbage collection (or at least the symbol liveness) does matter, because if the preempting definition is not used there is no ODR violation.
> 
> My intuition is that this should apply to non-default visibility symbols (protected, hidden and internal). I don’t think that fine grain control will be needed. Perhaps --no-allow-shlib-odr-violation is a better name?

The current [[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](https://github.com/llvm/llvm-project/pull/70130) checks the ODR violation before `if (!isLive) demoteDefined`.  
Therefore, the behavior is independent of garbage collection.

```auto
 static void demoteSymbolsAndComputeIsPreemptible() {
   llvm::TimeTraceScope timeScope("Demote symbols");
   DenseMap<InputFile *, DenseMap<SectionBase *, size_t>> sectionIndexMap;
+ bool allowHiddenSymbolsSharedWithDso =
+ config->allowHiddenSymbolsSharedWithDso;
   for (Symbol *sym : symtab.getSymbols()) {
     if (auto *d = dyn_cast<Defined>(sym)) {
+ if (!allowHiddenSymbolsSharedWithDso &&
+ d->visibility() == ELF::STV_HIDDEN) {
+ auto flags = d->flags.load(std::memory_order_relaxed);
+ if (flags & HAS_SHARED_DEF) {
+ errorOrWarn("hidden symbol also defined by DSO: " + toString(*sym) +
+ "\n>>> defined by " + toString(d->file));
+ } else if (flags & HAS_SHARED_REF) {
+ errorOrWarn("hidden symbol also referenced by DSO: " +
+ toString(*sym) + "\n>>> defined by " + toString(d->file));
+ }
+ }
+
       if (d->section && !d->section->isLive())
         demoteDefined(*d, sectionIndexMap[d->file]);

```

“garbage collection (or at least the symbol liveness) does matter” is an interesting point.  
With this interpretation, the diff needs to be adjusted to run `if (!isLive) demoteDefined` before the ODR violation check.

> [@smithp35](#):
>
> My intuition is that this should apply to non-default visibility symbols (protected, hidden and internal). I don’t think that fine grain control will be needed. Perhaps --no-allow-shlib-odr-violation is a better name?

If a hidden visibility `Defined` preempts a default visibility `SharedSymbol`, and the `Defined` survives under `--gc-sections`, since the `Defined` will not be exported, we are sure that there is an ODR violation.

If a protected visibility `Defined` preempts a default visibility `SharedSymbol`, we are uncertain whether the DSO symbol will be resolved to the executable at run-time.  
(Note: the `Defined` will satisfy `includeInDynsym()` as its `exportDynamic` is true)  
It seems that we cannot report an error if we want to be conservative.

If a `Defined` preempts a protected visibility `SharedSymbol`, since the `Defined` will not be exported, we are sure that there is an ODR violation.  
(A default visibility `SharedSymbol` linked with -Bsymbolic/–dynamic-list-family options are similar, but we cannot tell whether a DSO uses `--dynamic-list` and certain `-Bsymbolic` variants.)

Perhaps just checking hidden/internal visibility is good enough?

In practice, we can treat `STV_INTERNAL` the same as `STV_HIDDEN`.  
[SGI introduced `STV_INTERNAL`](https://groups.google.com/g/generic-abi/c/AahK3BVmYWQ/m/oylW2Jkvpy0J) for their link-time interprocedural optimization.  
They are more restricted than `STV_HIDDEN` in that they “are not accessed outside of the module” (I suspect that here “module” refers to “compile unit”).  
It is not used by GNU, HP, or Solaris.

> [@smithp35](#):
>
> I think starting with the check disabled is the easiest way to introduce this. A lot of projects need to link with GNU ld and LLD so unless GNU ld adopts a similarly named option then disabling it could be difficult for some projects.

Agree. I plan to raise a feature request when we have reached an initial consensus on the desired semantics, if we decide to introduce a new option.

> [@smithp35](#):
>
> Extending the check to default is potentially interesting, although if I’ve understood correctly, that will just say which symbols have been preempted. That is probably a separate “remark” level diagnostic.

Yes, extending the check to default visibility will catch link-time symbol interposition, which is (if users correctly respect ABI for link-time and run-time DSOs) a subset of run-time symbol interposition.  
There will probably be a lot of deploy problems due to vague linkage functions (especially if -fvisibility-inlines-hidden is not used) and certain runtime libraries.  
Checking the default visibility is probably a stretch goal that does not have a large return-on-investment.

---

<div class="post-metadata">

**Author:** ![smithp35](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/smithp35/32/2545_2.png) [@smithp35](https://discourse.llvm.org/u/smithp35)\
**Post date:** [October 26, 2023, 9:17am UTC](https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391/5 "2023-10-26T09:17:29Z")

</div>

> Perhaps just checking hidden/internal visibility is good enough?

Yes, agreed, good point about protected symbols being visible.

> “garbage collection (or at least the symbol liveness) does matter” is an interesting point.  
> With this interpretation, the diff needs to be adjusted to run `if (!isLive) demoteDefined` before the ODR violation check.

I think if we don’t consider liveness then we could get a false positive (warning or error). This might be rare enough that no-one is affected, or those that are can remove the dead code. Although there will always be those with binary libraries, or source code they can’t easily modify that won’t be able to do this. Not got a strong opinion here.

---

<div class="post-metadata">

**Author:** ![MaskRay](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/maskray/32/81_2.png) [@MaskRay](https://discourse.llvm.org/u/MaskRay)\
**Post date:** [October 31, 2023, 6:18am UTC](https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391/6 "2023-10-31T06:18:44Z")

</div>

> However, I am considering restoring the previous behavior (suppressing this error) in [#70130](https://github.com/llvm/llvm-project/pull/70130).

I have created a patch ([[ELF] Enhance --no-allow-shlib-undefined to report non-exported definition by MaskRay · Pull Request #70769 · llvm/llvm-project · GitHub](https://github.com/llvm/llvm-project/pull/70769)) on top of #70130 (loose the check) that improves the check to cover the cases when the relocatable object file definition is non-exported (hidden/internal visibility or localized by a version script).

GNU ld has detected these cases since [April 2003](https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=1b1fe8feb35ab988cdaf8481fc52ebdbf2db3d93) ([discussion](https://sourceware.org/pipermail/binutils/2003-April/026568.html))

```auto
# RUN: not ld.lld %t.o %t.so %tdef-hidden.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=HIDDEN

# HIDDEN: error: hidden symbol '_unresolved' is referenced by DSO '{{.*}}.tmp.so'

```

```auto
% ld.bfd a.o a.so adef-hidden.o --allow-shlib-undefined -y _unresolved # this error-checking is not governed by --allow-shlib-undefined in GNU ld
ld.bfd: a.so: reference to _unresolved
ld.bfd: adef-hidden.o: definition of _unresolved
ld.bfd: a.out: hidden symbol `_unresolved' in adef-hidden.o is referenced by DSO
ld.bfd: final link failed: bad value
% fld.lld a.o a.so adef-hidden.o
ld.lld: error: hidden symbol '_unresolved' is referenced by DSO 'a.so'

```

The new logic will be like

```auto
      for (SharedFile *file : ctx.sharedFiles) {
        bool allNeededIsKnown =
            llvm::all_of(file->dtNeeded, [&](StringRef needed) {
              return symtab.soNames.count(CachedHashStringRef(needed));
            });
        if (!allNeededIsKnown)
          continue;
        for (Symbol *sym : file->requiredSymbols) {
          if (sym->hasFlag(HAS_SHARED_DEF))
            continue;
          if (sym->isUndefined() && !sym->isWeak()) {
            diagnose(
                "undefined reference due to --no-allow-shlib-undefined: " +
                toString(*sym) + "\n>>> referenced by " + toString(file));
          } else if (sym->isDefined() && sym->computeBinding() == STB_LOCAL ) {
            diagnose("non-exported symbol '" + toString(*sym) +
                     "' is referenced by DSO '" + toString(file) + "'");
          }
        }
      }

```

Internally, I have identified 2 bugs (which would fail with LD\_BIND\_NOW=1) covered by commit 1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40 (for the discarded section cases).  
The above patch may identify more bugs (linker-reachable definitions that are not exercised at runtime; made benign by lazy binding).

* * *

I have made a summary: [DSO undef and non-exported definition | MaskRay](https://maskray.me/blog/2023-10-31-dso-undef-and-non-exported-definition)

---

<div class="post-metadata">

**Author:** ![smithp35](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/smithp35/32/2545_2.png) [@smithp35](https://discourse.llvm.org/u/smithp35)\
**Post date:** [October 31, 2023, 6:33pm UTC](https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391/7 "2023-10-31T18:33:48Z")

</div>

To check I understand as I got a bit confused with the three pull requests:

> <https://github.com/llvm/llvm-project/pull/70769>
>
> 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

> <https://github.com/llvm/llvm-project/pull/70130>
>
> 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\`.

> <https://github.com/llvm/llvm-project/pull/70163>
>
> 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](https://github.com/llvm/llvm-project/pull/70769) 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](https://github.com/llvm/llvm-project/pull/70163) 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](https://github.com/llvm/llvm-project/pull/70130) first? Then both [[ELF] Add --[no-]allow-non-exported-symbols-shared-with-dso by MaskRay · Pull Request #70163 · llvm/llvm-project · GitHub](https://github.com/llvm/llvm-project/pull/70163) and [[ELF] Enhance --no-allow-shlib-undefined to report non-exported definition by MaskRay · Pull Request #70769 · llvm/llvm-project · GitHub](https://github.com/llvm/llvm-project/pull/70769) 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.

---

<div class="post-metadata">

**Author:** ![MaskRay](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/maskray/32/81_2.png) [@MaskRay](https://discourse.llvm.org/u/MaskRay)\
**Post date:** [November 1, 2023, 5:00am UTC](https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391/8 "2023-11-01T05:00:15Z")

</div>

#70130 (`[ELF] Suppress --no-allow-shlib-undefined diagnostic ...`) is to remove an ODR violation check from `--no-allow-shlib-undefined`.

The lost check can be restored with #70163 (`[ELF] Add --[no-]allow-non-exported-symbols-shared-with-dso`), which also covers more case (even if the non-exported definitions survives GC). The issue is that if `--allow-...` is set to the default, the check will probably not be used. Therefore, we likely need to make `--no-allow-...` the default.

Personally, I wish that we wouldn’t add another option, but some people might argue that the ODR violation detected by the current `--no-allow-shlib-undefined` code is valid use case, and the check doesn’t fit into `--no-allow-shlib-undefined`, which is to simulate rtld behaviors.

For my users, I have fixed almost all issues and I can live without a turn-off option.

* * *

#70769 (`[ELF] Enhance --no-allow-shlib-undefined to report non-exported definition`) is an orthogonal patch to #70130 and #70163.

* * *

Based on the current information I have, **I don’t require any of the three patches for my users**. Please take your time, and don’t feel pressured. Thank you very much for reviewing the changes.

#70163, which adds a new option, is particularly in a no-hurry mode, as I should communicate the idea to the binutils community, but I want the idea to gain some initial consensus within the llvm-project.

@smeenai @igorkudrin @ruiu

---

<div class="post-metadata">

**Author:** ![smeenai](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/smeenai/32/1929_2.png) [@smeenai](https://discourse.llvm.org/u/smeenai)\
**Post date:** [November 1, 2023, 5:51pm UTC](https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391/9 "2023-11-01T17:51:21Z")

</div>

I don’t think any of my users will end up needing the options either, because most of what we build is shared libraries, so `--no-allow-shlib-undefined` doesn’t come into play at all. (This is for Android, so all your native code is built as shared library that’s loaded from Java or Kotlin, and we don’t have any standalone native executables to speak of.) That being said, I’d welcome anything that can help detect potential ODR issues, because I don’t think we have any tooling to detect cross-library ODR violations at all right now.

---

<div class="post-metadata">

**Author:** ![MaskRay](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/maskray/32/81_2.png) [@MaskRay](https://discourse.llvm.org/u/MaskRay)\
**Post date:** [January 22, 2024, 6:13pm UTC](https://discourse.llvm.org/t/relocatable-file-definitions-shared-with-dso/74391/10 "2024-01-22T18:13:24Z")

</div>

Landed [[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](https://github.com/llvm/llvm-project/pull/70130), which restores the previous (behavior before `release/18.x` is created), suppressing a diagnostic when:

- There is a DSO undef that has been satisfied by a definition from another DSO.
- The `SharedSymbol` is overridden by a non-exported (usually of hidden visibility) definition in a relocatable object file (`Defined`).
- The section containing the `Defined` is garbage-collected (it is not part of `.dynsym` and is not marked as live).

My latest thought is that we probably should not do #70163. This applies to too few scenarios. A generic ODR violation detector would be more useful.