Skip to content

Commit

Permalink
[ELF] Add --warn-backrefs-exclude=<glob>
Browse files Browse the repository at this point in the history
D77522 changed --warn-backrefs to not warn for linking sandwich
problems (-ldef1 -lref -ldef2). This removed lots of false positives.

However, glibc still has some problems. libc.a defines some symbols
which are normally in libm.a and libpthread.a, e.g. __isnanl/raise.

For a linking order `-lm -lpthread -lc`, I have seen:

```
// different resolutions: GNU ld/gold select libc.a(s_isnan.o) as the definition
backward reference detected: __isnanl in libc.a(printf_fp.o) refers to libm.a(m_isnanl.o)

// different resolutions: GNU ld/gold select libc.a(raise.o) as the definition
backward reference detected: raise in libc.a(abort.o) refers to libpthread.a(pt-raise.o)
```

To facilitate deployment of --warn-backrefs, add --warn-backrefs-exclude= so that
certain known issues (which may be impractical to fix) can be whitelisted.

Deliberate choices:

* Not a comma-separated list (`--warn-backrefs-exclude=liba.a,libb.a`).
  -Wl, splits the argument at commas, so we cannot use commas.
  --export-dynamic-symbol is similar.
* Not in the style of `--warn-backrefs='*' --warn-backrefs=-liba.a`.
  We just need exclusion, not inclusion. For easier build system
  integration, we should avoid order dependency. With the current
  scheme, we enable --warn-backrefs, and indivial libraries can add
  --warn-backrefs-exclude=<glob> to their LDFLAGS.

Reviewed By: psmith

Differential Revision: https://reviews.llvm.org/D77512
  • Loading branch information
MaskRay committed Apr 20, 2020
1 parent 9cf9710 commit 2325788
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 0 deletions.
2 changes: 2 additions & 0 deletions lld/ELF/Config.h
Expand Up @@ -17,6 +17,7 @@
#include "llvm/Support/CachePruning.h"
#include "llvm/Support/CodeGen.h"
#include "llvm/Support/Endian.h"
#include "llvm/Support/GlobPattern.h"
#include <atomic>
#include <vector>

Expand Down Expand Up @@ -201,6 +202,7 @@ struct Configuration {
bool unique;
bool useAndroidRelrTags = false;
bool warnBackrefs;
std::vector<llvm::GlobPattern> warnBackrefsExclude;
bool warnCommon;
bool warnIfuncTextrel;
bool warnMissingEntry;
Expand Down
8 changes: 8 additions & 0 deletions lld/ELF/Driver.cpp
Expand Up @@ -1148,6 +1148,14 @@ static void readConfigs(opt::InputArgList &args) {
{s, /*isExternCpp=*/false, /*hasWildcard=*/false});
}

for (opt::Arg *arg : args.filtered(OPT_warn_backrefs_exclude)) {
StringRef pattern(arg->getValue());
if (Expected<GlobPattern> pat = GlobPattern::create(pattern))
config->warnBackrefsExclude.push_back(std::move(*pat));
else
error(arg->getSpelling() + ": " + toString(pat.takeError()));
}

// Parses -dynamic-list and -export-dynamic-symbol. They make some
// symbols private. Note that -export-dynamic takes precedence over them
// as it says all symbols should be exported.
Expand Down
6 changes: 6 additions & 0 deletions lld/ELF/Options.td
Expand Up @@ -403,6 +403,12 @@ defm warn_backrefs: B<"warn-backrefs",
"Warn about backward symbol references to fetch archive members",
"Do not warn about backward symbol references to fetch archive members (default)">;

defm warn_backrefs_exclude
: Eq<"warn-backrefs-exclude",
"Glob describing an archive (or an object file within --start-lib) "
"which should be ignored for --warn-backrefs.">,
MetaVarName<"<glob>">;

defm warn_common: B<"warn-common",
"Warn about duplicate common symbols",
"Do not warn about duplicate common symbols (default)">;
Expand Down
12 changes: 12 additions & 0 deletions lld/ELF/Symbols.cpp
Expand Up @@ -16,6 +16,7 @@
#include "lld/Common/ErrorHandler.h"
#include "lld/Common/Strings.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
#include <cstring>

