[clang][Darwin] Prefer the toolchain-provided libc++.dylib if there is one #170303
Conversation
…s one When libc++ is bootstrapped with clang, the resulting clang uses the just-built libcxx headers from <install>/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 <install>/bin/../lib` to the linker search paths, which will allow the just-built libc++.dylib to be found. Fixes llvm#77653 rdar://107060541
tstellar
left a comment
tstellar
left a comment
There was a problem hiding this comment.
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
|
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. |
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.
It would add So if I do e.g. |
|
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. |
That's only the case if you do add e.g. |
It's not link time that's the problem but load/run time due to the |
ian-twilightcoder
left a comment
ian-twilightcoder
left a comment
There was a problem hiding this comment.
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.
|
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 |
I don't understand. There's no RPATH added by this patch.
Yeah, I agree this is not ideal and it was a tradeoff I was willing to make in order to fix this issue, but I think your suggestion is strictly better.
Another option would be to ship libc++ under a subdirectory and then add e.g. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver Author: Louis Dionne (ldionne) ChangesWhen libc++ is bootstrapped with clang, the resulting clang uses the just-built libcxx headers from <install>/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 Fixes #77653 Full diff: https://github.com/llvm/llvm-project/pull/170303.diff 10 Files Affected:
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"); // <install>/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 0000000000000..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 0000000000000..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 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/include/c++/v1/.keep b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/include/c++/v1/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++.a b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++.a
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++experimental.a b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++experimental.a
new file mode 100644
index 0000000000000..e69de29bb2d1d
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.cpp b/clang/test/Driver/darwin-link-libcxx.cpp
new file mode 100644
index 0000000000000..1c4f31b257512
--- /dev/null
+++ b/clang/test/Driver/darwin-link-libcxx.cpp
@@ -0,0 +1,81 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we link against the toolchain-provided libc++ built library when it is provided.
+// This is required to prefer the toolchain's libc++ over the system's libc++, which matches the behavior
+// we have for header search paths.
+
+// When libc++.dylib is NOT in the toolchain, we should use -lc++ and fall back to the libc++
+// in the sysroot.
+//
+// (1) Without -fexperimental-library.
+// RUN: %clangxx -### %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: "-lc++"
+// CHECK-1-NOT: "[[TOOLCHAIN]]/usr/lib"
+//
+// (2) With -fexperimental-library.
+// RUN: %clangxx -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: -fexperimental-library \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
+// RUN: --check-prefix=CHECK-2 %s
+// CHECK-2: "/usr/bin/ld"
+// CHECK-2: "-lc++" "-lc++experimental"
+// CHECK-2-NOT: "[[TOOLCHAIN]]/usr/lib"
+
+// When we have libc++.dylib in the toolchain, it should be used over the one in the sysroot.
+// There are a few cases worth testing.
+//
+// (1) Without -fexperimental-library.
+// RUN: %clangxx -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
+// RUN: --check-prefix=CHECK-3 %s
+// CHECK-3: "/usr/bin/ld"
+// CHECK-3: "[[TOOLCHAIN]]/usr/lib/libc++.dylib"
+// CHECK-3-NOT: "-lc++"
+//
+// (2) With -fexperimental-library.
+// RUN: %clangxx -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: -fexperimental-library \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
+// RUN: --check-prefix=CHECK-4 %s
+// CHECK-4: "/usr/bin/ld"
+// CHECK-4: "[[TOOLCHAIN]]/usr/lib/libc++.dylib"
+// CHECK-4: "[[TOOLCHAIN]]/usr/lib/libc++experimental.a"
+// CHECK-4-NOT: "-lc++"
+// CHECK-4-NOT: "-lc++experimental"
+
+// When we have libc++.a in the toolchain instead of libc++.dylib, it should be
+// used over the one in the sysroot.
+//
+// (1) Without -fexperimental-library.
+// RUN: %clangxx -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_static/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_static \
+// RUN: --check-prefix=CHECK-5 %s
+// CHECK-5: "/usr/bin/ld"
+// CHECK-5: "[[TOOLCHAIN]]/usr/lib/libc++.a"
+// CHECK-5-NOT: "-lc++"
+//
+// (2) With -fexperimental-library.
+// RUN: %clangxx -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_static/usr/bin \
+// RUN: -fexperimental-library \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_static \
+// RUN: --check-prefix=CHECK-6 %s
+// CHECK-6: "/usr/bin/ld"
+// CHECK-6: "[[TOOLCHAIN]]/usr/lib/libc++.a"
+// CHECK-6: "[[TOOLCHAIN]]/usr/lib/libc++experimental.a"
+// CHECK-6-NOT: "-lc++"
+// CHECK-6-NOT: "-lc++experimental"
diff --git a/clang/test/Driver/experimental-library-flag.cpp b/clang/test/Driver/experimental-library-flag.cpp
index 62b007516897e..0546a09d5518b 100644
--- a/clang/test/Driver/experimental-library-flag.cpp
+++ b/clang/test/Driver/experimental-library-flag.cpp
@@ -18,6 +18,8 @@
// CHECK: -fexperimental-library
// Depending on the stdlib in use, we should (or not) pass -lc++experimental.
-// CHECK-LIBCXX: -lc++experimental
-// CHECK-LIBSTDCXX-NOT: -lc++experimental
-// CHECK-NOSTDLIB-NOT: -lc++experimental
+// Note that we don't check for `-lc++experimental` specifically, since some targets
+// like Darwin pass the path to the library explicitly instead of using `-lx`.
+// CHECK-LIBCXX: c++experimental
+// CHECK-LIBSTDCXX-NOT: c++experimental
+// CHECK-NOSTDLIB-NOT: c++experimental
|
|
@llvm/pr-subscribers-clang Author: Louis Dionne (ldionne) ChangesWhen libc++ is bootstrapped with clang, the resulting clang uses the just-built libcxx headers from <install>/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 Fixes #77653 Full diff: https://github.com/llvm/llvm-project/pull/170303.diff 10 Files Affected:
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"); // <install>/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 0000000000000..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 0000000000000..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 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/include/c++/v1/.keep b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/include/c++/v1/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++.a b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++.a
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++experimental.a b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++experimental.a
new file mode 100644
index 0000000000000..e69de29bb2d1d
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.cpp b/clang/test/Driver/darwin-link-libcxx.cpp
new file mode 100644
index 0000000000000..1c4f31b257512
--- /dev/null
+++ b/clang/test/Driver/darwin-link-libcxx.cpp
@@ -0,0 +1,81 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we link against the toolchain-provided libc++ built library when it is provided.
+// This is required to prefer the toolchain's libc++ over the system's libc++, which matches the behavior
+// we have for header search paths.
+
+// When libc++.dylib is NOT in the toolchain, we should use -lc++ and fall back to the libc++
+// in the sysroot.
+//
+// (1) Without -fexperimental-library.
+// RUN: %clangxx -### %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: "-lc++"
+// CHECK-1-NOT: "[[TOOLCHAIN]]/usr/lib"
+//
+// (2) With -fexperimental-library.
+// RUN: %clangxx -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: -fexperimental-library \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
+// RUN: --check-prefix=CHECK-2 %s
+// CHECK-2: "/usr/bin/ld"
+// CHECK-2: "-lc++" "-lc++experimental"
+// CHECK-2-NOT: "[[TOOLCHAIN]]/usr/lib"
+
+// When we have libc++.dylib in the toolchain, it should be used over the one in the sysroot.
+// There are a few cases worth testing.
+//
+// (1) Without -fexperimental-library.
+// RUN: %clangxx -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
+// RUN: --check-prefix=CHECK-3 %s
+// CHECK-3: "/usr/bin/ld"
+// CHECK-3: "[[TOOLCHAIN]]/usr/lib/libc++.dylib"
+// CHECK-3-NOT: "-lc++"
+//
+// (2) With -fexperimental-library.
+// RUN: %clangxx -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: -fexperimental-library \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
+// RUN: --check-prefix=CHECK-4 %s
+// CHECK-4: "/usr/bin/ld"
+// CHECK-4: "[[TOOLCHAIN]]/usr/lib/libc++.dylib"
+// CHECK-4: "[[TOOLCHAIN]]/usr/lib/libc++experimental.a"
+// CHECK-4-NOT: "-lc++"
+// CHECK-4-NOT: "-lc++experimental"
+
+// When we have libc++.a in the toolchain instead of libc++.dylib, it should be
+// used over the one in the sysroot.
+//
+// (1) Without -fexperimental-library.
+// RUN: %clangxx -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_static/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_static \
+// RUN: --check-prefix=CHECK-5 %s
+// CHECK-5: "/usr/bin/ld"
+// CHECK-5: "[[TOOLCHAIN]]/usr/lib/libc++.a"
+// CHECK-5-NOT: "-lc++"
+//
+// (2) With -fexperimental-library.
+// RUN: %clangxx -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_static/usr/bin \
+// RUN: -fexperimental-library \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_static \
+// RUN: --check-prefix=CHECK-6 %s
+// CHECK-6: "/usr/bin/ld"
+// CHECK-6: "[[TOOLCHAIN]]/usr/lib/libc++.a"
+// CHECK-6: "[[TOOLCHAIN]]/usr/lib/libc++experimental.a"
+// CHECK-6-NOT: "-lc++"
+// CHECK-6-NOT: "-lc++experimental"
diff --git a/clang/test/Driver/experimental-library-flag.cpp b/clang/test/Driver/experimental-library-flag.cpp
index 62b007516897e..0546a09d5518b 100644
--- a/clang/test/Driver/experimental-library-flag.cpp
+++ b/clang/test/Driver/experimental-library-flag.cpp
@@ -18,6 +18,8 @@
// CHECK: -fexperimental-library
// Depending on the stdlib in use, we should (or not) pass -lc++experimental.
-// CHECK-LIBCXX: -lc++experimental
-// CHECK-LIBSTDCXX-NOT: -lc++experimental
-// CHECK-NOSTDLIB-NOT: -lc++experimental
+// Note that we don't check for `-lc++experimental` specifically, since some targets
+// like Darwin pass the path to the library explicitly instead of using `-lx`.
+// CHECK-LIBCXX: c++experimental
+// CHECK-LIBSTDCXX-NOT: c++experimental
+// CHECK-NOSTDLIB-NOT: c++experimental
|
|
As a side effect, this new way of linking against libc++ instead of passing 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. |
Sorry it's not the the |
|
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 |
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:
So, why is that more of a problem now than it was in the past? Simply because we've introduced more differences between the 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).
So, hopefully that is made clear by the above, but there is no intention to ever have |
|
Oh I see, and I was missing the part in |
cachemeifyoucan
left a comment
cachemeifyoucan
left a comment
There was a problem hiding this comment.
This version LGTM
|
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 |
|
Okay, I got the compiler-rt build to stop complaining. I think something that was previously a warning in I'm investigating the rpath situation now: So Now, when we build an executable using that toolchain: First, I can see that the executable references the rpath-relative However, when I run the executable, it fails at load time because it can't find the dylib in question: This all makes sense: we haven't told anyone what rpath the executable should be using. So, if we now tell it to use the right rpath when compiling: Now it's happy. So, I believe the right thing to do here is for whoever is actually installing the toolchain into its final destination to either:
I believe (2) is what everybody shipping a toolchain is already doing. I'm mostly writing this down to leave breadcrumbs of my reasoning. @tstellar, can you validate that we're OK with producing a toolchain that contains |
5 participants
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 <install>/bin/../libto the linker search paths, which will allow the just-built libc++.dylib to be found.Fixes #77653
rdar://107060541