[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#27866: Handle clang's internal libraries when finding compiler's int
From: |
Alex Ameen |
Subject: |
bug#27866: Handle clang's internal libraries when finding compiler's internal libraries |
Date: |
Sun, 15 May 2022 17:46:51 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 |
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