Page MenuHomePhabricator

[Driver] Automatically include C++ library dependencies
Needs ReviewPublic

Authored by phosek on Nov 19 2018, 2:13 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Existing drivers already include -lm as C++ library dependency, but
that alone is not sufficient when statically linking the C++ library.
To handle that case, we introduce a new method addCXXStdlibLinkDeps
which should automatically include all C++ library dependencies.

Diff Detail

Repository
rC Clang

Event Timeline

phosek created this revision.Nov 19 2018, 2:13 PM
dim added a subscriber: dim.Nov 19 2018, 10:54 PM

I think this is the wrong direction, placing "common" code in addCXXStdlibLinkDeps, which then has all kinds of ugly ifs and switches for different OSes? Let different OSes handle this in their own way, maybe.

Until now I have not encountered an issue where I needed to add -lpthread to the link command line for a static C++ application; I think that is only needed when you actually use threading primitives? But still, I would rather see this in the OS-dependent handling functions.

In D54724#1303809, @dim wrote:

I think this is the wrong direction, placing "common" code in addCXXStdlibLinkDeps, which then has all kinds of ugly ifs and switches for different OSes? Let different OSes handle this in their own way, maybe.

You mean handling this in individual ToolChain subclasses? I've considered doing that. The reason I went with a single method is because we already use this approach for other types of runtimes like sanitizers, XRay, etc. There'd be still some duplication for handling different C++ libraries, but I'd be fine changing the implementation to use that approach if there's a strong preference for doing so.

Until now I have not encountered an issue where I needed to add -lpthread to the link command line for a static C++ application; I think that is only needed when you actually use threading primitives? But still, I would rather see this in the OS-dependent handling functions.

Have you been using libc++ or just libstdc++? libstdc++ is cheating a bit, because it relies on threading functions that are provided by libgcc (which in turn call into pthread), so -lpthread is not needed when using libstdc++, even when you link it statically. libc++ doesn't rely on these functions because it supports different builtin libraries (like compiler-rt) hence having to specify -lpthread explicitly.

dim added a comment.Nov 20 2018, 11:00 PM
In D54724#1303809, @dim wrote:

I think this is the wrong direction, placing "common" code in addCXXStdlibLinkDeps, which then has all kinds of ugly ifs and switches for different OSes? Let different OSes handle this in their own way, maybe.

You mean handling this in individual ToolChain subclasses? I've considered doing that. The reason I went with a single method is because we already use this approach for other types of runtimes like sanitizers, XRay, etc. There'd be still some duplication for handling different C++ libraries, but I'd be fine changing the implementation to use that approach if there's a strong preference for doing so.

I think I understand the motivation, it's just that the toolchain classes were split up specifically to avoid if(OS==foo) ... else if(OS==bar) ... else if(OS==baz) mazes. So this feels a little like a regression in that sense, putting back the OS-specific ifs in a common function. But I guess it is a pattern that pops up more often.

Until now I have not encountered an issue where I needed to add -lpthread to the link command line for a static C++ application; I think that is only needed when you actually use threading primitives? But still, I would rather see this in the OS-dependent handling functions.

Have you been using libc++ or just libstdc++? libstdc++ is cheating a bit, because it relies on threading functions that are provided by libgcc (which in turn call into pthread), so -lpthread is not needed when using libstdc++, even when you link it statically. libc++ doesn't rely on these functions because it supports different builtin libraries (like compiler-rt) hence having to specify -lpthread explicitly.

We've been using libc++ by default since FreeBSD 10.x, and all older versions are now EOLed. That said, we have always used a linker script to link in libc++.so:

$ cat /usr/lib/libc++.so
/* $FreeBSD: projects/clang700-import/lib/libc++/libc++.ldscript 305102 2016-08-31 00:11:35Z bapt $ */
GROUP ( /usr/lib/libc++.so.1 /usr/lib/libcxxrt.so )

and for the static case, we've always just added the relevant .o files from libcxxrt into libc++.a:

