[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
ldionne wrote: Thanks for the reviews. For information, I'm currently investigating some new failures I'm seeing when doing a bootstrapping build. The compiler-rt build is complaining about something new I wasn't seeing before (but I did a clean reinstall of my system in between, so some stuff may have changed). Before I land this, I'll also want to make sure I understand the RPATH story for the toolchain we're building upstream -- since it's the first time we'll actually be linking against the `libc++.dylib` that we build in the toolchain, that is something new we haven't tested yet and I want to make sure everything behaves correctly. https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
https://github.com/cachemeifyoucan approved this pull request. This version LGTM https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
https://github.com/ian-twilightcoder approved this pull request. https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
@@ -2846,11 +2846,49 @@ void AppleMachO::AddCXXStdlibLibArgs(const ArgList
&Args,
CXXStdlibType Type = GetCXXStdlibType(Args);
switch (Type) {
- case ToolChain::CST_Libcxx:
-CmdArgs.push_back("-lc++");
+ case ToolChain::CST_Libcxx: {
+// On Darwin, we prioritize a libc++ located in the toolchain to a libc++
+// located in the sysroot. Unlike the driver for most other platforms, on
+// Darwin we do that by explicitly passing the library path to the linker
+// to avoid having to add the toolchain's `lib/` directory to the linker
+// search path, which would make other libraries findable as well.
+//
+// Prefering the toolchain library over the sysroot library matches the
+// behavior we have for headers, where we prefer headers in the toolchain
+// over headers in the sysroot if there are any. Note that it's important
+// for the header search path behavior to match the link-time search path
+// behavior to ensure that we link the program against a library that
+// matches the headers that were used to compile it.
+//
+// Otherwise, we end up compiling against some set of headers and then
+// linking against a different library (which, confusingly, shares the same
+// name) which may have been configured with different options, be at a
+// different version, etc.
+SmallString<128> InstallLib =
llvm::sys::path::parent_path(getDriver().Dir);
ian-twilightcoder wrote:
I have no idea, just saw that comment and wondered.
https://github.com/llvm/llvm-project/pull/170303
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
@@ -2846,11 +2846,49 @@ void AppleMachO::AddCXXStdlibLibArgs(const ArgList
&Args,
CXXStdlibType Type = GetCXXStdlibType(Args);
switch (Type) {
- case ToolChain::CST_Libcxx:
-CmdArgs.push_back("-lc++");
+ case ToolChain::CST_Libcxx: {
+// On Darwin, we prioritize a libc++ located in the toolchain to a libc++
+// located in the sysroot. Unlike the driver for most other platforms, on
+// Darwin we do that by explicitly passing the library path to the linker
+// to avoid having to add the toolchain's `lib/` directory to the linker
+// search path, which would make other libraries findable as well.
+//
+// Prefering the toolchain library over the sysroot library matches the
+// behavior we have for headers, where we prefer headers in the toolchain
+// over headers in the sysroot if there are any. Note that it's important
+// for the header search path behavior to match the link-time search path
+// behavior to ensure that we link the program against a library that
+// matches the headers that were used to compile it.
+//
+// Otherwise, we end up compiling against some set of headers and then
+// linking against a different library (which, confusingly, shares the same
+// name) which may have been configured with different options, be at a
+// different version, etc.
+SmallString<128> InstallLib =
llvm::sys::path::parent_path(getDriver().Dir);
ldionne wrote:
Actually, we do use `parent_path` for `libLTO.dylib`. I suspect the comment
about `InstallBin` being potentially relative might be outdated (it predates my
changes to the header search paths).
https://github.com/llvm/llvm-project/pull/170303
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
@@ -2846,11 +2846,49 @@ void AppleMachO::AddCXXStdlibLibArgs(const ArgList
&Args,
CXXStdlibType Type = GetCXXStdlibType(Args);
switch (Type) {
- case ToolChain::CST_Libcxx:
-CmdArgs.push_back("-lc++");
+ case ToolChain::CST_Libcxx: {
+// On Darwin, we prioritize a libc++ located in the toolchain to a libc++
+// located in the sysroot. Unlike the driver for most other platforms, on
+// Darwin we do that by explicitly passing the library path to the linker
+// to avoid having to add the toolchain's `lib/` directory to the linker
+// search path, which would make other libraries findable as well.
+//
+// Prefering the toolchain library over the sysroot library matches the
+// behavior we have for headers, where we prefer headers in the toolchain
+// over headers in the sysroot if there are any. Note that it's important
+// for the header search path behavior to match the link-time search path
+// behavior to ensure that we link the program against a library that
+// matches the headers that were used to compile it.
+//
+// Otherwise, we end up compiling against some set of headers and then
+// linking against a different library (which, confusingly, shares the same
+// name) which may have been configured with different options, be at a
+// different version, etc.
+SmallString<128> InstallLib =
llvm::sys::path::parent_path(getDriver().Dir);
ldionne wrote:
Yeah, that's a good point. I'll change it. I don't really understand how
`InstallBin` can be relative, but whatever.
https://github.com/llvm/llvm-project/pull/170303
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
ldionne wrote: > I'm still not sure this is a good idea, if there ever was a > XcodeDefault.xctoolchain/usr/lib/libc++.dylib it wouldn't be linkable from > anything outside of Xcode (its `LC_ID_DYLIB` would either be an absolute path > that would break if someone had their Xcode in a different location, or it > would be `@rpath` based which would break for anyone not contributing to > Xcode). Why can't we solve this with an extra linker search path in the > bootstrapping case? This whole change basically has nothing to do with Xcode or the way we distribute libc++ on Apple platforms as a system library. It's all about unbreaking the toolchain we produce in *upstream* builds of Clang and libc++. On Apple platforms, libc++ may be shipped in two different ways: 1. As a system library, with the headers and `.tbd` part of the SDK, and with the `.dylib` as part of the shared cache. When that's the case, Clang (whether AppleClang or LLVM Clang) will pick up libc++ from the `-isysroot`. This is the way we (at Apple) build and ship libc++. 2. Then, there's how everyone else is using libc++ based on upstream LLVM Clang: as part of the toolchain. In that world, we build Clang and libc++ within a single bootstrapping build, and libc++ is installed at `/usr/include/c++/v1` and `/usr/lib`. When you invoke Clang, you still provide a SDK because you need various headers to do anything useful (e.g. the `libc` headers and any other Apple system headers you might want to use). But that SDK already contains a different copy of `libc++` under `/usr/include/c++/v1` and `/usr/lib` (which has an arbitrary other version and has been configured differently -- think of it as being an entirely different library which happens to have the same name). Hence, Clang needs to do something in order to pick up the libc++ that you *actually* want to use, which is the one you built as part of your bootstrap and are shipping with your toolchain. That's why the Clang driver looks at whether `/usr/include/c++/v1` exists, and if so, [it uses that path](https://github.com/llvm/llvm-project/blob/ed6078c02368e0f2cf4025f1f03f27f00b4d6b5c/clang/lib/Driver/ToolChains/Darwin.cpp#L2753) instead of `/usr/include/c++/v1` for the C++ stdlib headers. Unfortunately, it doesn't do the same thing for the `dylib`, which leads to it mixing `/usr/include/c++/v1` with `/usr/lib`. Not only does that make no sense, this actually breaks in all kinds of way since, as I said, they're two different libraries. So for example the headers will advertise that some function exists in the dylib, but the dylib actually doesn't contain that function. At the moment, this fundamentally breaks any attempt to build and use upstream libc++ on Darwin as part of a toolchain -- which is pretty much as bad as it gets. This has resulted in numerous issues like the comments in https://github.com/llvm/llvm-project/issues/77653 and Homebrew being broken: https://github.com/Homebrew/homebrew-core/issues/235411. So, why is that more of a problem now than it was in the past? Simply because we've introduced more differences between the `libc++.dylib` we ship in the SDK and the upstream one (e.g. typed memory allocation). We've also added new symbols to the built library upstream, which means that the headers now think that `libc++.dylib` should provide those symbols. But if you link against the wrong `libc++.dylib`, well obviously they're not present. So, again, this has nothing to do with the way we ship libc++ within Xcode. In the Xcode case, there is no libc++ in the toolchain, so we always fall back to the SDK (for headers and for linking). https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
https://github.com/ian-twilightcoder edited https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
@@ -2846,11 +2846,49 @@ void AppleMachO::AddCXXStdlibLibArgs(const ArgList
&Args,
CXXStdlibType Type = GetCXXStdlibType(Args);
switch (Type) {
- case ToolChain::CST_Libcxx:
-CmdArgs.push_back("-lc++");
+ case ToolChain::CST_Libcxx: {
+// On Darwin, we prioritize a libc++ located in the toolchain to a libc++
+// located in the sysroot. Unlike the driver for most other platforms, on
+// Darwin we do that by explicitly passing the library path to the linker
+// to avoid having to add the toolchain's `lib/` directory to the linker
+// search path, which would make other libraries findable as well.
+//
+// Prefering the toolchain library over the sysroot library matches the
+// behavior we have for headers, where we prefer headers in the toolchain
+// over headers in the sysroot if there are any. Note that it's important
ian-twilightcoder wrote:
Is this strictly true? I don't think we ever add /usr/include/c++/v1
or /usr/lib/clang/nn/include/c++/v1
https://github.com/llvm/llvm-project/pull/170303
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
ian-twilightcoder wrote: I'm still not sure this is a good idea, if there ever was a XcodeDefault.xctoolchain/usr/lib/libc++ it wouldn't be linkable from anything outside of Xcode. Why can't we solve this with an extra linker search path in the bootstrapping case? https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
ian-twilightcoder wrote: > > It's not link time that's the problem but load/run time due to the > > `@loader_path` in its `LC_RPATH`. > > I don't understand. There's no RPATH added by this patch. Sorry it's not the the `@loader_path` in the `LC_RPATH` in the Xcode libraries that's the issue, it's that their `LC_ID_DYLIB` are all `@rpath` based, e.g. `@rpath/libclang.dylib`. That basically means they aren't usable outside of Xcode because you wouldn't be able to resolve `@rpath` to a given Xcode installation. https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
ldionne wrote: As a side effect, this new way of linking against libc++ instead of passing `-L` also avoids tripping the issue with the compiler-rt build. Indeed, since compiler-rt forcibly passes `-lc++` in its build (weird), Clang ends up passing both `-lc++` and `/lib/libc++.dylib` to the linker. However, since `/lib/libc++.dylib` doesn't contain the right architecture, it gets ignored by the linker and we fall back on the explicitly-specified `-lc++` by compiler-rt. The compiler-rt build is very wrong for doing that, but if we can avoid having to fix it to unblock this fix, that's a good thing. https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
https://github.com/ldionne updated
https://github.com/llvm/llvm-project/pull/170303
>From 65709c96d7596101330603ee469067a595104e05 Mon Sep 17 00:00:00 2001
From: Louis Dionne
Date: Tue, 2 Dec 2025 08:25:02 -0500
Subject: [PATCH 1/3] [clang][Darwin] Prefer the toolchain-provided
libc++.dylib if there is one
When libc++ is bootstrapped with clang, the resulting clang uses the
just-built libcxx headers from /bin/../include/c++/v1. However,
before this patch, clang would still use the system-provided libc++.dylib
(usually in the Apple SDK) because it would fail to add the corresponding
linker flag to find the just-built libc++. After this patch, Clang will
also add `-L /bin/../lib` to the linker search paths, which will
allow the just-built libc++.dylib to be found.
Fixes #77653
rdar://107060541
---
clang/lib/Driver/ToolChains/Darwin.cpp| 18 +
.../Driver/darwin-header-search-libcxx.cpp| 4 +-
.../darwin-link-libcxx-from-toolchain.cpp | 39 +++
3 files changed, 59 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp
b/clang/lib/Driver/ToolChains/Darwin.cpp
index fc3cd9030f71d..be17546c9c4ba 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -426,6 +426,24 @@ void darwin::Linker::AddLinkArgs(Compilation &C, const
ArgList &Args,
Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);
+ // Pass -L /bin/../lib/ to linker to prioritize the
+ // toolchain's libc++.dylib over the sysroot-provided one. This matches
+ // what we do for determining which libc++ headers to use.
+ //
+ // If the toolchain does not provide libc++.dylib, we will fall back to the
+ // normal system search paths for finding libc++.dylib (on Darwin, that would
+ // be in the SDK so we would find Apple's system libc++ instead of using
+ // LLVM's).
+ {
+llvm::SmallString<128> InstallBinPath =
+llvm::StringRef(D.Dir); // /bin
+llvm::SmallString<128> InstallLibPath = InstallBinPath;
+llvm::sys::path::append(InstallLibPath, "..",
+"lib"); // /bin/../lib
+CmdArgs.push_back("-L");
+CmdArgs.push_back(C.getArgs().MakeArgString(InstallLibPath));
+ }
+
// Give --sysroot= preference, over the Apple specific behavior to also use
// --isysroot as the syslibroot.
// We check `OPT__sysroot_EQ` directly instead of `getSysRoot` to make sure
we
diff --git a/clang/test/Driver/darwin-header-search-libcxx.cpp
b/clang/test/Driver/darwin-header-search-libcxx.cpp
index cc8ec9ceb89b3..e8985a4e22b2f 100644
--- a/clang/test/Driver/darwin-header-search-libcxx.cpp
+++ b/clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -92,7 +92,7 @@
// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-internal-isystem"
"[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1-NOT: "-internal-isystem"
"[[SYSROOT]]/usr/include/c++/v1"
-// Make sure that using -nostdinc, -nostdinc++ or -nostdlib will drop both the
toolchain
+// Make sure that using -nostdinc, -nostdinc++ or -nostdlibinc will drop both
the toolchain
// C++ include path and the sysroot one.
//
// RUN: %clang -### %s -fsyntax-only 2>&1 \
@@ -116,7 +116,7 @@
// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
// RUN: -stdlib=platform \
// RUN: -nostdinc++ \
-// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
+// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
// RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
// RUN: --check-prefix=CHECK-LIBCXX-NOSTDINCXX %s
// CHECK-LIBCXX-NOSTDINCXX: "-cc1"
diff --git a/clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
b/clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
new file mode 100644
index 0..1d35a1c6bbc0f
--- /dev/null
+++ b/clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -0,0 +1,39 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we pass -L /bin/../lib/ to the linker to
prioritize the toolchain's
+// libc++.dylib over the system's libc++.dylib on Darwin. This matches the
behavior we have for
+// header search paths, where we prioritize toolchain headers and then fall
back to the sysroot ones.
+
+// Check that we pass the right -L to the linker even when -stdlib=libc++ is
not passed.
+//
+// RUN: %clang -### %s 2>&1
\
+// RUN: --target=x86_64-apple-darwin
\
+// RUN: -ccc-install-dir
%S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx
\
+// RUN: --check-prefix=CHECK-1 %s
+//
+// CHECK-1: "/usr/bin/ld"
+// CHECK-1: "-L" "[[TOOLCHAIN]]/usr/bin/../lib"
+
+// Check that we pass the right -L
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
llvmbot wrote: @llvm/pr-subscribers-clang-driver Author: Louis Dionne (ldionne) Changes When libc++ is bootstrapped with clang, the resulting clang uses the just-built libcxx headers from/bin/../include/c++/v1. However, before this patch, clang would still use the system-provided libc++.dylib (usually in the Apple SDK) because it would fail to add the corresponding linker flag to find the just-built libc++. After this patch, Clang will also add `-L /bin/../lib` to the linker search paths, which will allow the just-built libc++.dylib to be found. Fixes #77653 rdar://107060541 --- Full diff: https://github.com/llvm/llvm-project/pull/170303.diff 10 Files Affected: - (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+41-3) - (added) clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib () - (added) clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++experimental.a () - (added) clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/bin/.keep () - (added) clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/include/c++/v1/.keep () - (added) clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++.a () - (added) clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++experimental.a () - (modified) clang/test/Driver/darwin-header-search-libcxx.cpp (+2-2) - (added) clang/test/Driver/darwin-link-libcxx.cpp (+81) - (modified) clang/test/Driver/experimental-library-flag.cpp (+5-3) ``diff diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index fc3cd9030f71d..6665b54ca473d 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2846,11 +2846,49 @@ void AppleMachO::AddCXXStdlibLibArgs(const ArgList &Args, CXXStdlibType Type = GetCXXStdlibType(Args); switch (Type) { - case ToolChain::CST_Libcxx: -CmdArgs.push_back("-lc++"); + case ToolChain::CST_Libcxx: { +// On Darwin, we prioritize a libc++ located in the toolchain to a libc++ +// located in the sysroot. Unlike the driver for most other platforms, on +// Darwin we do that by explicitly passing the library path to the linker +// to avoid having to add the toolchain's `lib/` directory to the linker +// search path, which would make other libraries findable as well. +// +// Prefering the toolchain library over the sysroot library matches the +// behavior we have for headers, where we prefer headers in the toolchain +// over headers in the sysroot if there are any. Note that it's important +// for the header search path behavior to match the link-time search path +// behavior to ensure that we link the program against a library that +// matches the headers that were used to compile it. +// +// Otherwise, we end up compiling against some set of headers and then +// linking against a different library (which, confusingly, shares the same +// name) which may have been configured with different options, be at a +// different version, etc. +SmallString<128> InstallLib = llvm::sys::path::parent_path(getDriver().Dir); +llvm::sys::path::append(InstallLib, "lib"); // /bin/../lib +auto Link = [&](StringRef Library) { + SmallString<128> Shared(InstallLib); + llvm::sys::path::append(Shared, + SmallString<4>("lib") + Library + ".dylib"); + SmallString<128> Static(InstallLib); + llvm::sys::path::append(Static, SmallString<4>("lib") + Library + ".a"); + SmallString<32> Relative("-l"); + Relative += Library; + + if (getVFS().exists(Shared)) { +CmdArgs.push_back(Args.MakeArgString(Shared)); + } else if (getVFS().exists(Static)) { +CmdArgs.push_back(Args.MakeArgString(Static)); + } else { +CmdArgs.push_back(Args.MakeArgString(Relative)); + } +}; + +Link("c++"); if (Args.hasArg(options::OPT_fexperimental_library)) - CmdArgs.push_back("-lc++experimental"); + Link("c++experimental"); break; + } case ToolChain::CST_Libstdcxx: // Unfortunately, -lstdc++ doesn't always exist in the standard search path; diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib b/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib new file mode 100644 index 0..e69de29bb2d1d diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++experimental.a b/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++experimental.a new file mode 100644 index 0..e69de29bb2d1d diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/bin/.keep b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/bin/.keep new file mode 100644 index 0..e69de29bb2d1d diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/include/c++/v1/.keep b/clang/test/Dr
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Louis Dionne (ldionne) Changes When libc++ is bootstrapped with clang, the resulting clang uses the just-built libcxx headers from/bin/../include/c++/v1. However, before this patch, clang would still use the system-provided libc++.dylib (usually in the Apple SDK) because it would fail to add the corresponding linker flag to find the just-built libc++. After this patch, Clang will also add `-L /bin/../lib` to the linker search paths, which will allow the just-built libc++.dylib to be found. Fixes #77653 rdar://107060541 --- Full diff: https://github.com/llvm/llvm-project/pull/170303.diff 10 Files Affected: - (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+41-3) - (added) clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib () - (added) clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++experimental.a () - (added) clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/bin/.keep () - (added) clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/include/c++/v1/.keep () - (added) clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++.a () - (added) clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++experimental.a () - (modified) clang/test/Driver/darwin-header-search-libcxx.cpp (+2-2) - (added) clang/test/Driver/darwin-link-libcxx.cpp (+81) - (modified) clang/test/Driver/experimental-library-flag.cpp (+5-3) ``diff diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index fc3cd9030f71d..6665b54ca473d 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2846,11 +2846,49 @@ void AppleMachO::AddCXXStdlibLibArgs(const ArgList &Args, CXXStdlibType Type = GetCXXStdlibType(Args); switch (Type) { - case ToolChain::CST_Libcxx: -CmdArgs.push_back("-lc++"); + case ToolChain::CST_Libcxx: { +// On Darwin, we prioritize a libc++ located in the toolchain to a libc++ +// located in the sysroot. Unlike the driver for most other platforms, on +// Darwin we do that by explicitly passing the library path to the linker +// to avoid having to add the toolchain's `lib/` directory to the linker +// search path, which would make other libraries findable as well. +// +// Prefering the toolchain library over the sysroot library matches the +// behavior we have for headers, where we prefer headers in the toolchain +// over headers in the sysroot if there are any. Note that it's important +// for the header search path behavior to match the link-time search path +// behavior to ensure that we link the program against a library that +// matches the headers that were used to compile it. +// +// Otherwise, we end up compiling against some set of headers and then +// linking against a different library (which, confusingly, shares the same +// name) which may have been configured with different options, be at a +// different version, etc. +SmallString<128> InstallLib = llvm::sys::path::parent_path(getDriver().Dir); +llvm::sys::path::append(InstallLib, "lib"); // /bin/../lib +auto Link = [&](StringRef Library) { + SmallString<128> Shared(InstallLib); + llvm::sys::path::append(Shared, + SmallString<4>("lib") + Library + ".dylib"); + SmallString<128> Static(InstallLib); + llvm::sys::path::append(Static, SmallString<4>("lib") + Library + ".a"); + SmallString<32> Relative("-l"); + Relative += Library; + + if (getVFS().exists(Shared)) { +CmdArgs.push_back(Args.MakeArgString(Shared)); + } else if (getVFS().exists(Static)) { +CmdArgs.push_back(Args.MakeArgString(Static)); + } else { +CmdArgs.push_back(Args.MakeArgString(Relative)); + } +}; + +Link("c++"); if (Args.hasArg(options::OPT_fexperimental_library)) - CmdArgs.push_back("-lc++experimental"); + Link("c++experimental"); break; + } case ToolChain::CST_Libstdcxx: // Unfortunately, -lstdc++ doesn't always exist in the standard search path; diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib b/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib new file mode 100644 index 0..e69de29bb2d1d diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++experimental.a b/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++experimental.a new file mode 100644 index 0..e69de29bb2d1d diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/bin/.keep b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/bin/.keep new file mode 100644 index 0..e69de29bb2d1d diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/include/c++/v1/.keep b/clang/test/Driver/In
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
https://github.com/ldionne updated
https://github.com/llvm/llvm-project/pull/170303
>From 65709c96d7596101330603ee469067a595104e05 Mon Sep 17 00:00:00 2001
From: Louis Dionne
Date: Tue, 2 Dec 2025 08:25:02 -0500
Subject: [PATCH 1/2] [clang][Darwin] Prefer the toolchain-provided
libc++.dylib if there is one
When libc++ is bootstrapped with clang, the resulting clang uses the
just-built libcxx headers from /bin/../include/c++/v1. However,
before this patch, clang would still use the system-provided libc++.dylib
(usually in the Apple SDK) because it would fail to add the corresponding
linker flag to find the just-built libc++. After this patch, Clang will
also add `-L /bin/../lib` to the linker search paths, which will
allow the just-built libc++.dylib to be found.
Fixes #77653
rdar://107060541
---
clang/lib/Driver/ToolChains/Darwin.cpp| 18 +
.../Driver/darwin-header-search-libcxx.cpp| 4 +-
.../darwin-link-libcxx-from-toolchain.cpp | 39 +++
3 files changed, 59 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp
b/clang/lib/Driver/ToolChains/Darwin.cpp
index fc3cd9030f71d..be17546c9c4ba 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -426,6 +426,24 @@ void darwin::Linker::AddLinkArgs(Compilation &C, const
ArgList &Args,
Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);
+ // Pass -L /bin/../lib/ to linker to prioritize the
+ // toolchain's libc++.dylib over the sysroot-provided one. This matches
+ // what we do for determining which libc++ headers to use.
+ //
+ // If the toolchain does not provide libc++.dylib, we will fall back to the
+ // normal system search paths for finding libc++.dylib (on Darwin, that would
+ // be in the SDK so we would find Apple's system libc++ instead of using
+ // LLVM's).
+ {
+llvm::SmallString<128> InstallBinPath =
+llvm::StringRef(D.Dir); // /bin
+llvm::SmallString<128> InstallLibPath = InstallBinPath;
+llvm::sys::path::append(InstallLibPath, "..",
+"lib"); // /bin/../lib
+CmdArgs.push_back("-L");
+CmdArgs.push_back(C.getArgs().MakeArgString(InstallLibPath));
+ }
+
// Give --sysroot= preference, over the Apple specific behavior to also use
// --isysroot as the syslibroot.
// We check `OPT__sysroot_EQ` directly instead of `getSysRoot` to make sure
we
diff --git a/clang/test/Driver/darwin-header-search-libcxx.cpp
b/clang/test/Driver/darwin-header-search-libcxx.cpp
index cc8ec9ceb89b3..e8985a4e22b2f 100644
--- a/clang/test/Driver/darwin-header-search-libcxx.cpp
+++ b/clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -92,7 +92,7 @@
// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-internal-isystem"
"[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1-NOT: "-internal-isystem"
"[[SYSROOT]]/usr/include/c++/v1"
-// Make sure that using -nostdinc, -nostdinc++ or -nostdlib will drop both the
toolchain
+// Make sure that using -nostdinc, -nostdinc++ or -nostdlibinc will drop both
the toolchain
// C++ include path and the sysroot one.
//
// RUN: %clang -### %s -fsyntax-only 2>&1 \
@@ -116,7 +116,7 @@
// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
// RUN: -stdlib=platform \
// RUN: -nostdinc++ \
-// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
+// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
// RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
// RUN: --check-prefix=CHECK-LIBCXX-NOSTDINCXX %s
// CHECK-LIBCXX-NOSTDINCXX: "-cc1"
diff --git a/clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
b/clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
new file mode 100644
index 0..1d35a1c6bbc0f
--- /dev/null
+++ b/clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -0,0 +1,39 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we pass -L /bin/../lib/ to the linker to
prioritize the toolchain's
+// libc++.dylib over the system's libc++.dylib on Darwin. This matches the
behavior we have for
+// header search paths, where we prioritize toolchain headers and then fall
back to the sysroot ones.
+
+// Check that we pass the right -L to the linker even when -stdlib=libc++ is
not passed.
+//
+// RUN: %clang -### %s 2>&1
\
+// RUN: --target=x86_64-apple-darwin
\
+// RUN: -ccc-install-dir
%S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx
\
+// RUN: --check-prefix=CHECK-1 %s
+//
+// CHECK-1: "/usr/bin/ld"
+// CHECK-1: "-L" "[[TOOLCHAIN]]/usr/bin/../lib"
+
+// Check that we pass the right -L
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
cachemeifyoucan wrote: I think searching for libcxx dylib in the install directory for bootstrap situation makes sense but I am very worried about ingesting such a search path in such a high priority location (before sys root). That can cause user unintentionally pick up libraries in there. I would rather if `clang++` can just explicitly link that libcxx if found with full path. https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
https://github.com/ian-twilightcoder requested changes to this pull request. I think we don't generally want this behavior for clients as it adds an undesired linker search path into Xcode, I think we want something special in clang/libc++ to find the libc++ that was previously built. https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
ian-twilightcoder wrote: > > It's a problem for the dylibs because you'll end up with a broken > > executable for rpath reasons. > > That's only the case if you do add e.g. `-lclang` but fail to provide the > appropriate `-L path/to/libclang/location` where the `libclang.dylib` you > actually intend to link against is located -- which can't happen because that > wouldn't link today. It's not link time that's the problem but load/run time due to the `@loader_path` in its `LC_RPATH`. The same is true for all of the dylibs in Xcode that aren't in SDKs. It allows Xcode to be relocatable but it means that nothing outside of the SDK is usable by anything outside of stuff in Xcode https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
ldionne wrote: > It's a problem for the dylibs because you'll end up with a broken executable > for rpath reasons. That's only the case if you do add e.g. `-lclang` but fail to provide the appropriate `-L path/to/libclang/location` where the `libclang.dylib` you actually intend to link against is located -- which can't happen because that wouldn't link today. https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
ian-twilightcoder wrote: It's a problem for the dylibs because you'll end up with a broken executable for rpath reasons. I don't think it makes sense to add XcodeDefault.xctoolchain/usr/lib to everyone's search paths by default. There's nothing in there that anyone should use outside of Xcode, and things that get packaged inside of Xcode. Maybe @cachemeifyoucan or https://github.com/jakepetroules have other opinions. https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
ldionne wrote: > Have you tested this with a multi-target runtime build? My fix was slightly > different so maybe you won't have the same problem, but I ran into an issue > where clang would pick up the libc++ build for watchos instead of the one > build for macOS. Here is an example of the error: > https://github.com/llvm/llvm-project/actions/runs/18506722806/job/52742884609?pr=163026 Yes, I am doing a bootstrapping build of Clang + compiler-rt + libc++ for multiple targets (well, it does that by default for compiler-rt AFAICT). However, I am running into a different issue that I am still investigating: the compiler-rt build is creating fat libraries for e.g. `x86_64`, `arm64` and `arm64e` in a single go, i.e. within a single build of compiler-rt. However, the libc++ build creates a library built for a single target, and instead it gets built multiple times via `runtimes/CMakeLists.txt`. With this patch, we (correctly) link against the just-built `/usr/lib/libc++.dylib` instead of the macOS system `/usr/lib/libc++.dylib`. However, since that library is NOT a fat library (unlike the compiler-rt libraries), we end up with unresolved symbols for the architectures that aren't present in `/usr/lib/libc++.dylib` when trying to link compiler-rt. I believe this is a pre-existing issue and it simply went unnoticed until now since we were silently using the SDK-provided `libc++.dylib`, which is a fat library. > Where does this resolve in Xcode? If it goes to > `Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib` or > `Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/21/lib/darwin` > then I don't think we want to be adding this, as the libraries in there > don't have a usable install name. It would add `-L <...>/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib`, however in a default Apple setup with Xcode, there wouldn't be any `libc++.dylib` to pick up from that location (so it would end up using the SDK-provided `libc++.tbd`). There are a few other libraries like `libclang.dylib` or `libIndexStore.dylib`, so it does mean that in principle if you add `-lclang` you would find `Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/libclang.dylib` after landing this patch whereas you wouldn't before this patch. I'm not sure I understand why that would be a problem, though? This is what's done on other platforms, for example on Linux the linker is passed `-L /usr/lib/llvm-22/bin/../lib` and that directory contains a bunch of stuff: ``` $ ls /usr/lib/llvm-22/bin/../lib LLVMPolly.so libLLVMARMInfo.a libLLVMBitReader.a libLLVMDiff.a libLLVMHipStdPar.a libLLVMM68kInfo.a libLLVMOrcDebugging.a libLLVMSelectionDAG.a libLLVMVEInfo.a libLLVMipo.a libclang.so libclangHandleLLVM.a libclangTidyBugproneModule.a libclangToolingInclusions.a libomp.so.5 LLVMgold.so libLLVMARMUtils.a libLLVMBitWriter.a libLLVMDlltoolDriver.a libLLVMIRPrinter.a libLLVMMC.a libLLVMOrcJIT.a libLLVMSparcAsmParser.a libLLVMVectorize.a libLTO.so libclangAPINotes.a libclangIncludeCleaner.a libclangTidyCERTModule.a libclangToolingInclusionsStdlib.a libompd.so amdgcn-amd-amdhsa libLLVMAVRAsmParser.a libLLVMBitstreamReader.a libLLVMExecutionEngine.a libLLVMIRReader.a libLLVMMCA.a libLLVMOrcShared.a libLLVMSparcCodeGen.a libLLVMWebAssemblyAsmParser.a libLTO.so.22.0libclangAST.a libclangIncludeFixer.alibclangTidyConcurrencyModule.a libclangToolingRefactoring.alibomptarget.so libLLVMAArch64AsmParser.a libLLVMAsmParser.a libLLVMCoroutines.a libLLVMExtensions.a libLLVMLanaiAsmParser.a libLLVMMSP430Desc.a libLLVMPowerPCDisassembler.a libLLVMSystemZCodeGen.a libLLVMWindowsManifest.a libc++.a libclangChangeNamespace.a libclangMove.a libclangTidyLLVMModule.a libclangdRemoteIndexServiceProto.a nvptx64-nvidia-cuda libLLVMAArch64CodeGen.a libLLVMAsmPrinter.a libLLVMCoverage.a libLLVMFileCheck.alibLLVMLanaiCodeGen.a libLLVMMSP430Disassembler.a libLLVMPowerPCInfo.a libLLVMSystemZDesc.a libLLVMX86AsmParser.a libc++.modules.json libclangCodeGen.a libclangParse.a libclangTidyLinuxKernelModule.alibclangdRemoteMarshalling.a python3 libLLVMAArch64Desc
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
ian-twilightcoder wrote: Where does this resolve in Xcode? If it goes to Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib or Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/21/lib/darwin then I don't think we want to be adding this, as the libraries in there don't have a usable install name. https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one (PR #170303)
https://github.com/tstellar commented: Have you tested this with a multi-target runtime build? My fix was slightly different so maybe you won't have the same problem, but I ran into an issue where clang would pick up the libc++ build for watchos instead of the one build for macOS. Here is an example of the error: https://github.com/llvm/llvm-project/actions/runs/18506722806/job/52742884609?pr=163026 https://github.com/llvm/llvm-project/pull/170303 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
