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

[Alex Ameen]

Hey I reread my message, and will make my way through your responses today, but something I forgot to mention was : really thank you for submitting these. I hope my response didn't come across as critical. I really do appreciate the work you've done here, and the effort you've made towards clarifying.

On Mon, May 16, 2022, 2:26 AM Martin Storsjö <martin@martin.st> wrote:

On Sun, 15 May 2022, Alex Ameen wrote:

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

This can reproduced with most libtool based projects that include C++
code. One example that I used for small standalone testing is this:
http://github.com/mstorsjo/fdk-aac

This project normally links the library as if it was C code, not C++, to
avoid unnecessary dependencies on the C++ standard library (which also
implicitly avoids this issue). To expose this issue, uncomment this line
in Makefile.am:

     libfdk_aac_la_LINK = $(LINK) $(libfdk_aac_la_LDFLAGS)

Then regenerate the build files with ./autogen.sh.

Then build the project with a toolchain from
https://github.com/mstorsjo/llvm-mingw/releases - there are prebuilt ones
for x86_64 and aarch64 linux and macos (and windows). Assuming you run on
linux, configure the project like this:

    ../fdk-aac/configure --host=x86_64-w64-mingw32

When building, you'll end up with this build error:

   CXXLD    libfdk-aac.la
ld.lld: error: undefined symbol: ___chkstk_ms
>>> referenced by ../fdk-aac/libAACdec/src/FDK_delay.cpp:130
>>>
libAACdec/src/.libs/FDK_delay.o:(FDK_Delay_Apply(FDK_SignalDelay*, long*,
unsigned int, unsigned char))
>>> referenced by ../fdk-aac/libAACdec/src/aacdec_hcr.cpp:435
>>>
libAACdec/src/.libs/aacdec_hcr.o:(HcrDecoder(CErHcrInfo*,
CAacDecoderChannelInfo*, SamplingRateInfo const*, FDK_BITSTREAM*))
>>> referenced by ../fdk-aac/libAACdec/src/aacdecoder.cpp:2520
>>>
libAACdec/src/.libs/aacdecoder.o:(CAacDecoder_DecodeFrame(AAC_DECODER_INSTANCE*,
unsigned int, long*, int, int))
>>> referenced 14 more times

Are you able to reproduce this setup, or do you want me to package up a
full build environment with the pre-setup code in this configuration?


> 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',

Which patch is that? In both
https://github.com/msys2/MSYS2-packages/blob/master/libtool/0011-Pick-up-clang_rt-static-archives-compiler-internal-l.patch
and
https://github.com/msys2/MSYS2-packages/blob/master/libtool/0013-Allow-statically-linking-compiler-support-libraries-.patch
it's spelled out as libclang_rt and libgcc, not *clang_rt.

> and why they allowed `libgcc*' in `ltmain.in' but not `m4/libtool.m4'.

The reason for that is that when clang refers to the clang_rt builtins
library, it does so by passing an absolute path to
libclang_rt.builtins-<arch>.a is specified in the link command (which
libtool tries to analyze and pick up, by running $CC -v).

https://github.com/msys2/MSYS2-packages/blob/master/libtool/0011-Pick-up-clang_rt-static-archives-compiler-internal-l.patch
makes sure that this library is picked up here. For the case of libgcc,
it's never specified as an absolute path to the library, but always
specified as -lgcc<suffixes> which libtool does pick up on. So the patch
to m4/libtool.m4 only needs special casing of libclang_rt.builtins; libgcc
is handled just fine by the existing pattern of -l*.

But later if either libclang_rt.builtins, or libgcc, is linked statically
(the only option for libclang_rt.builtins), they need to be marked as
allowed in build-aux/ltmain.in, in
https://github.com/msys2/MSYS2-packages/blob/master/libtool/0013-Allow-statically-linking-compiler-support-libraries-.patch.

> 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 );

I think that author you refer to is me.

> and that really this wasn't really designed to be a general purpose
> patch to `libtool'.

It was meant to be a as general purpose patch as possible, to fix the
issues I had observed - without having the entirely full picture as I'm
not the libtool maintainer of course. But they were indeed meant to be
upstreamable.

> 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.

That sounds like an incorrect conclusion.

The MSYS2 packaged and patched libtool is used when building the packages
in https://github.com/msys2/MINGW-packages/. Currently this is a bit over
2000 packages. Out of those, 320 have 'autoreconf' as part of their build
instructions, 63 contain 'autogen.sh', 13 contain 'libtoolize'.

So roughly at least around 400 packages are routinely built with the MSYS2
patched libtool installed in the packages - possibly some more that update
their bundled libtool via some other command.

> 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'.

Indeed, I never quite understood why libtool needs to build C++ libraries
with -nostdlib and try to replicate the compiler's default libraries in
the first place.

> 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.

I'm not familiar with the patches for '-fsanitize=' - but in the case of
the patch I'm primarily talking about, it's all about the single static
library libclang_rt.builtins-<arch>.a, which doesn't have any associated
object files. I'm not referring to the sanitizers (which do have a much
more tricky setup), but just the compiler helper builtins library, which
doesn't have any of the tricky-init-object-files issues.

// Martin