[ELF] --warn-backrefs: don't warn for linking sandwich problems

This is an alternative design to D77512.

D45195 added --warn-backrefs to detect

* A. certain input orders which GNU ld either errors ("undefined reference")
  or has different resolution semantics
* B. (byproduct) some latent multiple definition problems (-ldef1 -lref -ldef2) which I
  call "linking sandwich problems". def2 may or may not be the same as def1.

When an archive appears more than once (-ldef -lref -ldef), lld and GNU
ld may have the same resolution but --warn-backrefs may warn. This is
not uncommon. For example, currently lld itself has such a problem:

```
liblldCommon.a liblldCOFF.a ... liblldCommon.a
  _ZN3lld10DWARFCache13getDILineInfoEmm in liblldCOFF.a refers to liblldCommon.a(DWARF.cpp.o)
libLLVMSupport.a also appears twice and has a similar warning
```

glibc has such problems. It is somewhat destined because of its separate
libc/libpthread/... and arbitrary grouping. The situation is getting
improved over time but I have seen:
```
-lc __isnanl references -lm
-lc _IO_funlockfile references -lpthread
```

There are also various issues in interaction with other runtime
libraries such as libgcc_eh and libunwind:
```
-lc __gcc_personality_v0 references -lgcc_eh
-lpthread __gcc_personality_v0 references -lgcc_eh
-lpthread _Unwind_GetCFA references -lunwind
```

