[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

Louis Dionne via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Sep 22 16:29:08 PDT 2018


ldionne added a comment.

In https://reviews.llvm.org/D45639#1242444, @phosek wrote:

> In https://reviews.llvm.org/D45639#1193112, @ldionne wrote:
>
> > @phosek I don't understand how you can expect code compiled with new headers to link against an old dylib, unless you're setting the target platform, in which case anything that would fail to link will instead be a compiler error. I mean, we're adding stuff to the dylib from one version to the next, so _of course_ it won't always work.
>
>
> @ldionne it's the other way round. Today, Clang driver on macOS automatically uses headers that are part of the compiler distribution (i.e. `-I../include/c++/v1`) but it always links against the system library (`/usr/lib/libc++.dylib`) because the library path to libraries that are shipped with the compiler (`-L../lib`) is never added to the link-line. Those two may not necessarily be the same version, and almost certainly won't be if you use Clang build from trunk.


Sorry, my comment was wrong. You're right, using new libc++ headers and linking against an old `libc++.dylib` is a supported use case, and in fact this is exactly what happens whenever you use new libc++ headers and link against the system-provided `libc++.dylib` on macOS. However, what is _not_ supported is linking against a new `libc++.dylib` and then trying to run using the system's `libc++.dylib`, which may be older.

If you add `-L<driver-path>/../lib`, will you start linking against the Clang-provided `libc++.dylib`? If so, and if you run the resulting application without setting the `DYLD_LIBRARY_PATH` to include the Clang-provided `libc++.dylib`, your program won't run because the system `libc++.dylib` may not include all symbols that the newer Clang-provided `libc++.dylib` contains.

> We hit this case again recently where developers are trying to use libc++ filesystem library. Using `#include <filesystem>` just works but `-lc++fs` fails because that library is not shipped as part of macOS at the moment, so developers need to add explicit `-L<path to compiler>/../lib` on macOS. This is not the case on other platforms such as Linux.

Not shipping filesystem on macOS is a design choice we're making because the filesystem library is not ABI stable. Adding `-L<path-to-compiler>/../lib` explicitly shows that you understand you're doing something unusual (and not officially supported), which is good.

I assume this does not happen on Linux because Linux distributions must include `libc++fs.a` as part of their system -- is that really the case?

> The purpose of this patch is to make the driver behave more consistently across platforms so developers who build their own Clang don't need to explicitly pass `-L<path to compiler>/../lib` on macOS.

Thanks for the good explanation -- now I understand the purpose of the patch. However, I think we need a larger discussion around how libc++ is shipped to users and what use cases we want to support. For example, one question I have is why we're even shipping `libc++.dylib`, `libc++abi.dylib` and `libunwind.dylib` in our LLVM releases for MacOS, given they are provided by the system (and mixing them is a recipe for disaster). Another question is whether the LLVM-provided Clang should instead always link to the LLVM-provided libraries (which would require users setting the `DYLD_LIBRARY_PATH` properly to avoid falling back onto the macOS-provided `libc++.dylib`).

I'm quite sympathetic to your use case (and in fact we have similar use cases), but I'm uncomfortable moving forward with this patch until we have a better understanding of some important surrounding questions. I'd like to talk with @dexonsmith about it and then maybe we can meet at the LLVM Dev Meeting (if you plan to attend) with other libc++ people to flesh those questions out?


Repository:
  rC Clang

https://reviews.llvm.org/D45639





More information about the cfe-commits mailing list