[ELF] Suppress --no-allow-shlib-undefined diagnostic when a SharedSymbol is overridden by a hidden visibility Defined which is later discarded

Commit 1981b1b unexpectedly made --no-allow-shlib-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 1981b1b.

---

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`.

Author: MaskRay

lld/ELF/InputFiles.cpp

@@ -1545,6 +1545,7 @@ template <class ELFT> void SharedFile::parse() {
       auto *s = symtab.addSymbol(
           SharedSymbol{*this, name, sym.getBinding(), sym.st_other,
                        sym.getType(), sym.st_value, sym.st_size, alignment});
+      s->setFlags(HAS_SHARED_DEF);
       if (s->file == this)
         s->verdefIndex = ver;
     }
@@ -1562,6 +1563,7 @@ template <class ELFT> void SharedFile::parse() {
     auto *s = symtab.addSymbol(
         SharedSymbol{*this, saver().save(name), sym.getBinding(), sym.st_other,
                      sym.getType(), sym.st_value, sym.st_size, alignment});
+    s->setFlags(HAS_SHARED_DEF);
     if (s->file == this)
       s->verdefIndex = idx;
   }

lld/ELF/Symbols.h

@@ -54,6 +54,7 @@ enum {
   NEEDS_TLSGD_TO_IE = 1 << 6,
   NEEDS_GOT_DTPREL = 1 << 7,
   NEEDS_TLSIE = 1 << 8,
+  HAS_SHARED_DEF = 1 << 9,
 };
 
 // Some index properties of a symbol are stored separately in this auxiliary

lld/ELF/Writer.cpp

@@ -2016,6 +2016,11 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
       // to catch more cases. That is too much for us. Our approach resembles
       // the one used in ld.gold, achieves a good balance to be useful but not
       // too smart.
+      //
+      // If a DSO reference is resolved by a SharedSymbol, but the SharedSymbol
+      // is overridden by a hidden visibility Defined (which is later discarded
+      // due to GC), don't report the diagnostic. However, this may indicate an
+      // unintended SharedSymbol.
       for (SharedFile *file : ctx.sharedFiles) {
         bool allNeededIsKnown =
             llvm::all_of(file->dtNeeded, [&](StringRef needed) {
@@ -2024,6 +2029,8 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
         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));

lld/test/ELF/allow-shlib-undefined.s

@@ -40,7 +40,7 @@
 # RUN: not ld.lld --gc-sections main.o a.so def-hidden.o -o /dev/null 2>&1 | FileCheck %s
 ## The definition def.so is ignored.
 # RUN: ld.lld -shared def.o -o def.so
-# RUN: not ld.lld --gc-sections main.o a.so def.so def-hidden.o -o /dev/null 2>&1 | FileCheck %s
+# RUN: ld.lld --gc-sections main.o a.so def.so def-hidden.o --fatal-warnings -o /dev/null
 
 # CHECK-NOT:   error:
 # CHECK:       error: undefined reference due to --no-allow-shlib-undefined: x1{{$}}