From: Fangrui Song Date: Mon, 6 Apr 2020 05:27:46 +0000 (-0700) Subject: [ELF] --warn-backrefs: don't warn for linking sandwich problems X-Git-Tag: llvmorg-12-init~9789 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=03c825c224420c498f3f7aef8ad4fb005d62b8ec;p=platform%2Fupstream%2Fllvm.git [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 --- diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index 914cfef..c90ac58 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -94,6 +94,7 @@ bool link(ArrayRef args, bool canExitEarly, raw_ostream &stdoutOS, bitcodeFiles.clear(); objectFiles.clear(); sharedFiles.clear(); + backwardReferences.clear(); config = make(); driver = make(); @@ -1921,6 +1922,9 @@ template 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(); + + // Symbol resolution finished. Report backward reference problems. + reportBackrefs(); if (errorCount()) return; diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp index 50c153d..fe1c2ca 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 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 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 ebee4af..3459c26 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 backwardReferences; } // namespace elf } // namespace lld diff --git a/lld/test/ELF/warn-backrefs.s b/lld/test/ELF/warn-backrefs.s index 9538cb0..3d6bcb9 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 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