Skip to content

Commit b5e52e5

Browse files
ldionneldionne
committed
Pass the specific library path instead, to avoid adding a new directory to -L
1 parent 65709c9 commit b5e52e5

File tree

10 files changed

+127
-63
lines changed

10 files changed

+127
-63
lines changed

‎clang/lib/Driver/ToolChains/Darwin.cpp‎

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -426,24 +426,6 @@ void darwin::Linker::AddLinkArgs(Compilation &C, const ArgList &Args,
426426
Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
427427
Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);
428428

429-
// Pass -L <toolchain-install>/bin/../lib/ to linker to prioritize the
430-
// toolchain's libc++.dylib over the sysroot-provided one. This matches
431-
// what we do for determining which libc++ headers to use.
432-
//
433-
// If the toolchain does not provide libc++.dylib, we will fall back to the
434-
// normal system search paths for finding libc++.dylib (on Darwin, that would
435-
// be in the SDK so we would find Apple's system libc++ instead of using
436-
// LLVM's).
437-
{
438-
llvm::SmallString<128> InstallBinPath =
439-
llvm::StringRef(D.Dir); // <install>/bin
440-
llvm::SmallString<128> InstallLibPath = InstallBinPath;
441-
llvm::sys::path::append(InstallLibPath, "..",
442-
"lib"); // <install>/bin/../lib
443-
CmdArgs.push_back("-L");
444-
CmdArgs.push_back(C.getArgs().MakeArgString(InstallLibPath));
445-
}
446-
447429
// Give --sysroot= preference, over the Apple specific behavior to also use
448430
// --isysroot as the syslibroot.
449431
// We check `OPT__sysroot_EQ` directly instead of `getSysRoot` to make sure we
@@ -2864,11 +2846,49 @@ void AppleMachO::AddCXXStdlibLibArgs(const ArgList &Args,
28642846
CXXStdlibType Type = GetCXXStdlibType(Args);
28652847

28662848
switch (Type) {
2867-
case ToolChain::CST_Libcxx:
2868-
CmdArgs.push_back("-lc++");
2849+
case ToolChain::CST_Libcxx: {
2850+
// On Darwin, we prioritize a libc++ located in the toolchain to a libc++
2851+
// located in the sysroot. Unlike the driver for most other platforms, on
2852+
// Darwin we do that by explicitly passing the library path to the linker
2853+
// to avoid having to add the toolchain's `lib/` directory to the linker
2854+
// search path, which would make other libraries findable as well.
2855+
//
2856+
// Prefering the toolchain library over the sysroot library matches the
2857+
// behavior we have for headers, where we prefer headers in the toolchain
2858+
// over headers in the sysroot if there are any. Note that it's important
2859+
// for the header search path behavior to match the link-time search path
2860+
// behavior to ensure that we link the program against a library that
2861+
// matches the headers that were used to compile it.
2862+
//
2863+
// Otherwise, we end up compiling against some set of headers and then
2864+
// linking against a different library (which, confusingly, shares the same
2865+
// name) which may have been configured with different options, be at a
2866+
// different version, etc.
2867+
SmallString<128> InstallLib = llvm::sys::path::parent_path(getDriver().Dir);
2868+
llvm::sys::path::append(InstallLib, "lib"); // <install>/bin/../lib
2869+
auto Link = [&](StringRef Library) {
2870+
SmallString<128> Shared(InstallLib);
2871+
llvm::sys::path::append(Shared,
2872+
SmallString<4>("lib") + Library + ".dylib");
2873+
SmallString<128> Static(InstallLib);
2874+
llvm::sys::path::append(Static, SmallString<4>("lib") + Library + ".a");
2875+
SmallString<32> Relative("-l");
2876+
Relative += Library;
2877+
2878+
if (getVFS().exists(Shared)) {
2879+
CmdArgs.push_back(Args.MakeArgString(Shared));
2880+
} else if (getVFS().exists(Static)) {
2881+
CmdArgs.push_back(Args.MakeArgString(Static));
2882+
} else {
2883+
CmdArgs.push_back(Args.MakeArgString(Relative));
2884+
}
2885+
};
2886+
2887+
Link("c++");
28692888
if (Args.hasArg(options::OPT_fexperimental_library))
2870-
CmdArgs.push_back("-lc++experimental");
2889+
Link("c++experimental");
28712890
break;
2891+
}
28722892

28732893
case ToolChain::CST_Libstdcxx:
28742894
// Unfortunately, -lstdc++ doesn't always exist in the standard search path;

‎clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib‎

Whitespace-only changes.

‎clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++experimental.a‎

Whitespace-only changes.

‎clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/bin/.keep‎

Whitespace-only changes.

‎clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/include/c++/v1/.keep‎

Whitespace-only changes.

‎clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++.a‎

Whitespace-only changes.

‎clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++experimental.a‎

Whitespace-only changes.

‎clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp‎

Lines changed: 0 additions & 39 deletions
This file was deleted.

‎clang/test/Driver/darwin-link-libcxx.cpp‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// UNSUPPORTED: system-windows
2+
3+
// Tests to check that we link against the toolchain-provided libc++ built library when it is provided.
4+
// This is required to prefer the toolchain's libc++ over the system's libc++, which matches the behavior
5+
// we have for header search paths.
6+
7+
// When libc++.dylib is NOT in the toolchain, we should use -lc++ and fall back to the libc++
8+
// in the sysroot.
9+
//
10+
// (1) Without -fexperimental-library.
11+
// RUN: %clangxx -### %s 2>&1 \
12+
// RUN: --target=x86_64-apple-darwin \
13+
// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
14+
// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
15+
// RUN: --check-prefix=CHECK-1 %s
16+
// CHECK-1: "/usr/bin/ld"
17+
// CHECK-1: "-lc++"
18+
// CHECK-1-NOT: "[[TOOLCHAIN]]/usr/lib"
19+
//
20+
// (2) With -fexperimental-library.
21+
// RUN: %clangxx -### %s 2>&1 \
22+
// RUN: --target=x86_64-apple-darwin \
23+
// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
24+
// RUN: -fexperimental-library \
25+
// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
26+
// RUN: --check-prefix=CHECK-2 %s
27+
// CHECK-2: "/usr/bin/ld"
28+
// CHECK-2: "-lc++" "-lc++experimental"
29+
// CHECK-2-NOT: "[[TOOLCHAIN]]/usr/lib"
30+
31+
// When we have libc++.dylib in the toolchain, it should be used over the one in the sysroot.
32+
// There are a few cases worth testing.
33+
//
34+
// (1) Without -fexperimental-library.
35+
// RUN: %clangxx -### %s 2>&1 \
36+
// RUN: --target=x86_64-apple-darwin \
37+
// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
38+
// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
39+
// RUN: --check-prefix=CHECK-3 %s
40+
// CHECK-3: "/usr/bin/ld"
41+
// CHECK-3: "[[TOOLCHAIN]]/usr/lib/libc++.dylib"
42+
// CHECK-3-NOT: "-lc++"
43+
//
44+
// (2) With -fexperimental-library.
45+
// RUN: %clangxx -### %s 2>&1 \
46+
// RUN: --target=x86_64-apple-darwin \
47+
// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
48+
// RUN: -fexperimental-library \
49+
// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
50+
// RUN: --check-prefix=CHECK-4 %s
51+
// CHECK-4: "/usr/bin/ld"
52+
// CHECK-4: "[[TOOLCHAIN]]/usr/lib/libc++.dylib"
53+
// CHECK-4: "[[TOOLCHAIN]]/usr/lib/libc++experimental.a"
54+
// CHECK-4-NOT: "-lc++"
55+
// CHECK-4-NOT: "-lc++experimental"
56+
57+
// When we have libc++.a in the toolchain instead of libc++.dylib, it should be
58+
// used over the one in the sysroot.
59+
//
60+
// (1) Without -fexperimental-library.
61+
// RUN: %clangxx -### %s 2>&1 \
62+
// RUN: --target=x86_64-apple-darwin \
63+
// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_static/usr/bin \
64+
// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_static \
65+
// RUN: --check-prefix=CHECK-5 %s
66+
// CHECK-5: "/usr/bin/ld"
67+
// CHECK-5: "[[TOOLCHAIN]]/usr/lib/libc++.a"
68+
// CHECK-5-NOT: "-lc++"
69+
//
70+
// (2) With -fexperimental-library.
71+
// RUN: %clangxx -### %s 2>&1 \
72+
// RUN: --target=x86_64-apple-darwin \
73+
// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_static/usr/bin \
74+
// RUN: -fexperimental-library \
75+
// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_static \
76+
// RUN: --check-prefix=CHECK-6 %s
77+
// CHECK-6: "/usr/bin/ld"
78+
// CHECK-6: "[[TOOLCHAIN]]/usr/lib/libc++.a"
79+
// CHECK-6: "[[TOOLCHAIN]]/usr/lib/libc++experimental.a"
80+
// CHECK-6-NOT: "-lc++"
81+
// CHECK-6-NOT: "-lc++experimental"

‎clang/test/Driver/experimental-library-flag.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
// CHECK: -fexperimental-library
1919

2020
// Depending on the stdlib in use, we should (or not) pass -lc++experimental.
21-
// CHECK-LIBCXX: -lc++experimental
22-
// CHECK-LIBSTDCXX-NOT: -lc++experimental
23-
// CHECK-NOSTDLIB-NOT: -lc++experimental
21+
// Note that we don't check for `-lc++experimental` specifically, since some targets
22+
// like Darwin pass the path to the library explicitly instead of using `-lx`.
23+
// CHECK-LIBCXX: c++experimental
24+
// CHECK-LIBSTDCXX-NOT: c++experimental
25+
// CHECK-NOSTDLIB-NOT: c++experimental

0 commit comments

Comments
 (0)