$ ar tv /usr/lib/libc++.a
rw-r--r--       0/0         21720 Jan  1 01:00 1970 variant.o
rw-r--r--       0/0         61648 Jan  1 01:00 1970 valarray.o
rw-r--r--       0/0         18224 Jan  1 01:00 1970 utility.o
rw-r--r--       0/0         10656 Jan  1 01:00 1970 typeinfo.o
rw-r--r--       0/0        198080 Jan  1 01:00 1970 strstream.o
rw-r--r--       0/0         59264 Jan  1 01:00 1970 shared_mutex.o
rw-r--r--       0/0        231552 Jan  1 01:00 1970 regex.o
rw-r--r--       0/0         96200 Jan  1 01:00 1970 random.o
rw-r--r--       0/0         25240 Jan  1 01:00 1970 optional.o
rw-r--r--       0/0        239552 Jan  1 01:00 1970 iostream.o
rw-r--r--       0/0         17728 Jan  1 01:00 1970 functional.o
rw-r--r--       0/0        219984 Jan  1 01:00 1970 debug.o
rw-r--r--       0/0         58256 Jan  1 01:00 1970 chrono.o
rw-r--r--       0/0         35672 Jan  1 01:00 1970 charconv.o
rw-r--r--       0/0         21280 Jan  1 01:00 1970 bind.o
rw-r--r--       0/0         24312 Jan  1 01:00 1970 any.o
rw-r--r--       0/0       1010288 Jan  1 01:00 1970 algorithm.o
rw-r--r--       0/0         42944 Jan  1 01:00 1970 hash.o
rw-r--r--       0/0         45152 Jan  1 01:00 1970 cxxrt_typeinfo.o
rw-r--r--       0/0        278784 Jan  1 01:00 1970 cxxrt_libelftc_dem_gnu3.o
rw-r--r--       0/0         17704 Jan  1 01:00 1970 cxxrt_stdexcept.o
rw-r--r--       0/0        224664 Jan  1 01:00 1970 thread.o
rw-r--r--       0/0        218392 Jan  1 01:00 1970 future.o
rw-r--r--       0/0         31456 Jan  1 01:00 1970 exception.o
rw-r--r--       0/0       4604424 Jan  1 01:00 1970 locale.o
rw-r--r--       0/0         27216 Jan  1 01:00 1970 vector.o
rw-r--r--       0/0        102712 Jan  1 01:00 1970 mutex.o
rw-r--r--       0/0         63872 Jan  1 01:00 1970 memory.o
rw-r--r--       0/0       1261888 Jan  1 01:00 1970 ios.o
rw-r--r--       0/0         69528 Jan  1 01:00 1970 condition_variable.o
rw-r--r--       0/0        181368 Jan  1 01:00 1970 system_error.o
rw-r--r--       0/0       2006648 Jan  1 01:00 1970 string.o
rw-r--r--       0/0        105408 Jan  1 01:00 1970 stdexcept.o
rw-r--r--       0/0         37552 Jan  1 01:00 1970 new.o
rw-r--r--       0/0         10280 Jan  1 01:00 1970 cxxrt_memory.o
rw-r--r--       0/0          4880 Jan  1 01:00 1970 cxxrt_auxhelper.o
rw-r--r--       0/0          2872 Jan  1 01:00 1970 cxxrt_terminate.o
rw-r--r--       0/0        123504 Jan  1 01:00 1970 cxxrt_exception.o
rw-r--r--       0/0         18136 Jan  1 01:00 1970 cxxrt_dynamic_cast.o
rw-r--r--       0/0          5696 Jan  1 01:00 1970 cxxrt_guard.o

But I can link almost all C++ programs statically without having to add -lpthread. I guess you only really need it if you start using the multithreading primitives? I'm sort of hesitant to always add if, if it is not always needed. @emaste, what's your opinion on this?

In D54724#1304986, @dim wrote:

I think I understand the motivation, it's just that the toolchain classes were split up specifically to avoid if(OS==foo) ... else if(OS==bar) ... else if(OS==baz) mazes. So this feels a little like a regression in that sense, putting back the OS-specific ifs in a common function. But I guess it is a pattern that pops up more often.

I considered both options, the reason I went with conditionals is because we seem to use the same approach for other runtimes like compoler-rt builtins, sanitizers or XRay, but also because in this case there's likely going to be more duplication, e.g. the switch to handle either libstdc++ or libc++ would have to be duplicated in FreeBSD, NetBSD and OpenBSD drivers (unless we introduce a common base class for *BSD drivers). I could try implementing it the other way for comparison.

We've been using libc++ by default since FreeBSD 10.x, and all older versions are now EOLed. That said, we have always used a linker script to link in libc++.so:

$ cat /usr/lib/libc++.so
/* $FreeBSD: projects/clang700-import/lib/libc++/libc++.ldscript 305102 2016-08-31 00:11:35Z bapt $ */
GROUP ( /usr/lib/libc++.so.1 /usr/lib/libcxxrt.so )

