[lld] 70c23e2 - [LLD] Improve reporting unresolved symbols in shared libraries

[Igor Kudrin via llvm-commits]

Author: Igor Kudrin
Date: 2021-05-11T12:48:29+07:00
New Revision: 70c23e232e50b190c9e0d8e4b5b6a8ddfc19b80c

URL: https://github.com/llvm/llvm-project/commit/70c23e232e50b190c9e0d8e4b5b6a8ddfc19b80c
DIFF: https://github.com/llvm/llvm-project/commit/70c23e232e50b190c9e0d8e4b5b6a8ddfc19b80c.diff

LOG: [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

Added: 
    

Modified: 
    lld/ELF/InputFiles.cpp
    lld/ELF/InputFiles.h
    lld/ELF/Writer.cpp
    lld/test/ELF/allow-shlib-undefined.s

Removed: 
    


################################################################################
diff  --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 268afae117cde..3e0c1d77823fd 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/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;
     }
 

diff  --git a/lld/ELF/InputFiles.h b/lld/ELF/InputFiles.h
index cdd6b5c2ce99e..f66d7a25d9549 100644
--- a/lld/ELF/InputFiles.h
+++ b/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,

diff  --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 2301a064e3285..75ddd2dce1ea7 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2006,6 +2006,10 @@ 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.
@@ -2013,23 +2017,18 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
     // 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]");
+    }
   }
 
   {

diff  --git a/lld/test/ELF/allow-shlib-undefined.s b/lld/test/ELF/allow-shlib-undefined.s
index 4994466ec5dc7..4c0e06e5d0092 100644
--- a/lld/test/ELF/allow-shlib-undefined.s
+++ b/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 at 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 at 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]