[PATCH] D77522: [ELF] --warn-backrefs: don't warn for linking sandwich problems

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: grimar, psmith, ruiu.
Herald added subscribers: llvm-commits, arichardson, aprantl, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.
MaskRay added a comment.

I will try this in our internal large code base and ask Android/ChromeOS folks for help as well. If this goes well (little or no false positive), I hope we can eventually make --warn-backrefs the default.
Here for "false positive" I mean --warn-backrefs warns but our symbol resolution is actually the same as GNU ld's.


This is an alternative design to D77512 <https://reviews.llvm.org/D77512>.

D45195 <https://reviews.llvm.org/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).

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77522

Files:
  lld/ELF/Driver.cpp
  lld/ELF/Symbols.cpp
  lld/ELF/Symbols.h
  lld/test/ELF/warn-backrefs.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77522.255225.patch
Type: text/x-patch
Size: 4822 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200406/56a3ecd9/attachment.bin>