[ELF] Add --warn-backrefs-exclude=<glob>
authorFangrui Song <maskray@google.com>
Sun, 5 Apr 2020 04:31:36 +0000 (21:31 -0700)
committerFangrui Song <maskray@google.com>
Mon, 20 Apr 2020 14:52:15 +0000 (07:52 -0700)
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

lld/ELF/Config.h
lld/ELF/Driver.cpp
lld/ELF/Options.td
lld/ELF/Symbols.cpp
lld/docs/ld.lld.1
lld/test/ELF/warn-backrefs.s

index 70ef862..e01a16b 100644 (file)
@@ -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>
 
@@ -201,6 +202,7 @@ struct Configuration {
   bool unique;
   bool useAndroidRelrTags = false;
   bool warnBackrefs;
+  std::vector<llvm::GlobPattern> warnBackrefsExclude;
   bool warnCommon;
   bool warnIfuncTextrel;
   bool warnMissingEntry;
index 35153a1..b074ae6 100644 (file)
@@ -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.
index 8dd5251..799707a 100644 (file)
@@ -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)">;
index fe1c2ca..235ebe4 100644 (file)
@@ -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>
 
@@ -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
index 7af9a16..1d55cf0 100644 (file)
@@ -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
index 5dec458..6308c57 100644 (file)
 # 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
 # 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
 
@@ -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