[llvm-dev] LLD: Can we make --warn-backrefs the default?

Thu Sep 3 12:45:40 PDT 2020

On Tue, Sep 1, 2020 at 5:35 PM Fāng-ruì Sòng via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> On 2020-09-01, Petr Hosek wrote:
> >I see the GNU ld behavior as a limitation, not as a feature, as Peter
> Smith
> >also pointed out in https://reviews.llvm.org/D86762. While it can be
> argued
> >that there are certain cases where it can help detect layering
> >violations as you mentioned in your change, I'm not sure how valuable that
> >is in practice. Every case I've encountered so far either in Chrome or in
> >Fuchsia was a valid use case, most commonly interceptors. The solution
> >has always been the same, wrap all libraries in --start-group/--stop-group
> >and it's what most projects do by default to avoid dealing with these
> >issues, see for example [Chromium](
> >
> https://source.chromium.org/chromium/chromium/src/+/master:build/toolchain/gcc_toolchain.gni;l=409
> ).
> >In our case, compatibility with linkers on other platforms is more
> >important than compatibility with GNU ld, so I'd prefer to keep the
> current
> >behavior. Projects that care about compatibility with GNU ld can use
> >--warn-backrefs.
>
> I totally understand that some users may not want to deal with GNU ld
> compatibility:) I'll then question about Chromium's addition of -z defs:
> https://crrev.com/843583006 :)
>
> -z defs is like a layering checking tool for shared objects while
> --warn-backrefs is for archives. For performance, ABI concerns and ease
> of deployment, many projects tend to build their own components as
> archives instead of shared objects. In this sense --warn-backrefs will
> probably be more useful than -z defs.
>
> (
> TIL lorder and tsort were created to define an order of archives in
> early versions of Unix.
> https://www.gnu.org/software/coreutils/manual/html_node/tsort-background.html
> It seems that the article missed the point that proper library layering is
> still useful
> )
>

I'm not a fan of this idea of reframing GNU ld behavior as a "layering
checking tool". It is an incomplete layering checking tool because it does
not detect the scenario where, for example, you have the intended
dependency graph:

A -> B
A -> C
B -> D
C -> D

(resulting in -la -lb -lc -ld) and you have an unexpected dependency B ->
C. There is already a way to detect layering problems, that detects
practical layering problems and not just theoretical ones, which is to link
programs that use subsets of the libraries. For example, linking a program
that depends only on B would result in detecting the invalid B -> C
dependency. It's also worth noting that even that would only detect the
layering problem if the program depends on the part of B that depends on C.

A better way to go about achieving layering checking would IMHO be to
implement a separate tool (not part of the linker) that is capable of
a complete layering check. Such a tool would only depend on symbol table
features common to all object formats, so it could probably be implemented
generically.

Peter

> >On Mon, Aug 31, 2020 at 1:44 PM Fāng-ruì Sòng <maskray at google.com> wrote:
> >
> >> On Mon, Aug 31, 2020 at 1:29 PM David Blaikie <dblaikie at gmail.com>
> wrote:
> >> >
> >> >
> >> >
> >> > On Mon, Aug 31, 2020 at 1:24 PM Fāng-ruì Sòng <maskray at google.com>
> >> wrote:
> >> >>
> >> >> On Fri, Aug 28, 2020 at 11:16 AM David Blaikie <dblaikie at gmail.com>
> >> wrote:
> >> >> >
> >> >> > Would you like to conduct the conversation here, or on the review
> >> thread? (I lean towards having them here, but don't mind if folks feel
> like
> >> it keeps the noise down & want to more post a notice saying "hey, here's
> >> this thing, if you're interested, go discuss it over there" - more an
> >> optional opt-in rather than requiring people to opt-out via muting the
> >> thread, etc)
> >> >>
> >> >> Yes, we can conduct the "should we enable --warn-backrefs by default"
> >> >> conversation here. Since the semantics --warn-backrefs of are a bit
> >> >> complex, we need a documentation. https://reviews.llvm.org/D86762 is
> >> >> put up to get wording suggestions. Explicitly adding the people to
> the
> >> >> CC list...
> >> >>
> >> >> FWIW for many code bases, --warn-backrefs should produce no warnings
> >> >> (error if --fatal-warnings). For some code bases, GNU ld may error
> >> >> "undefined reference".  --warn-backrefs can catch such problems.
> >> >
> >> >
> >> > One of the questions raised on the thread there was about different
> >> linker semantics. I assume the "--warn-backrefs by default" we're
> >> discussing is only related to the ld.lld frontend? Not the Windows
> linker
> >> lld behavior (or ld64 (old or new) lld behavior)?
> >>
> >> Yes, the ELF port (lld/ELF). No other port has implemented
> >> --warn-backrefs. The intion of --warn-backrefs is to capture behavior
> >> differences with GNU ld's ELF ports. If traditional ELF linkers don't
> >> behave like that, I can replace "traditional ELF linkers" in
> >> https://reviews.llvm.org/D86762 with "GNU linkers".
> >>
> >> >>
> >> >>
> >> >>
> >> >> > On Thu, Aug 27, 2020 at 10:15 PM Fangrui Song via llvm-dev <
> >> llvm-dev at lists.llvm.org> wrote:
> >> >> >>
> >> >> >> Hi all, LLD's --warn-backrefs is a tool to identify potential
> >> >> >> incompatible archive selection semantics with traditional Unix
> >> linkers.
> >> >> >> I have improved it (via D77522,D77630 and D77512) to a state
> where a
> >> >> >> --warn-backrefs diagnostic almost assuredly means that the link
> will
> >> >> >> fail with GNU ld, or the symbol will get different resolution in
> GNU
> >> ld and LLD.
> >> >> >>
> >> >> >> My conclusion is that --warn-backrefs is a very useful layering
> >> check tool.
> >> >> >> I just wrote a documentation about the advantage (of GNU ld's
> archive
> >> >> >> selection semantics..... But we can do better with
> --warn-backrefs!
> >> >> >> GNU ld just reports "undefined reference" with no actionable
> feedback
> >> >> >> about the offending archive)
> >> >> >>
> >> >> >> https://reviews.llvm.org/D86762
> >> >> >>
> >> >> >> I am wondering whether in the next release we can make
> >> --warn-backrefs
> >> >> >> the default.  I have added many known users to the review.
> >> >> >> (There is no need for --no-warn-backrefs because
> >> --warn-backrefs-exclude='*' does the same job)
> >> >> >> _______________________________________________
> >> >> >> LLVM Developers mailing list
> >> >> >> llvm-dev at lists.llvm.org
> >> >> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200903/d3c1c657/attachment-0001.html>