bug#27866: Handle clang's internal libraries when finding compiler's internal libraries

[Alex Ameen]

Nice catch, I fat-fingered that one.

Could you share the workspace you tested in, or a snippet? I need to make a test case.


There was one aspect I wanted some clarity on though from the patches MSYS developed, which was "why is it that `m4/libtool.m4' used `*NAME*.a' when `build-aux/ltmain.in' used `*/libNAME*.$libext', and why they allowed `libgcc*' in `ltmain.in' but not `m4/libtool.m4'. I'm glad I looked into it because when I investigated the original patches' commit logs I realized that these were really just aimed at getting the builds for a small number of MSYS2 packages to succeed ( in the author's case VLC was the one they were focused on ); and that really this wasn't really designed to be a general purpose patch to `libtool'. From what I can tell MSYS2 is only using `libtool' for like 15 packages, so for their use case hard coding a few whitelisted libraries is totally fine. I'm actually really glad I dug into this more because In the field this would have caused serious UB issues - the likes of which are actually what led me to learn about linking/loading and eventually towards `libtool'.

I think my initial gut feeling on this one was right, and I'm probably going to revert that change and iron this out in a project branch that handles this alongside some related issues with flag-specs. The underlying problem here, which has been discussed in a few other threads is that `libtool' tries to outsmart the user and the compiler-collection by reinterpreting flags library linkage flags like `-l:libfoo.a' or `./libfoo.so', replacing them with `libfoo.la' and in rare cases even swapping system libs which entirely different alternatives for a particular platform.

In the MSYS patches their issue was that `libtool' doesn't respect the flag-spec/implicit linkage for either GCC or Clang - so libraries which are implicitly linked by the compiler-collection such as `libgcc_s.so' or `libclang_rt.a' don't get added, usually what you get instead is a manual invocation of `ld'. The result is that libtool removes certain libs which are normally added by the compiler-collection's invocation of `ld':

```
ld /path-to-libc/{crt1,crti}.o              \
   /path-to-cc/lib/crt<CC-init>.o           \  # CC adds, but not `libtool'
   <USER-FLAGS>                             \
   -L/path-to-cc/lib -l<CC-libc-overrides>  \  # CC adds, but not `libtool' ( almost always a static lib )
   -L/path-to-libc/lib -lc                  \
   -l<CC-runtime>                           \  # CC adds, but not `libtool' ( usually a shared lib )
   /path-to-cc/lib/crt<CC-cleanup>.o        \  # CC adds, but not `libtool' ( this one can get you in the most trouble )
   /path-to-libc/crtn.o;
```

While the issues they're dealing with for Clang really focus on the RT and libc override style libraries - the CC specific `.o' files can be a real silent killer ( I know from experience before becoming maintainer ). The solution I'd like for this is going to take some time to develop, and is going to require some real changes to the stated behavior of `libtool' ( definitely not backwards compatible ) - but `libtool' needs to either parse flag-specs and perfectly reproduce the underlying CC's implicit libs, or it needs to stop directly invoking `ld'.

The big issue I see with the MSYS patches, and similar ad-hoc patches that have been submitted to work around issues in flag-specs `-fsanitize=' for example ) is that they often explicitly add linkage for the relevant libraries, and fail to add linkage for the `.o' files. The nasty thing about this is that it'll probably work fine 99% of the time, but the 1% of the time that it misbehaves will be nearly impossible to debug. You have initialization/cleanup routine in those `.o' files that alter the initialization of executables and shared libraries in discreet ways which effect a subset of of the RT and libc override implementations provided by CCs - if those routines aren't linked you often won't get a failure at link-editing time, rather you'll get bizarre UB at runtime and ( in my experience ) by the time you figure out what's wrong you'll have read two books, read the manuals for CC and `ld' backwards and forwards a dozen times, GDB sessions will haunt your dreams, and four months of your life have passed by lol.

In my case the mistake didn't involve `libtool', it was just a dev dropping the ball while porting a Makefile from AIX to Linux. It's worth noting, that as obnoxious as it is that `libtool' doesn't try to replicate the CC linkage - in cases where people didn't try to work around this by manually linking their CC specific libs, they would be protected from the issue I encountered. I startled myself today realizing that I checked in a patch that would have caused `libtool' to make the exact issue that led me to learn about linking ( the irony would have been palpable ). I am interested to learn if the patches MSYS is using have caused the sort of UB I dealt with, I've got a list of at least 3 issues I'd wager they run into with C++ "old ABI", `pthread', and `dlopen'.

On 5/15/22 15:21, Martin Storsjö wrote:

> On Sun,
>       15 May 2022, Alex Ameen wrote:
>       
>       
> > Earlier this week I read through the
> >         thread, and created a patch based on the ones posted. This was
> >         checked if you would like to experiment with it.
> >         
> >         
> >         What I did notice was that this change has a wider effect than
> >         the problem statement initially suggests. I'm not crazy about
> >         the way it has a conditional behavior for two specific libraries
> >         since it is an ad-hoc solution directed at two
> >         compiler-collections, as opposed to a general purpose solution;
> >         but for the time being I see this as a practical change.
> >         
> >         
> >         As a side effect this change should also resolve issues with
> >         certain flag-specs such as `-fsanitize' which is nice; but the
> >         impact of unknown side effects is something I expect will rear
> >         its head in the near future. With that in mind, I think this is
> >         a necessary change, but I want to express up front that "I'm
> >         confident this will break a lot of existing builds, and I
> >         consider this to be a first draft".
> >         
> >         
> >         I would greatly appreciate y'all taking this for a spin on any
> >         available projects you have to get a sense of how it will behave
> >         "in the field". This change really effects "unspecified
> >         behavior" that the test-suite isn't designed to audit, but
> >         nonetheless has a practical effect on users.
> >         
> >       
>       
>       Thanks!
>       
>       
>       I tested this now, and it doesn't work quite as is - it needs this
>       modification:
>       
>       
>       diff --git a/m4/libtool.m4 b/m4/libtool.m4
>       
>       index ab5af335..9c084816 100644
>       
>       --- a/m4/libtool.m4
>       
>       +++ b/m4/libtool.m4
>       
>       @@ -7554,7 +7554,7 @@ if AC_TRY_EVAL(ac_compile); then
>       
>          for p in `eval "$output_verbose_link_cmd"`; do
>       
>            case $prev$p in
>       
>       
>       -    -L* | -R* | -l* | */clang_rt*.a)
>       
>       +    -L* | -R* | -l* | */libclang_rt*.a)
>       
>               # Some compilers place space between "-{L,R}" and the
>       path.
>       
>               # Remove the space.
>       
>               if test x-L = "$p" ||
>       
>       
>       (This was correct in one out of two instances in
>       1d2577357ee704da2d6d7c7da119ad82ba8ca172.)
>       
>       
>       With that changed, it does seem to work as it should for me on a
>       test project.
>       
>       
>       // Martin
>       
>       
>