These problems are actually benign. We want --warn-backrefs to focus on
its main task A and defer task B (which is also useful) to a more
specific future feature (see gold --detect-odr-violations and
https://bugs.llvm.org/show_bug.cgi?id=43110).

Instead of warning immediately, we store the message and only report it
if no subsequent lazy definition exists.

The use of the static variable `backrefDiags` is similar to `undefs` in
Relocations.cpp

Reviewed By: grimar

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

lld/ELF/Driver.cpp

@@ -94,6 +94,7 @@ bool link(ArrayRef<const char *> args, bool canExitEarly, raw_ostream &stdoutOS,
   bitcodeFiles.clear();
   objectFiles.clear();
   sharedFiles.clear();
+  backwardReferences.clear();
 
   config = make<Configuration>();
   driver = make<LinkerDriver>();
@@ -1921,6 +1922,9 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
   // With this the symbol table should be complete. After this, no new names
   // except a few linker-synthesized ones will be added to the symbol table.
   compileBitcodeFiles<ELFT>();
+
+  // Symbol resolution finished. Report backward reference problems.
+  reportBackrefs();
   if (errorCount())
     return;
 

lld/ELF/Symbols.cpp

@@ -63,6 +63,7 @@ Defined *ElfSym::relaIpltStart;
 Defined *ElfSym::relaIpltEnd;
 Defined *ElfSym::riscvGlobalPointer;
 Defined *ElfSym::tlsModuleBase;
+DenseMap<const Symbol *, const InputFile *> backwardReferences;
 
 static uint64_t getSymVA(const Symbol &sym, int64_t &addend) {
   switch (sym.kind()) {
@@ -373,6 +374,14 @@ bool computeIsPreemptible(const Symbol &sym) {
   return true;
 }
 
+void reportBackrefs() {
+  for (auto &it : backwardReferences) {
+    const Symbol &sym = *it.first;
+    warn("backward reference detected: " + sym.getName() + " in " +
+         toString(it.second) + " refers to " + toString(sym.file));
+  }
+}
+
 static uint8_t getMinVisibility(uint8_t va, uint8_t vb) {
   if (va == STV_DEFAULT)
     return vb;
@@ -509,9 +518,13 @@ void Symbol::resolveUndefined(const Undefined &other) {
 
     // We don't report backward references to weak symbols as they can be
     // overridden later.
+    //
+    // A traditional linker does not error for -ldef1 -lref -ldef2 (linking
+    // sandwich), where def2 may or may not be the same as def1. We don't want
+    // to warn for this case, so dismiss the warning if we see a subsequent lazy
+    // definition.
     if (backref && !isWeak())
-      warn("backward reference detected: " + other.getName() + " in " +
-           toString(other.file) + " refers to " + toString(file));
+      backwardReferences.try_emplace(this, other.file);
     return;
   }
 
@@ -668,8 +681,12 @@ void Symbol::resolveDefined(const Defined &other) {
 }
 
 template <class LazyT> void Symbol::resolveLazy(const LazyT &other) {
-  if (!isUndefined())
+  if (!isUndefined()) {
+    // See the comment in resolveUndefined().
+    if (isDefined())
+      backwardReferences.erase(this);
     return;
+  }
 
   // An undefined weak will not fetch archive members. See comment on Lazy in
   // Symbols.h for the details.

lld/ELF/Symbols.h

@@ -17,6 +17,7 @@
 #include "InputSection.h"
 #include "lld/Common/LLVM.h"
 #include "lld/Common/Strings.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ELF.h"
 
@@ -556,6 +557,11 @@ void Symbol::replace(const Symbol &newSym) {
 
 void maybeWarnUnorderableSymbol(const Symbol *sym);
 bool computeIsPreemptible(const Symbol &sym);
+void reportBackrefs();
+
+// A mapping from a symbol to an InputFile referencing it backward. Used by
+// --warn-backrefs.
+extern llvm::DenseMap<const Symbol *, const InputFile *> backwardReferences;
 
 } // namespace elf
 } // namespace lld

lld/test/ELF/warn-backrefs.s

@@ -4,6 +4,7 @@
 # RUN: echo '.globl foo; foo:' | llvm-mc -filetype=obj -triple=x86_64 - -o %t2.o
 # RUN: rm -f %t2.a
 # RUN: llvm-ar rcs %t2.a %t2.o
+# RUN: ld.lld -shared %t2.o -o %t2.so
 
 ## A forward reference is accepted by a traditional Unix linker.
 # RUN: ld.lld --fatal-warnings %t1.o %t2.a -o /dev/null
@@ -27,12 +28,16 @@
 # RUN: echo 'GROUP("%t2.a")' > %t3.lds
 # RUN: ld.lld --warn-backrefs %t3.lds %t1.o -o /dev/null 2>&1 | FileCheck %s
 # RUN: ld.lld --fatal-warnings --warn-backrefs '-(' %t3.lds %t1.o '-)' -o /dev/null
+## If a lazy definition appears after the backward reference, don't warn.
+# RUN: ld.lld --fatal-warnings --warn-backrefs %t3.lds %t1.o %t3.lds -o /dev/null
 
 # CHECK: warning: backward reference detected: foo in {{.*}}1.o refers to {{.*}}2.a
 
 ## A backward reference from %t1.o to %t2.o
 # RUN: ld.lld --warn-backrefs --start-lib %t2.o --end-lib %t1.o -o /dev/null 2>&1 | \
 # RUN:   FileCheck --check-prefix=OBJECT %s
+## If a lazy definition appears after the backward reference, don't warn.
+# RUN: ld.lld --fatal-warnings --warn-backrefs --start-lib %t2.o --end-lib %t1.o --start-lib %t2.o --end-lib -o /dev/null
 
 # OBJECT: warning: backward reference detected: foo in {{.*}}1.o refers to {{.*}}2.o
 
@@ -45,6 +50,18 @@
 # RUN: echo '.weak foo; foo:' | llvm-mc -filetype=obj -triple=x86_64 - -o %tweak.o
 # RUN: ld.lld --fatal-warnings --warn-backrefs --start-lib %tweak.o --end-lib %t1.o %t2.o -o /dev/null
 
+## If a lazy definition appears after the backward reference, don't warn.
+## A traditional Unix linker will resolve the reference to the later definition.
+# RUN: ld.lld --fatal-warnings --warn-backrefs %t2.a %t1.o %t2.a -o /dev/null
+
+## lld fetches the archive while GNU ld resolves the reference to the shared definition.
+## Warn because the resolution rules are different.
+# RUN: ld.lld --warn-backrefs %t2.a %t1.o %t2.so -o /dev/null 2>&1 | FileCheck %s
+
+## This is a limitation. The resolution rules are different but
+## --warn-backrefs does not warn.
+# RUN: ld.lld --fatal-warnings --warn-backrefs %t2.a %t1.o %t2.so %t2.a -o /dev/null
+
 .globl _start, foo
 _start:
   call foo