[PATCH] D46624: [ELF] --warn-backref: don't report backref to weak symbols.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added a reviewer: ruiu.
Herald added subscribers: llvm-commits, arichardson, emaste.
Herald added a reviewer: espindola.

Suppose we visit symbols in this order:

1. weak definition of foo in a lazy object
2. reference of foo

3 (optional). definition of foo

bfd/gold allows 123 but not 12.

Current --warn-backrefs implementation will report both cases as a backward reference. With this change, both 123 (intended) and 12 (unintended) are allowed. The usage of weak definitions usually imply there are also global definitions, so the trade-off is justified.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D46624

Files:
  ELF/SymbolTable.cpp
  test/ELF/warn-backrefs.s


Index: test/ELF/warn-backrefs.s
===================================================================
--- test/ELF/warn-backrefs.s
+++ test/ELF/warn-backrefs.s
@@ -39,6 +39,10 @@
 # RUN: echo ".globl foo; foo: call bar" | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %t4.o
 # RUN: ld.lld --fatal-warnings --warn-backrefs %t1.o --start-lib %t3.o %t4.o --end-lib -o %t.exe
 
+# We don't report backward references to weak symbols as they can be overriden later.
+# RUN: echo ".weak foo; foo:" | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %t5.o
+# RUN: ld.lld --fatal-warnings --warn-backrefs --start-lib %t5.o --end-lib %t1.o %t2.o
+
 .globl _start, foo
 _start:
   call foo
Index: ELF/SymbolTable.cpp
===================================================================
--- ELF/SymbolTable.cpp
+++ ELF/SymbolTable.cpp
@@ -339,10 +339,8 @@
 // A forms group 0. B form group 1. C and D (including their member object
 // files) form group 2. E forms group 3. I think that you can see how this
 // group assignment rule simulates the traditional linker's semantics.
-static void checkBackrefs(StringRef Name, InputFile *Old, InputFile *New) {
-  if (Config->WarnBackrefs && New && Old->GroupId < New->GroupId)
-    warn("backward reference detected: " + Name + " in " + toString(New) +
-         " refers to " + toString(Old));
+static bool checkBackrefs(StringRef Name, InputFile *Old, InputFile *New) {
+  return Config->WarnBackrefs && New && Old->GroupId < New->GroupId;
 }
 
 template <class ELFT>
@@ -377,8 +375,13 @@
       return S;
     }
 
-    checkBackrefs(Name, S->File, File);
+    bool Backref = checkBackrefs(Name, S->File, File);
     fetchLazy<ELFT>(S);
+    // We don't report backward references to weak symbols as they can be
+    // overriden later.
+    if (Backref && S->Binding != STB_WEAK)
+      warn("backward reference detected: " + Name + " in " + toString(File) +
+           " refers to " + toString(S->File));
   }
   return S;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46624.145853.patch
Type: text/x-patch
Size: 1981 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180509/d0588fa8/attachment.bin>