Expand Down Expand Up @@ -514,6 +515,17 @@ void Symbol::resolveUndefined(const Undefined &other) {
// group assignment rule simulates the traditional linker's semantics.
bool backref = config->warnBackrefs && other.file &&
file->groupId < other.file->groupId;
if (backref) {
// Some libraries have known problems and can cause noise. Filter them out
// with --warn-backrefs-exclude=.
StringRef name =
!file->archiveName.empty() ? file->archiveName : file->getName();
for (const llvm::GlobPattern &pat : config->warnBackrefsExclude)
if (pat.match(name)) {
backref = false;
break;
}
}
fetch();

// We don't report backward references to weak symbols as they can be
Expand Down
4 changes: 4 additions & 0 deletions lld/docs/ld.lld.1
Expand Up @@ -574,6 +574,10 @@ Read version script from
Warn about reverse or cyclic dependencies to or between static archives.
This can be used to ensure linker invocation remains compatible with
traditional Unix-like linkers.
.It Fl -warn-backrefs-exclude Ns = Ns Ar glob
Glob describing an archive (or an object file within --start-lib)
which should be ignored for
.Fl -warn-backrefs
.It Fl -warn-common
Warn about duplicate common symbols.
.It Fl -warn-ifunc-textrel
Expand Down
13 changes: 13 additions & 0 deletions lld/test/ELF/warn-backrefs.s
Expand Up @@ -15,9 +15,16 @@
# RUN: ld.lld --fatal-warnings --warn-backrefs %t1.lds -o /dev/null

## A backward reference from %t1.o to %t2.a
## Warn unless the archive is excluded by --warn-backrefs-exclude
# RUN: ld.lld --fatal-warnings %t2.a %t1.o -o /dev/null
# RUN: ld.lld --warn-backrefs %t2.a %t1.o -o /dev/null 2>&1 | FileCheck %s
# RUN: ld.lld --warn-backrefs %t2.a '-(' %t1.o '-)' -o /dev/null 2>&1 | FileCheck %s
# RUN: ld.lld --warn-backrefs --warn-backrefs-exclude='*3.a' %t2.a %t1.o -o /dev/null 2>&1 | FileCheck %s
# RUN: ld.lld --fatal-warnings --warn-backrefs --warn-backrefs-exclude='*2.a' %t2.a %t1.o -o /dev/null
# RUN: ld.lld --fatal-warnings --warn-backrefs --warn-backrefs-exclude '*2.a' \
# RUN: --warn-backrefs-exclude not_exist %t2.a %t1.o -o /dev/null
## Without --warn-backrefs, --warn-backrefs-exclude is ignored.
# RUN: ld.lld --fatal-warnings --warn-backrefs-exclude=not_exist %t2.a %t1.o -o /dev/null

## Placing the definition and the backward reference in a group can suppress the warning.
# RUN: echo 'GROUP("%t2.a" "%t1.o")' > %t2.lds
Expand All @@ -28,14 +35,17 @@
# 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
# RUN: ld.lld --fatal-warnings --warn-backrefs --warn-backrefs-exclude='*2.a' -o /dev/null %t3.lds %t1.o
## 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
## --warn-backrefs-exclude= applies to --start-lib covered object files.
# RUN: ld.lld --warn-backrefs --start-lib %t2.o --end-lib %t1.o -o /dev/null 2>&1 | \
# RUN: FileCheck --check-prefix=OBJECT %s
# RUN: ld.lld --fatal-warnings --warn-backrefs --warn-backrefs-exclude=%/t2.o --start-lib %/t2.o --end-lib %t1.o -o /dev/null
## 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

Expand Down Expand Up @@ -68,6 +78,9 @@
## In GNU gold, --export-dynamic-symbol does not make a backward reference.
# RUN: ld.lld --fatal-warnings --warn-backrefs --export-dynamic-symbol foo %t2.a %t1.o -o /dev/null

# RUN: not ld.lld --warn-backrefs-exclude='[' 2>&1 | FileCheck --check-prefix=INVALID %s
# INVALID: error: --warn-backrefs-exclude: invalid glob pattern: [

.globl _start, foo
_start:
call foo

0 comments on commit 2325788

Please sign in to comment.