<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Sep 3, 2020 at 5:15 PM Nico Weber <<a href="mailto:thakis@chromium.org">thakis@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">I wanted to chime in and say that I think we should keep the current default too, for three reasons:<div><br></div><div>1. The current default is more user friendly. Users shouldn't have to worry about if they pass -lpthread before or after their .o files (...or other libraries. I know I know for pthread it's `-pthread` and the driver will figure it out). <a href="https://stackoverflow.com/questions/45135/why-does-the-order-in-which-libraries-are-linked-sometimes-cause-errors-in-gcc" target="_blank">https://stackoverflow.com/questions/45135/why-does-the-order-in-which-libraries-are-linked-sometimes-cause-errors-in-gcc</a> is a huge FAQ and a cause of confusion for beginners. LLD's current defaults are much friendlier.</div><div><br></div><div>2. The current default is more self-consistent: It's consistent with ld64, link.exe, and hence lld-link and (new) lld.ld64.</div></div></blockquote><div><br>I think this misses the aspect of each of those linkers being consistent with the counterparts they're intended to emulate, right?<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>3. As Peter says correctly, using this for layering isn't a great fit since it only checks a linearized topological sort of the build graph nodes. A good check needs to work on a higher abstraction level.</div><div><br></div><div>You seem to focus on the use case where compiler and linker commands are generated by some meta build system. I think humans and bash scripts also do a lot of building, and for that case ease-of-use matters much more. Experts who use meta build systems will have a much easier time opting in to this than new folks will have to opting out of it. (And even then, given 2, I think opting in to this is only attractive to non-win non-mac shops, and even then, given 3, I'm not sure it's all that useful there either.)</div></div></blockquote><div><br>Not sure there - projects like LLVM having lld provide bfd-ld-esque behavior gives a better chance the project won't regress the ability to link with bfd or gold (be a bit unfortunate if LLVM could only be linked with lld, or putting the cost on developers using bfd/gold to cleanup regressions other folks introduce). At least open source projects tend not to (so far as I know - probably fairly limited guess, admittedly) be "can only compile with this compile and only link with this linker" - probably have somewhat broad compatibility goals. Admittedly, for LLVM, maybe we'd want to opt-in to this behavior if using lld on any platform, not just ld.lld.<br><br>I'm not massively invested in the default behavior one way or the other here - and can appreciate that users with no interest in linker portability with bfd.ld/gold might find the semantics to be an unnecessary annoyance. I'd have thought most projects - anymore than a scratch program to be thrown away, is probably intended to be compilable/linkable with platform defaults/not restricted to one linker.<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>Nico</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Sep 3, 2020 at 7:08 PM Fāng-ruì Sòng <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2020-09-03, Peter Collingbourne wrote:<br>
>On Thu, Sep 3, 2020 at 2:00 PM Fāng-ruì Sòng <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>> wrote:<br>
><br>
>> On 2020-09-03, Peter Collingbourne wrote:<br>
>> >On Tue, Sep 1, 2020 at 5:35 PM Fāng-ruì Sòng via llvm-dev <<br>
>> ><a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>> ><br>
>> >> On 2020-09-01, Petr Hosek wrote:<br>
>> >> >I see the GNU ld behavior as a limitation, not as a feature, as Peter<br>
>> >> Smith<br>
>> >> >also pointed out in <a href="https://reviews.llvm.org/D86762" rel="noreferrer" target="_blank">https://reviews.llvm.org/D86762</a>. While it can be<br>
>> >> argued<br>
>> >> >that there are certain cases where it can help detect layering<br>
>> >> >violations as you mentioned in your change, I'm not sure how valuable<br>
>> that<br>
>> >> >is in practice. Every case I've encountered so far either in Chrome or<br>
>> in<br>
>> >> >Fuchsia was a valid use case, most commonly interceptors. The solution<br>
>> >> >has always been the same, wrap all libraries in<br>
>> --start-group/--stop-group<br>
>> >> >and it's what most projects do by default to avoid dealing with these<br>
>> >> >issues, see for example [Chromium](<br>
>> >> ><br>
>> >><br>
>> <a href="https://source.chromium.org/chromium/chromium/src/+/master:build/toolchain/gcc_toolchain.gni;l=409" rel="noreferrer" target="_blank">https://source.chromium.org/chromium/chromium/src/+/master:build/toolchain/gcc_toolchain.gni;l=409</a><br>
>> >> ).<br>
>> >> >In our case, compatibility with linkers on other platforms is more<br>
>> >> >important than compatibility with GNU ld, so I'd prefer to keep the<br>
>> >> current<br>
>> >> >behavior. Projects that care about compatibility with GNU ld can use<br>
>> >> >--warn-backrefs.<br>
>> >><br>
>> >> I totally understand that some users may not want to deal with GNU ld<br>
>> >> compatibility:) I'll then question about Chromium's addition of -z defs:<br>
>> >> <a href="https://crrev.com/843583006" rel="noreferrer" target="_blank">https://crrev.com/843583006</a> :)<br>
>> >><br>
>> >> -z defs is like a layering checking tool for shared objects while<br>
>> >> --warn-backrefs is for archives. For performance, ABI concerns and ease<br>
>> >> of deployment, many projects tend to build their own components as<br>
>> >> archives instead of shared objects. In this sense --warn-backrefs will<br>
>> >> probably be more useful than -z defs.<br>
>> >><br>
>> >> (<br>
>> >> TIL lorder and tsort were created to define an order of archives in<br>
>> >> early versions of Unix.<br>
>> >><br>
>> <a href="https://www.gnu.org/software/coreutils/manual/html_node/tsort-background.html" rel="noreferrer" target="_blank">https://www.gnu.org/software/coreutils/manual/html_node/tsort-background.html</a><br>
>> >> It seems that the article missed the point that proper library layering<br>
>> is<br>
>> >> still useful<br>
>> >> )<br>
>> >><br>
>> ><br>
>> >I'm not a fan of this idea of reframing GNU ld behavior as a "layering<br>
>> >checking tool". It is an incomplete layering checking tool because it does<br>
>> >not detect the scenario where, for example, you have the intended<br>
>> >dependency graph:<br>
>> ><br>
>> >A -> B<br>
>> >A -> C<br>
>> >B -> D<br>
>> >C -> D<br>
>> ><br>
>> >(resulting in -la -lb -lc -ld) and you have an unexpected dependency B -><br>
>> >C.<br>
>><br>
>> Yes, the GNU ld layering checking behavior is incomplete (yet important<br>
>> and sufficient if we aim for compatibility).<br>
>><br>
><br>
>As Petr mentioned, only certain users care about this aspect of GNU ld<br>
>compatibility, and those users can just turn this feature on.<br>
<br>
OK, I will consider you and Petr are two users not caring about this aspect of GNU ld.<br>
<br>
If the other side turns out to be overwhelming: the users who don't want this<br>
feature can turn it off (--no-warn-backrefs).<br>
<br>
>The build system pick two orders:<br>
>> -la -lb -lc -ld<br>
>> -la -lc -lb -ld<br>
>><br>
><br>
>Unless you have multiple programs linking against A, you've just introduced<br>
>non-determinism in your build tool, which is generally considered to be a<br>
>Bad Thing.<br>
><br>
><br>
>> >There is already a way to detect layering problems, that detects<br>
>> >practical layering problems and not just theoretical ones, which is to<br>
>> link<br>
>> >programs that use subsets of the libraries. For example, linking a program<br>
>> >that depends only on B would result in detecting the invalid B -> C<br>
>> >dependency.<br>
>><br>
>> This is actually cumbersome and is explicitly described in<br>
>> <a href="https://reviews.llvm.org/D86762" rel="noreferrer" target="_blank">https://reviews.llvm.org/D86762</a><br>
><br>
><br>
>Right, and as I mention below even that doesn't catch all cases. My point<br>
>is that there is already a way to detect "practical" layering problems,<br>
>defined as "layering problems that cause an undefined symbol error in<br>
>programs that are linked at the same time as the library is built".<br>
><br>
>- Users who don't care about GNU ld can link their libraries normally.<br>
>- Users who care about GNU ld can pass --warn-backrefs.<br>
><br>
>Users who care more about "theoretical" layering problems, such as users<br>
>who ship prebuilt archive files to customers, will not be satisfied by<br>
>--warn-backrefs, as this will not catch every possible layering problem<br>
>before shipping the library, as they will not have a copy of every<br>
>customer's program. Instead, they will be better served by a separate tool.<br>
><br>
>If B -> C is not specified,<br>
>><br>
>> * If people write B_test ("linking a program that depends only on B"),<br>
>>    they will notice the dependency issue immediately via the "undefined<br>
>> symbol" diagnostic.<br>
>> * If such a B_test does not exist. The user may work on a large<br>
>>    application which depends on B (and transitively on D) but not on A.<br>
>>    OK, they will get an undefined symbol from C. However, it is not<br>
>>    appropriate for them to add a dependency on C because they don't use C<br>
>>    directly. See the "If adding the dependency does not form a cycle:"<br>
>>    paragraph in D86762.<br>
>><br>
>>    If their application actually depends on A (thus they will get the<br>
>>    dependency on C), their link may or may not succeed with GNU ld,<br>
>>    depending on whether the build system picks<br>
>>    -la -lb -lc -ld or -la -lc -lb -ld, and the diagnostic can be very<br>
>>    confusing "undefined reference to" while C is actually linked.<br>
>><br>
>> >It's also worth noting that even that would only detect the<br>
>> >layering problem if the program depends on the part of B that depends on<br>
>> C.<br>
>><br>
>><br>
>> >A better way to go about achieving layering checking would IMHO be to<br>
>> >implement a separate tool (not part of the linker) that is capable of<br>
>> >a complete layering check. Such a tool would only depend on symbol table<br>
>> >features common to all object formats, so it could probably be implemented<br>
>> >generically.<br>
>> ><br>
>> >Peter<br>
>><br>
>> A standalone tool will not achieve sufficient ergonomics.<br>
>> It will read all input files and duplicate the symbol resolution work done<br>
>> by the linker,<br>
>> which can be slow. (ld.lld --warn-backrefs only imposes negligible<br>
>> overhead when a lazy object is fetched.)<br>
>><br>
><br>
>As I mentioned, it would do additional work that the linker is not<br>
>currently doing, which is necessary to implement a complete layering<br>
>checking tool, such as reading all archive members out of each archive<br>
>(whereas linkers only need to read depended-on archive members), and which<br>
>linkers should not be doing by default exactly for performance reasons.<br>
>Furthermore, the additional tool can be moved out of the critical<br>
>edit-build-run path, unlike a linker based tool, which should improve<br>
>performance as well.<br>
<br>
If the build system uses --start-lib more than archives, making the linker do<br>
the additional check may have little additional overhead. Lazy object files<br>
don't have the archive symbol table, which was much useful in the old days but<br>
limited in today's viewpoint. LLD already has quite a bit logic iterating the<br>
symbol table, doing one or two more things may not affect overall performance.<br>
<br>
>An additional tool would give the flexibility of allowing the interface to<br>
>specify the actual dependencies instead of just giving us only what we can<br>
>achieve as a result of historical design decisions. It would free build<br>
>tools like cmake or gn from needing to topologically sort libraries in<br>
>order to implement layering checking; instead, they can simply dump their<br>
>dependency graph.<br>
><br>
>IMHO, selling this feature as a layering checker is worse than having no<br>
>layering checker at all, because it will mislead users into thinking "oh,<br>
>I'm using lld's layering checker, that must mean that my program is<br>
>properly layered".<br>
><br>
>Peter<br>
<br>
The option name is still "--warn-backrefs", not "--check-layering". The selling<br>
point is more about the compatibility checker. While it does incomplete layering<br>
checking, it is an almost full GNU ld compatibility checking tool. I have<br>
personally checked dozens of executables with --warn-backrefs problems. Most<br>
don't link with GNU ld and the few remaining cases are whether symbol resolution<br>
will pick different definitions with LLD and GNU ld.<br>
<br>
>><br>
>> ><br>
>> >> >On Mon, Aug 31, 2020 at 1:44 PM Fāng-ruì Sòng <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>><br>
>> wrote:<br>
>> >> ><br>
>> >> >> On Mon, Aug 31, 2020 at 1:29 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>><br>
>> >> wrote:<br>
>> >> >> ><br>
>> >> >> ><br>
>> >> >> ><br>
>> >> >> > On Mon, Aug 31, 2020 at 1:24 PM Fāng-ruì Sòng <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>><br>
>> >> >> wrote:<br>
>> >> >> >><br>
>> >> >> >> On Fri, Aug 28, 2020 at 11:16 AM David Blaikie <<br>
>> <a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>><br>
>> >> >> wrote:<br>
>> >> >> >> ><br>
>> >> >> >> > Would you like to conduct the conversation here, or on the<br>
>> review<br>
>> >> >> thread? (I lean towards having them here, but don't mind if folks<br>
>> feel<br>
>> >> like<br>
>> >> >> it keeps the noise down & want to more post a notice saying "hey,<br>
>> here's<br>
>> >> >> this thing, if you're interested, go discuss it over there" - more an<br>
>> >> >> optional opt-in rather than requiring people to opt-out via muting<br>
>> the<br>
>> >> >> thread, etc)<br>
>> >> >> >><br>
>> >> >> >> Yes, we can conduct the "should we enable --warn-backrefs by<br>
>> default"<br>
>> >> >> >> conversation here. Since the semantics --warn-backrefs of are a<br>
>> bit<br>
>> >> >> >> complex, we need a documentation. <a href="https://reviews.llvm.org/D86762" rel="noreferrer" target="_blank">https://reviews.llvm.org/D86762</a><br>
>> is<br>
>> >> >> >> put up to get wording suggestions. Explicitly adding the people to<br>
>> >> the<br>
>> >> >> >> CC list...<br>
>> >> >> >><br>
>> >> >> >> FWIW for many code bases, --warn-backrefs should produce no<br>
>> warnings<br>
>> >> >> >> (error if --fatal-warnings). For some code bases, GNU ld may error<br>
>> >> >> >> "undefined reference".  --warn-backrefs can catch such problems.<br>
>> >> >> ><br>
>> >> >> ><br>
>> >> >> > One of the questions raised on the thread there was about different<br>
>> >> >> linker semantics. I assume the "--warn-backrefs by default" we're<br>
>> >> >> discussing is only related to the ld.lld frontend? Not the Windows<br>
>> >> linker<br>
>> >> >> lld behavior (or ld64 (old or new) lld behavior)?<br>
>> >> >><br>
>> >> >> Yes, the ELF port (lld/ELF). No other port has implemented<br>
>> >> >> --warn-backrefs. The intion of --warn-backrefs is to capture behavior<br>
>> >> >> differences with GNU ld's ELF ports. If traditional ELF linkers don't<br>
>> >> >> behave like that, I can replace "traditional ELF linkers" in<br>
>> >> >> <a href="https://reviews.llvm.org/D86762" rel="noreferrer" target="_blank">https://reviews.llvm.org/D86762</a> with "GNU linkers".<br>
>> >> >><br>
>> >> >> >><br>
>> >> >> >><br>
>> >> >> >><br>
>> >> >> >> > On Thu, Aug 27, 2020 at 10:15 PM Fangrui Song via llvm-dev <<br>
>> >> >> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>> >> >> >> >><br>
>> >> >> >> >> Hi all, LLD's --warn-backrefs is a tool to identify potential<br>
>> >> >> >> >> incompatible archive selection semantics with traditional Unix<br>
>> >> >> linkers.<br>
>> >> >> >> >> I have improved it (via D77522,D77630 and D77512) to a state<br>
>> >> where a<br>
>> >> >> >> >> --warn-backrefs diagnostic almost assuredly means that the link<br>
>> >> will<br>
>> >> >> >> >> fail with GNU ld, or the symbol will get different resolution<br>
>> in<br>
>> >> GNU<br>
>> >> >> ld and LLD.<br>
>> >> >> >> >><br>
>> >> >> >> >> My conclusion is that --warn-backrefs is a very useful layering<br>
>> >> >> check tool.<br>
>> >> >> >> >> I just wrote a documentation about the advantage (of GNU ld's<br>
>> >> archive<br>
>> >> >> >> >> selection semantics..... But we can do better with<br>
>> >> --warn-backrefs!<br>
>> >> >> >> >> GNU ld just reports "undefined reference" with no actionable<br>
>> >> feedback<br>
>> >> >> >> >> about the offending archive)<br>
>> >> >> >> >><br>
>> >> >> >> >> <a href="https://reviews.llvm.org/D86762" rel="noreferrer" target="_blank">https://reviews.llvm.org/D86762</a><br>
>> >> >> >> >><br>
>> >> >> >> >> I am wondering whether in the next release we can make<br>
>> >> >> --warn-backrefs<br>
>> >> >> >> >> the default.  I have added many known users to the review.<br>
>> >> >> >> >> (There is no need for --no-warn-backrefs because<br>
>> >> >> --warn-backrefs-exclude='*' does the same job)<br>
>> >> >> >> >> _______________________________________________<br>
>> >> >> >> >> LLVM Developers mailing list<br>
>> >> >> >> >> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> >> >> >> >> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>> >> >><br>
>> >><br>
>> >><br>
>> >> _______________________________________________<br>
>> >> LLVM Developers mailing list<br>
>> >> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> >> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>> >><br>
>> ><br>
>> ><br>
>> >--<br>
>> >--<br>
>> >Peter<br>
>><br>
><br>
><br>
>-- <br>
>-- <br>
>Peter<br>
</blockquote></div>
</blockquote></div></div>