I'm specifically focusing on static case, for shared case this is not relevant since libc++.so.1 already dynamically links all the dependencies.

and for the static case, we've always just added the relevant .o files from libcxxrt into libc++.a:

$ ar tv /usr/lib/libc++.a
rw-r--r--       0/0         21720 Jan  1 01:00 1970 variant.o
rw-r--r--       0/0         61648 Jan  1 01:00 1970 valarray.o
rw-r--r--       0/0         18224 Jan  1 01:00 1970 utility.o
rw-r--r--       0/0         10656 Jan  1 01:00 1970 typeinfo.o
rw-r--r--       0/0        198080 Jan  1 01:00 1970 strstream.o
rw-r--r--       0/0         59264 Jan  1 01:00 1970 shared_mutex.o
rw-r--r--       0/0        231552 Jan  1 01:00 1970 regex.o
rw-r--r--       0/0         96200 Jan  1 01:00 1970 random.o
rw-r--r--       0/0         25240 Jan  1 01:00 1970 optional.o
rw-r--r--       0/0        239552 Jan  1 01:00 1970 iostream.o
rw-r--r--       0/0         17728 Jan  1 01:00 1970 functional.o
rw-r--r--       0/0        219984 Jan  1 01:00 1970 debug.o
rw-r--r--       0/0         58256 Jan  1 01:00 1970 chrono.o
rw-r--r--       0/0         35672 Jan  1 01:00 1970 charconv.o
rw-r--r--       0/0         21280 Jan  1 01:00 1970 bind.o
rw-r--r--       0/0         24312 Jan  1 01:00 1970 any.o
rw-r--r--       0/0       1010288 Jan  1 01:00 1970 algorithm.o
rw-r--r--       0/0         42944 Jan  1 01:00 1970 hash.o
rw-r--r--       0/0         45152 Jan  1 01:00 1970 cxxrt_typeinfo.o
rw-r--r--       0/0        278784 Jan  1 01:00 1970 cxxrt_libelftc_dem_gnu3.o
rw-r--r--       0/0         17704 Jan  1 01:00 1970 cxxrt_stdexcept.o
rw-r--r--       0/0        224664 Jan  1 01:00 1970 thread.o
rw-r--r--       0/0        218392 Jan  1 01:00 1970 future.o
rw-r--r--       0/0         31456 Jan  1 01:00 1970 exception.o
rw-r--r--       0/0       4604424 Jan  1 01:00 1970 locale.o
rw-r--r--       0/0         27216 Jan  1 01:00 1970 vector.o
rw-r--r--       0/0        102712 Jan  1 01:00 1970 mutex.o
rw-r--r--       0/0         63872 Jan  1 01:00 1970 memory.o
rw-r--r--       0/0       1261888 Jan  1 01:00 1970 ios.o
rw-r--r--       0/0         69528 Jan  1 01:00 1970 condition_variable.o
rw-r--r--       0/0        181368 Jan  1 01:00 1970 system_error.o
rw-r--r--       0/0       2006648 Jan  1 01:00 1970 string.o
rw-r--r--       0/0        105408 Jan  1 01:00 1970 stdexcept.o
rw-r--r--       0/0         37552 Jan  1 01:00 1970 new.o
rw-r--r--       0/0         10280 Jan  1 01:00 1970 cxxrt_memory.o
rw-r--r--       0/0          4880 Jan  1 01:00 1970 cxxrt_auxhelper.o
rw-r--r--       0/0          2872 Jan  1 01:00 1970 cxxrt_terminate.o
rw-r--r--       0/0        123504 Jan  1 01:00 1970 cxxrt_exception.o
rw-r--r--       0/0         18136 Jan  1 01:00 1970 cxxrt_dynamic_cast.o
rw-r--r--       0/0          5696 Jan  1 01:00 1970 cxxrt_guard.o

We use that solution for libc++abi as well but that doesn't handle other dependencies.

But I can link almost all C++ programs statically without having to add -lpthread. I guess you only really need it if you start using the multithreading primitives? I'm sort of hesitant to always add if, if it is not always needed. @emaste, what's your opinion on this?

Correct, on other platforms you also need -ldl, but on *BSD this is part of libc AFAIK. We could also consider surrounding these dependencies with --push-state --as-needed and --pop-state to avoid linking them in if they're not needed.