[lld] 03c825c - [ELF] --warn-backrefs: don't warn for linking sandwich problems
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 7 10:25:35 PDT 2020
Author: Fangrui Song
Date: 2020-04-07T10:25:23-07:00
New Revision: 03c825c224420c498f3f7aef8ad4fb005d62b8ec
URL: https://github.com/llvm/llvm-project/commit/03c825c224420c498f3f7aef8ad4fb005d62b8ec
DIFF: https://github.com/llvm/llvm-project/commit/03c825c224420c498f3f7aef8ad4fb005d62b8ec.diff
LOG: [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
Added:
Modified:
lld/ELF/Driver.cpp
lld/ELF/Symbols.cpp
lld/ELF/Symbols.h
lld/test/ELF/warn-backrefs.s
Removed:
################################################################################
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 914cfefbd329..c90ac589d2d1 100644
--- a/lld/ELF/Driver.cpp
+++ b/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;
diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index 50c153d63ab7..fe1c2cacd310 100644
--- a/lld/ELF/Symbols.cpp
+++ b/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.
diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index ebee4af1fdad..3459c264e1fa 100644
--- a/lld/ELF/Symbols.h
+++ b/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
diff --git a/lld/test/ELF/warn-backrefs.s b/lld/test/ELF/warn-backrefs.s
index 9538cb086613..3d6bcb933100 100644
--- a/lld/test/ELF/warn-backrefs.s
+++ b/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
diff erent.
+# 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
diff erent 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
More information about the llvm-commits
mailing list