[LLD] Improve reporting unresolved symbols in shared libraries

Currently, when reporting unresolved symbols in shared libraries, if an
undefined symbol is firstly seen in a regular object file that shadows
the reference for the same symbol in a shared object. As a result, the
error for the unresolved symbol in the shared library is not reported.
If referencing sections in regular object files are discarded because of
'--gc-sections', no reports about such symbols are generated, and the
linker finishes successfully, generating an output image that fails on
the run.

The patch fixes the issue by keeping symbols, which should be checked,
for each shared library separately.

Differential Revision: https://reviews.llvm.org/D101996

Author: igorkudrin

lld/ELF/InputFiles.cpp

@@ -1567,6 +1567,9 @@ template <class ELFT> void SharedFile::parse() {
       Symbol *s = symtab->addSymbol(
           Undefined{this, name, sym.getBinding(), sym.st_other, sym.getType()});
       s->exportDynamic = true;
+      if (s->isUndefined() && !s->isWeak() &&
+          config->unresolvedSymbolsInShlib != UnresolvedPolicy::Ignore)
+        requiredSymbols.push_back(s);
       continue;
     }
 

lld/ELF/InputFiles.h

@@ -381,12 +381,13 @@ class SharedFile : public ELFFileBase {
 
   template <typename ELFT> void parse();
 
-  // Used for --no-allow-shlib-undefined.
-  bool allNeededIsKnown;
-
   // Used for --as-needed
   bool isNeeded;
 
+  // Non-weak undefined symbols which are not yet resolved when the SO is
+  // parsed. Only filled for `--no-allow-shlib-undefined`.
+  std::vector<Symbol *> requiredSymbols;
+
 private:
   template <typename ELFT>
   std::vector<uint32_t> parseVerneed(const llvm::object::ELFFile<ELFT> &obj,

lld/ELF/Writer.cpp

@@ -2006,30 +2006,29 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
     in.iplt->addSymbols();
 
   if (config->unresolvedSymbolsInShlib != UnresolvedPolicy::Ignore) {
+    auto diagnose =
+        config->unresolvedSymbolsInShlib == UnresolvedPolicy::ReportError
+            ? errorOrWarn
+            : warn;
     // Error on undefined symbols in a shared object, if all of its DT_NEEDED
     // entries are seen. These cases would otherwise lead to runtime errors
     // reported by the dynamic linker.
     //
     // ld.bfd traces all DT_NEEDED to emulate the logic of the dynamic linker 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.
-    for (SharedFile *file : sharedFiles)
-      file->allNeededIsKnown =
+    for (SharedFile *file : sharedFiles) {
+      bool allNeededIsKnown =
           llvm::all_of(file->dtNeeded, [&](StringRef needed) {
             return symtab->soNames.count(needed);
           });
-
-    for (Symbol *sym : symtab->symbols())
-      if (sym->isUndefined() && !sym->isWeak())
-        if (auto *f = dyn_cast_or_null<SharedFile>(sym->file))
-          if (f->allNeededIsKnown) {
-            auto diagnose = config->unresolvedSymbolsInShlib ==
-                                    UnresolvedPolicy::ReportError
-                                ? errorOrWarn
-                                : warn;
-            diagnose(toString(f) + ": undefined reference to " +
-                     toString(*sym) + " [--no-allow-shlib-undefined]");
-          }
+      if (!allNeededIsKnown)
+        continue;
+      for (Symbol *sym : file->requiredSymbols)
+        if (sym->isUndefined() && !sym->isWeak())
+          diagnose(toString(file) + ": undefined reference to " +
+                   toString(*sym) + " [--no-allow-shlib-undefined]");
+    }
   }
 
   {

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

@@ -25,9 +25,22 @@
 # RUN: ld.lld -shared --allow-shlib-undefined %t1.o -o /dev/null
 # RUN: ld.lld -shared --no-allow-shlib-undefined %t1.o -o /dev/null
 
+## Check that the error is reported if an unresolved symbol is first seen in a
+## regular object file.
+# RUN: echo 'callq _unresolved@PLT' | \
+# RUN:   llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %tref.o
+# RUN: not ld.lld --gc-sections %t.o %tref.o %t.so -o /dev/null 2>&1 | FileCheck %s
+
+## Check that the error is reported for each shared library where the symbol
+## is referenced.
+# RUN: cp %t.so %t2.so
+# RUN: not ld.lld %t.o %t.so %t2.so -o /dev/null 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=CHECK,CHECK2
+
 .globl _start
 _start:
   callq _shared@PLT
 
 # CHECK: error: {{.*}}.so: undefined reference to _unresolved [--no-allow-shlib-undefined]
+# CHECK2: error: {{.*}}2.so: undefined reference to _unresolved [--no-allow-shlib-undefined]
 # WARN: warning: {{.*}}.so: undefined reference to _unresolved [--no-allow-shlib-undefined]