[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: Can someone review this patch? Exactly the same strategy (check if the desired subfolder is below `InstalledDir`, and, if not, check again below `Dir`), so this patch is consistent with the rest of the code. https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
@@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); ilg-ul wrote: Thank you, Louis @ldionne. If I understand your review right, the comments need a small update. That shouldn't be a problem. As for the tests, I have no experience with this testing infrastructure, but I'll try to figure out a solution. https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 1/3] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 2/3] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 3/3] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
@@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; ilg-ul wrote: Done. https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
@@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. ilg-ul wrote: Done. https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
@@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) ilg-ul wrote: Done. https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 1/4] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 2/4] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 3/4] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 1/5] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 2/5] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 3/5] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 1/6] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 2/6] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 3/6] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 1/7] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 2/7] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 3/7] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 1/8] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 2/8] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 3/8] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 1/9] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 2/9] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 3/9] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: I tried to write the test, but after several atempts I was not able to properly check the paths... :-( @ldionne, do you have any suggestions on how to fix the test? https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 01/10] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 02/10] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 03/10] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 01/11] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 02/11] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 03/11] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 01/12] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 02/12] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 03/12] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 01/13] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 02/13] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 03/13] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 01/14] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 02/14] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 03/14] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 01/15] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 02/15] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 03/15] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 01/16] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 02/16] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 03/16] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: After several more attempts I identified the reason for the failure: the folder structure in the build location does not include the `bin/../include/c++/v1` folder required for this to work: ``` clang version 18.0.0 (https://github.com/llvm-premerge-tests/llvm-project.git 6bfa3255358eeb2b109525059920e5f2b2abe41b) Target: x86_64-apple-darwin Thread model: posix InstalledDir: /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-tdgkv-1/llvm-project/clang-ci/build/build-clang/tools/clang/test/Driver/Output/darwin-header-search-libcxx.cpp.tmp/xpacks/.bin ignoring nonexistent directory "/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-tdgkv-1/llvm-project/clang-ci/build/build-clang/tools/clang/test/Driver/Output/darwin-header-search-libcxx.cpp.tmp/xpacks/.bin/../include/c++/v1" ignoring nonexistent directory "/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-tdgkv-1/llvm-project/clang-ci/build/build-clang/bin/../include/c++/v1" ``` After creating this include folder, the Linux test finally passed. I don't know if creating this folder is acceptable for the testing infrastructure, but if we want this test together with the bug fix, we probably have no other choice. Louis @ldionne, could you review the test code? https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
@@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); ilg-ul wrote: Test added. https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
@@ -172,3 +172,20 @@ // RUN: --check-prefix=CHECK-LIBCXX-STDLIB-UNSPECIFIED %s // CHECK-LIBCXX-STDLIB-UNSPECIFIED: "-cc1" // CHECK-LIBCXX-STDLIB-UNSPECIFIED: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" + +// Reproduce the xPack use case; there must be no include here, +// to select the executable folder. +// RUN: rm -rf %t/xpacks +// RUN: mkdir -pv %t/xpacks/.bin +// RUN: ln -svf %clang %t/xpacks/.bin/clang +// The build folders do not include this include; create it. +// RUN: mkdir -pv $(dirname $(which %clang))/../include/c++/v1 ilg-ul wrote: > `// UNSUPPORTED: system-windows` The current file already has this definition at top. Isn't this enough? > create a new temporary folder with the appropriate structure, and copy the > clang binary into it What if the binaries are built with absolute RPATH definitions? https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 01/17] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 02/17] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 03/17] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: I pushed a new test that copies `clang` to a local `install` folder, to avoid creating folders in the build location. The Linux build passed. Any other comments? https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: @ldionne, do you agree with the current changes? Since your request for changes currently blocks this PR. https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/70817 >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH 01/18] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); + if (getVFS().exists(InstallBin)) { +addSystemInclude(DriverArgs, CC1Args, InstallBin); +return; + } else if (DriverArgs.hasArg(options::OPT_v)) { +llvm::errs() << "ignoring nonexistent directory \"" << InstallBin + << "\"\n"; + } +} + // Otherwise, check for (2) llvm::SmallString<128> SysrootUsr = Sysroot; llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1"); >From b2f287c6a53697ac9bdce3eaa1ce55d345d734b1 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:05:50 +0200 Subject: [PATCH 02/18] Darwin.cpp: update comments --- clang/lib/Driver/ToolChains/Darwin.cpp | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index de55307385966cf..e6bcf0227d7dc65 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2470,14 +2470,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( switch (GetCXXStdlibType(DriverArgs)) { case ToolChain::CST_Libcxx: { -// On Darwin, libc++ can be installed in one of the following two places: +// On Darwin, libc++ can be installed in one of the following places: // 1. Alongside the compiler in /include/c++/v1 -// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1 +// 2. Alongside the compiler in /../include/c++/v1 +// 3. In a SDK (or a custom sysroot) in /usr/include/c++/v1 // -// The precendence of paths is as listed above, i.e. we take the first path -// that exists. Also note that we never include libc++ twice -- we take the -// first path that exists and don't send the other paths to CC1 (otherwise +// The precedence of paths is as listed above, i.e. we take the first path +// that exists. Note that we never include libc++ twice -- we take the first +// path that exists and don't send the other paths to CC1 (otherwise // include_next could break). +// +// Also note that in most cases, (1) and (2) are exactly the same path. +// Those two paths will differ only when the `clang` program being run +// is actually a symlink to the real executable. // Check for (1) // Get from '/bin' to '/include/c++/v1'. @@ -2494,7 +2499,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } -// Check for the folder where the executable is located, if different. +// (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { InstallBin = llvm::StringRef(getDriver().Dir.c_str()); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); >From 31b7482ffabda61c005a4cd90a07bcfec65c232e Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 10 Nov 2023 12:08:51 +0200 Subject: [PATCH 03/18] Darwin.cpp: construct StringRef from string --- clang/lib/Driver/ToolChains/Darwin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index e6bcf0227d7dc65..d89b6d60f1852e4 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2501,7 +2501,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( // (2) Check for the folder where the executable is located, if different. if (getDriver().getInstalledDir() != getDriver().Dir) { - InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + InstallBin = llvm::StringRef(getDriver().Dir); llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: > assuming CI passes It passed on my Mac, let's hope that the CI will pass too. But, not related to this PR, on my Mac, I always had several test failing: ``` Failed Tests (7): Clang :: Driver/baremetal.cpp Clang :: Driver/csky-toolchain.c Clang :: Driver/freebsd-include-paths.c Clang :: Driver/hexagon-toolchain-linux.c Clang :: Driver/sysroot.c Clang :: Frontend/warning-poison-system-directories.c Clang :: Index/pch-from-libclang.c Testing Time: 844.78s Skipped :36 Unsupported : 3071 Passed : 30977 Expectedly Failed:24 Failed : 7 FAILED: tools/clang/test/CMakeFiles/check-clang /Users/ilg/Work/xpack-dev-tools-build/clang-17.0.5-1/darwin-x64/x86_64-apple-darwin23.1.0/build/llvm-17.0.5/tools/clang/test/CMakeFiles/check-clang ``` Is this expected? https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: The CI passed. Do we need a second review, or the PR can be merged? https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: Thank you, Louis @ldionne, I highly appreciate your help! https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: The ppc64 build failed with: ``` RUN: at line 186: mkdir -pv /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Driver/Output/darwin-header-search-libcxx.cpp.tmp/install/bin + mkdir -pv /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Driver/Output/darwin-header-search-libcxx.cpp.tmp/install/bin mkdir: illegal option -- v Usage: mkdir [-p] [-e] [-m mode] Directory ... ``` How can I fix the test now? https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Remove -v from mkdir & ln -s from a test since it fails on ppc64 (PR #72924)
https://github.com/ilg-ul created https://github.com/llvm/llvm-project/pull/72924 The PR https://github.com/llvm/llvm-project/pull/70817 introduced a small bug, the tests failed on ppc64 with: ``` RUN: at line 186: mkdir -pv /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Driver/Output/darwin-header-search-libcxx.cpp.tmp/install/bin + mkdir -pv /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Driver/Output/darwin-header-search-libcxx.cpp.tmp/install/bin mkdir: illegal option -- v Usage: mkdir [-p] [-e] [-m mode] Directory ... ``` This PR removes the verbose flag from both `mkdir` and `ln -s`. >From bf70f0276bc7db350d0d1b0b5a97800cee2d8ac6 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 21 Nov 2023 01:24:49 +0200 Subject: [PATCH] Remove -v from mkdir & ln -s --- clang/test/Driver/darwin-header-search-libcxx.cpp | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/clang/test/Driver/darwin-header-search-libcxx.cpp b/clang/test/Driver/darwin-header-search-libcxx.cpp index 8f530299d53c6d8..70cc06090a993d6 100644 --- a/clang/test/Driver/darwin-header-search-libcxx.cpp +++ b/clang/test/Driver/darwin-header-search-libcxx.cpp @@ -183,15 +183,15 @@ // local folder hierarchy that meets this requirement. // Note: this might not work with weird RPATH configurations. // RUN: rm -rf %t/install -// RUN: mkdir -pv %t/install/bin +// RUN: mkdir -p %t/install/bin // RUN: cp %clang %t/install/bin/clang -// RUN: mkdir -pv %t/install/include/c++/v1 +// RUN: mkdir -p %t/install/include/c++/v1 // Headers in (1) and in (2) -> (1) is preferred over (2) // RUN: rm -rf %t/symlinked1 -// RUN: mkdir -pv %t/symlinked1/bin -// RUN: ln -svf %t/install/bin/clang %t/symlinked1/bin/clang -// RUN: mkdir -pv %t/symlinked1/include/c++/v1 +// RUN: mkdir -p %t/symlinked1/bin +// RUN: ln -sf %t/install/bin/clang %t/symlinked1/bin/clang +// RUN: mkdir -p %t/symlinked1/include/c++/v1 // RUN: %t/symlinked1/bin/clang -### %s -fsyntax-only 2>&1 \ // RUN: --target=x86_64-apple-darwin \ @@ -207,8 +207,8 @@ // Headers in (2) and in (3) -> (2) is preferred over (3) // RUN: rm -rf %t/symlinked2 -// RUN: mkdir -pv %t/symlinked2/bin -// RUN: ln -svf %t/install/bin/clang %t/symlinked2/bin/clang +// RUN: mkdir -p %t/symlinked2/bin +// RUN: ln -sf %t/install/bin/clang %t/symlinked2/bin/clang // RUN: %t/symlinked2/bin/clang -### %s -fsyntax-only 2>&1 \ // RUN: --target=x86_64-apple-darwin \ ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: I opened https://github.com/llvm/llvm-project/pull/72924 to fix this bug. https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Remove -v from mkdir & ln -s from a test since it fails on ppc64 (PR #72924)
ilg-ul wrote: @ldionne, could you take a look? This should fix the bug introduced by the previous PR. https://github.com/llvm/llvm-project/pull/72924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver, test] Remove -v from mkdir & ln -s from a test since it fails on AIX (PR #72924)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/72924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver, test] Remove -v from mkdir & ln -s from a test since it fails on AIX (PR #72924)
ilg-ul wrote: Thank you @MaskRay for correcting the title and for the approval. Please also merge it when possible. https://github.com/llvm/llvm-project/pull/72924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver, test] Remove -v from mkdir & ln -s from a test since it fails on AIX (PR #72924)
ilg-ul wrote: Great! Thank you! https://github.com/llvm/llvm-project/pull/72924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver, test] Remove -v from mkdir & ln -s from a test since it fails on AIX (PR #72924)
ilg-ul wrote: Is there a page documenting these tags? https://github.com/llvm/llvm-project/pull/72924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang] [clang][driver] Fix an issue where clang does not correctly resolve the system header if invoked via symlink (on MacOS) (PR #68091)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/68091 >From f3812174546270051c4a2903b9a99408bf5b7ba0 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:07:48 +0300 Subject: [PATCH 1/6] [clang][driver] Use platform specific calls to get the executable absolute path (#66704) In clang/tools/driver/driver.cpp, the function SetInstallDir() tries to determine the location where the toolchain is installed, basically by taking the parent folder of the executable absolute path. This is not correct when the compiler is invoked via a link, since it returns the parent of the link. This leads to subtle errors, for example on macOS it silently picks the wrong system headers. --- clang/tools/driver/driver.cpp | 61 +++ 1 file changed, 61 insertions(+) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 531b5b4a61c1804..c8ad167cba0a423 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -57,6 +57,15 @@ using namespace clang; using namespace clang::driver; using namespace llvm::opt; +#if defined(__linux__) +#include +#elif defined(__APPLE__) +#include +#include +#elif defined(__MINGW32__) +#include +#endif + std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) { if (!CanonicalPrefixes) { SmallString<128> ExecutablePath(Argv0); @@ -331,6 +340,56 @@ static void SetInstallDir(SmallVectorImpl &argv, // path being a symlink. SmallString<128> InstalledPath(argv[0]); +#if defined(__linux__) + + char ProcessAbsolutePath[PATH_MAX]; + + int len = readlink("/proc/self/exe", ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__APPLE__) + + // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call + // fails with ENOMEM (12) - Cannot allocate memory. + // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + + int len = proc_pidpath(getpid(), ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: proc_pidpath() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__MINGW32__) + + char ProcessAbsolutePath[PATH_MAX]; + + len = GetModuleFileName(NULL, ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: GetModuleFileName() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#else + // Do a PATH lookup, if there are no directory components. if (llvm::sys::path::filename(InstalledPath) == InstalledPath) if (llvm::ErrorOr Tmp = llvm::sys::findProgramByName( @@ -341,6 +400,8 @@ static void SetInstallDir(SmallVectorImpl &argv, if (CanonicalPrefixes) llvm::sys::fs::make_absolute(InstalledPath); +#endif + StringRef InstalledPathParent(llvm::sys::path::parent_path(InstalledPath)); if (llvm::sys::fs::exists(InstalledPathParent)) TheDriver.setInstalledDir(InstalledPathParent); >From eb7b122d5e6ce160d3f0880aac44a2df720d182f Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:16:30 +0300 Subject: [PATCH 2/6] clang-format driver.cpp patch --- clang/tools/driver/driver.cpp | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index c8ad167cba0a423..45e1469aafca95e 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -346,7 +346,7 @@ static void SetInstallDir(SmallVectorImpl &argv, int len = readlink("/proc/self/exe", ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) { llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " << strerror(errno) << "\n"; exit(1); @@ -360,11 +360,11 @@ static void SetInstallDir(SmallVectorImpl &argv, // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call // fails with ENOMEM (12) - Cannot allocate memory. // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c - char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE + 1]; int len = proc_pidpath(getpid(), ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) { llvm::errs() << "Inte
[llvm] [clang] [clang-tools-extra] [clang][driver] Fix an issue where clang does not correctly resolve the system header if invoked via symlink (on MacOS) (PR #68091)
https://github.com/ilg-ul closed https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
https://github.com/ilg-ul created https://github.com/llvm/llvm-project/pull/70817 On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. ```console % ln -s /Users/ilg/Library/xPacks/@xpack-dev-tools/clang/15.0.7-3.1/.content/bin/clang++ ~/tmp/clang++ % ~/tmp/clang++ -v hello.cpp -stdlib=libc++ xPack x86_64 clang version 15.0.7 (https://github.com/xpack-dev-tools/clang-xpack 9b1ff65945b1aaddfe7c0c4d99001ebca5d67b03) Target: x86_64-apple-darwin21.6.0 Thread model: posix InstalledDir: /Users/ilg/tmp/. <--- !!! ignoring nonexistent directory "/Users/ilg/tmp/./../include/c++/v1" <--- !!! "/Users/ilg/Library/xPacks/@xpack-dev-tools/clang/15.0.7-3.1/.content/bin/clang-15" -cc1 -triple x86_64-apple-macosx12.0.0 -Wundef-prefix=TARGET_OS_ -Werror=undef-prefix -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -emit-obj -mrelax-all --mrelax-relocations -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name hello.cpp -mrelocation-model pic -pic-level 2 -mframe-pointer=all -ffp-contract=on -fno-rounding-math -funwind-tables=2 -fcompatibility-qualified-id-block-type-checking -fvisibility-inlines-hidden-static-local-var -target-cpu penryn -tune-cpu generic -mllvm -treat-scalable-fixed-error-as-warning -debugger-tuning=lldb -target-linker-version 409.12 -v -fcoverage-compilation-dir=/Users/ilg/tmp -resource-dir /Users/ilg/Library/xPacks/@xpack-dev-tools/clang/15.0.7-3.1/.content/lib/clang/15.0.7 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -stdlib=libc++ -internal-isystem /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1 -internal-isystem /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/local/include -internal-isystem /Users/ilg/Library/xPacks/@xpack-dev-tools/clang/15.0.7-3.1/.content/lib/clang/15.0.7/include -internal-externc-isystem /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include -fdeprecated-macro -fdebug-compilation-dir=/Users/ilg/tmp -ferror-limit 19 -stack-protector 1 -fblocks -fencode-extended-block-signature -fregister-global-dtors-with-atexit -fgnuc-version=4.2.1 -fcxx-exceptions -fexceptions -fmax-type-align=16 -fcolor-diagnostics -D__GCC_HAVE_DWARF2_CFI_ASM=1 -o /var/folders/gr/13tt3vcd7m1gnbhwtkmf5cnwgn/T/hello-a87934.o -x c++ hello.cpp clang -cc1 version 15.0.7 based upon LLVM 15.0.7 default target x86_64-apple-darwin21.6.0 ignoring nonexistent directory "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/local/include" ignoring nonexistent directory "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/Library/Frameworks" #include "..." search starts here: #include <...> search starts here: /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1 <--- Wrong! /Users/ilg/Library/xPacks/@xpack-dev-tools/clang/15.0.7-3.1/.content/lib/clang/15.0.7/include /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks (framework directory) End of search list. "/usr/bin/ld" -demangle -lto_library /Users/ilg/Library/xPacks/@xpack-dev-tools/clang/15.0.7-3.1/.content/lib/libLTO.dylib -no_deduplicate -dynamic -arch x86_64 -macosx_version_min 12.0.0 -syslibroot /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -o a.out /var/folders/gr/13tt3vcd7m1gnbhwtkmf5cnwgn/T/hello-a87934.o -lc++ -lSystem /Users/ilg/Library/xPacks/@xpack-dev-tools/clang/15.0.7-3.1/.content/lib/clang/15.0.7/lib/darwin/libclang_rt.osx.a ``` Using the system headers instead of the toolchain headers may have very subtle consequences, sometimes leading to compile errors which are hard to diagnose. This fix adds a second check using the folder where the executable is located. >From 7fbc229ee7316d826517480ee7896c91dad941f3 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 31 Oct 2023 17:09:04 +0200 Subject: [PATCH] Add \/../include/c++/v1 to include path On macOS, when clang is invoked via a symlink, since the InstalledDir is where the link is located, the C++ headers are not identified and the default system headers are used. This fix adds a second check using the folder where the executable is located. --- clang/lib/Driver/ToolChains/Darwin.cpp | 13 + 1 file changed, 13 insertions(+) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f28e08d81bf29b4..de55307385966cf 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs( << "\"\n"; } +// Check for the folder where the executable is located, if different. +if (getDriver().getInstalledDir() != getDriver().Dir) { + InstallBin = llvm::StringRef(getDriver().Dir.c_str()); + llvm::sy
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: @Endilll, @AaronBallman, @bjope, @mstorsjo, this is a cleanup of a previous PR (#68091) where you contributed. Could you take a second look? https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: > Only one of `Dir`/`InstalledDir` should be used The current behaviour is: - if `InstalledDir/../include/c++/v1` exists, it is used - otherwise the `MacOSX.sdk/usr/include/c++/v1` is used. In most cases, when invoked via a link, there is no `include` in that folder (see the example above), and the sdk folder is wrongly used. The proposed behaviour is: - if `InstalledDir/../include/c++/v1` exists, it is used - if `Dir/../include/c++/v1` exists, it is used - otherwise the `MacOSX.sdk/usr/include/c++/v1` is used. Only one of them is used. https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: > Is there any reason why we don't just use Dir all the time? That's a good question. There was a long discussion in #68091, and my understanding is that, for compatibility reasons, the current behaviour must be preserved. > What's the difference between those two? `Dir` is the location where the actual executables are installed. `InstalledDir`, when the compiler is invoked via a link, is the location where the link comes from. In my use case, and probably in most use cases, the location where the link comes from is a plain folder, not a hierarchy specific to a toolchain. However, for testing purposes, including several clang tests, such a hierarchy is created, with other headers, and a link is placed there, with the expectation that the compiler will use that environment and not the one where the toolchain is actually installed. There are many places in the source code where something is searched for first in the `InstalledDir`, and, if not found, it is also searched in `Dir`. This patch uses exactly the same strategy for identifying the C++ headers on Darwin. Without the patch, when using links the compiler chooses the SDK headers, which is plainly wrong and damaging. https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: > for your use case, can you stick symlinks in to preserve that toolchain > folder structure relative to the ~/tmp/clang++ symlink? i.e. make that > ~/tmp/bin/clang++ -> > /Users/ilg/Library/xPacks/@xpack-dev-tools/clang/15.0.7-3.1/.content/bin/clang-15 > and add ~/tmp/lib -> > /Users/ilg/Library/xPacks/@xpack-dev-tools/clang/15.0.7-3.1/.content/lib (and > same for include). I'm not sure I understand your proposal, but I'm afraid changing the structure of the symlinks is not possible, it is done automatically by a tool. The example above was just to demonstrate the bug. In the actual use case, which is inspired by the npm use case, in the project there is a folder `xpacks` with symlinks to all dependent binaries, with dual indirections, something like this: ``` ilg@wksi native-cmake-clang13-debug % ls -lAR xpacks total 0 drwxr-xr-x 61 ilg staff 1952 Oct 31 14:42 .bin drwxr-xr-x 3 ilg staff96 Oct 31 14:42 @micro-os-plus drwxr-xr-x 3 ilg staff96 Oct 31 14:42 @xpack-dev-tools xpacks/.bin: total 0 lrwxr-xr-x 1 ilg staff 44 Oct 31 14:42 clang -> ../@xpack-dev-tools/clang/.content/bin/clang lrwxr-xr-x 1 ilg staff 46 Oct 31 14:42 clang++ -> ../@xpack-dev-tools/clang/.content/bin/clang++ lrwxr-xr-x 1 ilg staff 50 Oct 31 14:42 clang-check -> ../@xpack-dev-tools/clang/.content/bin/clang-check lrwxr-xr-x 1 ilg staff 47 Oct 31 14:42 clang-cl -> ../@xpack-dev-tools/clang/.content/bin/clang-cl lrwxr-xr-x 1 ilg staff 48 Oct 31 14:42 clang-cpp -> ../@xpack-dev-tools/clang/.content/bin/clang-cpp lrwxr-xr-x 1 ilg staff 48 Oct 31 14:42 clang-doc -> ../@xpack-dev-tools/clang/.content/bin/clang-doc lrwxr-xr-x 1 ilg staff 51 Oct 31 14:42 clang-format -> ../@xpack-dev-tools/clang/.content/bin/clang-format lrwxr-xr-x 1 ilg staff 60 Oct 31 14:42 clang-offload-bundler -> ../@xpack-dev-tools/clang/.content/bin/clang-offload-bundler lrwxr-xr-x 1 ilg staff 60 Oct 31 14:42 clang-offload-wrapper -> ../@xpack-dev-tools/clang/.content/bin/clang-offload-wrapper lrwxr-xr-x 1 ilg staff 53 Oct 31 14:42 clang-refactor -> ../@xpack-dev-tools/clang/.content/bin/clang-refactor lrwxr-xr-x 1 ilg staff 51 Oct 31 14:42 clang-rename -> ../@xpack-dev-tools/clang/.content/bin/clang-rename lrwxr-xr-x 1 ilg staff 54 Oct 31 14:42 clang-scan-deps -> ../@xpack-dev-tools/clang/.content/bin/clang-scan-deps lrwxr-xr-x 1 ilg staff 49 Oct 31 14:42 clang-tidy -> ../@xpack-dev-tools/clang/.content/bin/clang-tidy lrwxr-xr-x 1 ilg staff 45 Oct 31 14:42 clangd -> ../@xpack-dev-tools/clang/.content/bin/clangd lrwxr-xr-x 1 ilg staff 61 Oct 31 14:42 clangd-xpc-test-client -> ../@xpack-dev-tools/clang/.content/bin/clangd-xpc-test-client lrwxr-xr-x 1 ilg staff 51 Oct 31 14:42 darwin-debug -> ../@xpack-dev-tools/clang/.content/bin/darwin-debug lrwxr-xr-x 1 ilg staff 47 Oct 31 14:42 diagtool -> ../@xpack-dev-tools/clang/.content/bin/diagtool lrwxr-xr-x 1 ilg staff 55 Oct 31 14:42 git-clang-format -> ../@xpack-dev-tools/clang/.content/bin/git-clang-format lrwxr-xr-x 1 ilg staff 47 Oct 31 14:42 hmaptool -> ../@xpack-dev-tools/clang/.content/bin/hmaptool lrwxr-xr-x 1 ilg staff 45 Oct 31 14:42 ld.lld -> ../@xpack-dev-tools/clang/.content/bin/ld.lld lrwxr-xr-x 1 ilg staff 47 Oct 31 14:42 ld64.lld -> ../@xpack-dev-tools/clang/.content/bin/ld64.lld lrwxr-xr-x 1 ilg staff 57 Oct 31 14:42 ld64.lld.darwinnew -> ../@xpack-dev-tools/clang/.content/bin/ld64.lld.darwinnew lrwxr-xr-x 1 ilg staff 42 Oct 31 14:42 lld -> ../@xpack-dev-tools/clang/.content/bin/lld lrwxr-xr-x 1 ilg staff 47 Oct 31 14:42 lld-link -> ../@xpack-dev-tools/clang/.content/bin/lld-link lrwxr-xr-x 1 ilg staff 43 Oct 31 14:42 lldb -> ../@xpack-dev-tools/clang/.content/bin/lldb lrwxr-xr-x 1 ilg staff 53 Oct 31 14:42 lldb-argdumper -> ../@xpack-dev-tools/clang/.content/bin/lldb-argdumper lrwxr-xr-x 1 ilg staff 49 Oct 31 14:42 lldb-instr -> ../@xpack-dev-tools/clang/.content/bin/lldb-instr lrwxr-xr-x 1 ilg staff 50 Oct 31 14:42 lldb-server -> ../@xpack-dev-tools/clang/.content/bin/lldb-server lrwxr-xr-x 1 ilg staff 50 Oct 31 14:42 lldb-vscode -> ../@xpack-dev-tools/clang/.content/bin/lldb-vscode lrwxr-xr-x 1 ilg staff 53 Oct 31 14:42 llvm-addr2line -> ../@xpack-dev-tools/clang/.content/bin/llvm-addr2line lrwxr-xr-x 1 ilg staff 46 Oct 31 14:42 llvm-ar -> ../@xpack-dev-tools/clang/.content/bin/llvm-ar lrwxr-xr-x 1 ilg staff 46 Oct 31 14:42 llvm-as -> ../@xpack-dev-tools/clang/.content/bin/llvm-as lrwxr-xr-x 1 ilg staff 57 Oct 31 14:42 llvm-bitcode-strip -> ../@xpack-dev-tools/clang/.content/bin/llvm-bitcode-strip lrwxr-xr-x 1 ilg staff 50 Oct 31 14:42 llvm-config -> ../@xpack-dev-tools/clang/.content/bin/llvm-config lrwxr-xr-x 1 ilg staff 47 Oct 31 14:42 llvm-cov -> ../@xpack-dev-tools/clang/.content/bin/llvm-cov lrwxr-xr-x 1 ilg staff 51 Oct 31 14:42 llvm-cxxdump -> ../@xpack-dev-tools/clang/.content/bin/llvm-cxxdump lrwxr-xr-x 1 ilg
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: Can you clearly state why you are against this PR? You don't consider the current behaviour of silently using the wrong headers a bug, or the proposed solution is not correct? https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Add \/../include/c++/v1 to include path on Darwin (PR #70817)
ilg-ul wrote: Then can this bug fix be accepted? Are there any other solutions? https://github.com/llvm/llvm-project/pull/70817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Match -isysroot behaviour with system compiler on Darwin (PR #80524)
ilg-ul wrote: Sorry, the discussion went too specific for me :-( But after taking a quick look, my impression is that this change reverses the bug fixed in https://github.com/llvm/llvm-project/pull/70817. To recap, that PR preferred the C++ headers available in the toolchain distribution over the SDK headers, when the compiler was launched via a symbolic link to the executable (a behaviour common to the npm/xpm ecosystem). If I understand this proposal right (please correct me if I'm wrong), it will move the SDK headers to the top of the list. If this will happen only when an explicit `-isysroot` is passed on the compiler command line, it might be ok, but if it will happen for all cases, this will simply bring us back to the pre 70817 case, when builds on old macOS-es (like 10.13) with a new clang (like 17.x) will fail, due to the out of sync old headers from the SDK with the new library from the toolchain. I wasted quite a lot of time to diagnose this subtle issue by that time. https://github.com/llvm/llvm-project/pull/80524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Match -isysroot behaviour with system compiler on Darwin (PR #80524)
ilg-ul wrote: I did a test with this patch, and, as expected, the resulting behaviour is wrong. The verbose build of a hello-world test resulted in: ``` [/Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/bin/clang++ simple-hello.cpp -o simple-hello-cpp-one -v -v] xPack x86_64 clang version 18.1.0rc Target: x86_64-apple-darwin23.2.0 Thread model: posix InstalledDir: /Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/bin "/Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/bin/clang-18" -cc1 -triple x86_64-apple-macosx10.13.0 -Wundef-prefix=TARGET_OS_ -Werror=undef-prefix -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -emit-obj -mrelax-all -dumpdir simple-hello-cpp-one- -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name simple-hello.cpp -mrelocation-model pic -pic-level 2 -mframe-pointer=all -ffp-contract=on -fno-rounding-math -funwind-tables=2 -fcompatibility-qualified-id-block-type-checking -fvisibility-inlines-hidden-static-local-var -fbuiltin-headers-in-system-modules -fdefine-target-os-macros -target-cpu penryn -tune-cpu generic -debugger-tuning=lldb -fdebug-compilation-dir=/Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/x86_64-apple-darwin23.2.0/tests/clang/c-cpp -target-linker-version 1022.1 -v -v -fcoverage-compilation-dir=/Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/x86_64-apple-darwin23.2.0/tests/clang/c-cpp -resource-dir /Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/lib/clang/18 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -internal-isystem /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1 -internal-isystem /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/local/include -internal-isystem /Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/lib/clang/18/include -internal-externc-isystem /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include -fdeprecated-macro -ferror-limit 19 -stack-protector 1 -fblocks -fencode-extended-block-signature -fregister-global-dtors-with-atexit -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fcxx-exceptions -fexceptions -fmax-type-align=16 -D__GCC_HAVE_DWARF2_CFI_ASM=1 -o /var/folders/gr/13tt3vcd7m1gnbhwtkmf5cnwgn/T/simple-hello-6b5d6d.o -x c++ simple-hello.cpp clang -cc1 version 18.1.0rc based upon LLVM 18.1.0rc default target x86_64-apple-darwin23.2.0 ignoring nonexistent directory "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/local/include" ignoring nonexistent directory "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/Library/Frameworks" #include "..." search starts here: #include <...> search starts here: /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1 <--- INCORRECT! /Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/lib/clang/18/include /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks (framework directory) End of search list. "/usr/bin/ld" -demangle -lto_library /Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/lib/libLTO.dylib -no_deduplicate -dynamic -arch x86_64 -platform_version macos 10.13.0 10.13.0 -syslibroot /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -mllvm -enable-linkonceodr-outlining -o simple-hello-cpp-one /var/folders/gr/13tt3vcd7m1gnbhwtkmf5cnwgn/T/simple-hello-6b5d6d.o -lc++ -lSystem /Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/lib/clang/18/lib/darwin/libclang_rt.osx.a ``` The same run with the un-patched source: ``` [/Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/bin/clang++ simple-hello.cpp -o simple-hello-cpp-one -v -v] xPack x86_64 clang version 18.1.0rc Target: x86_64-apple-darwin23.2.0 Thread model: posix InstalledDir: /Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/bin "/Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/bin/clang-18" -cc1 -triple x86_64-apple-macosx10.13.0 -Wundef-prefix=TARGET_OS_ -Werror=undef-prefix -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -emit-obj -mrelax-all -dumpdir simple-hello-cpp-one- -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name simple-hello.cpp -mrelocation-model pic -pic-level 2 -mframe-pointer=all -ffp-contract=on -fno-rounding-math -funwind-tables=2 -fcompatibility-qualified-id-block-type-checking -fvisibility-inlines-hidden-static-local-var -fbuiltin-headers-in-system-modules -fdefine-target-os-macros -target-cpu penryn -tune-cpu generic -debugger-tuning=lldb -fdebug-compilation-dir=/Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/x86_64-apple-darwin23.2.0/tests/cla
[clang] [clang] Match -isysroot behaviour with system compiler on Darwin (PR #80524)
ilg-ul wrote: > I am suspicious of SDKROOT maybe being in the environment. Can you check if > your environment is providing that value under the hood? I checked and there is no SDKROOT in the environment. https://github.com/llvm/llvm-project/pull/80524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Match -isysroot behaviour with system compiler on Darwin (PR #80524)
ilg-ul wrote: If, for any reason, you need the SDK path to be the first in the search list, I suggest you add an extra condition, the explicit presence of the `-isysroot`, since it looks like the `Sysroot` variable may come from other configuration definitions. https://github.com/llvm/llvm-project/pull/80524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Match -isysroot behaviour with system compiler on Darwin (PR #80524)
ilg-ul wrote: Yes, that's why I said that your patch must be elaborated and check for the presence of `-isysroot`, since the internal variable may come from other settings. https://github.com/llvm/llvm-project/pull/80524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Match -isysroot behaviour with system compiler on Darwin (PR #80524)
ilg-ul wrote: > they change the behaviour you introduced in > https://github.com/llvm/llvm-project/pull/70817 when -isysroot is provided. I need to take a closer look, since at first reading I can't evaluate the consequences, especially if this does change the behaviour when -isysroot is **not** provided. And I do not know exactly the use case you are considering. My use case was relatively straightforward, multiple versions of the toolchain are installed in versioned custom folders in user home, and different projects requiring different toolchain versions have symbolic links from the project folder to one of the clang executable. If your change does not affect the above use case and also adds more consistency with Apple clang, it should be fine. https://github.com/llvm/llvm-project/pull/80524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Use platform specific calls to get the executable absolute path (PR #68091)
https://github.com/ilg-ul created https://github.com/llvm/llvm-project/pull/68091 This patch fixes #66704. >From f3812174546270051c4a2903b9a99408bf5b7ba0 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:07:48 +0300 Subject: [PATCH 1/2] [clang][driver] Use platform specific calls to get the executable absolute path (#66704) In clang/tools/driver/driver.cpp, the function SetInstallDir() tries to determine the location where the toolchain is installed, basically by taking the parent folder of the executable absolute path. This is not correct when the compiler is invoked via a link, since it returns the parent of the link. This leads to subtle errors, for example on macOS it silently picks the wrong system headers. --- clang/tools/driver/driver.cpp | 61 +++ 1 file changed, 61 insertions(+) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 531b5b4a61c1804..c8ad167cba0a423 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -57,6 +57,15 @@ using namespace clang; using namespace clang::driver; using namespace llvm::opt; +#if defined(__linux__) +#include +#elif defined(__APPLE__) +#include +#include +#elif defined(__MINGW32__) +#include +#endif + std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) { if (!CanonicalPrefixes) { SmallString<128> ExecutablePath(Argv0); @@ -331,6 +340,56 @@ static void SetInstallDir(SmallVectorImpl &argv, // path being a symlink. SmallString<128> InstalledPath(argv[0]); +#if defined(__linux__) + + char ProcessAbsolutePath[PATH_MAX]; + + int len = readlink("/proc/self/exe", ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__APPLE__) + + // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call + // fails with ENOMEM (12) - Cannot allocate memory. + // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + + int len = proc_pidpath(getpid(), ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: proc_pidpath() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__MINGW32__) + + char ProcessAbsolutePath[PATH_MAX]; + + len = GetModuleFileName(NULL, ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: GetModuleFileName() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#else + // Do a PATH lookup, if there are no directory components. if (llvm::sys::path::filename(InstalledPath) == InstalledPath) if (llvm::ErrorOr Tmp = llvm::sys::findProgramByName( @@ -341,6 +400,8 @@ static void SetInstallDir(SmallVectorImpl &argv, if (CanonicalPrefixes) llvm::sys::fs::make_absolute(InstalledPath); +#endif + StringRef InstalledPathParent(llvm::sys::path::parent_path(InstalledPath)); if (llvm::sys::fs::exists(InstalledPathParent)) TheDriver.setInstalledDir(InstalledPathParent); >From eb7b122d5e6ce160d3f0880aac44a2df720d182f Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:16:30 +0300 Subject: [PATCH 2/2] clang-format driver.cpp patch --- clang/tools/driver/driver.cpp | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index c8ad167cba0a423..45e1469aafca95e 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -346,7 +346,7 @@ static void SetInstallDir(SmallVectorImpl &argv, int len = readlink("/proc/self/exe", ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) { llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " << strerror(errno) << "\n"; exit(1); @@ -360,11 +360,11 @@ static void SetInstallDir(SmallVectorImpl &argv, // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call // fails with ENOMEM (12) - Cannot allocate memory. // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c - char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE + 1]; int len = proc_pidpath(getpid(), ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) {
[clang] [clang][driver] Use platform specific calls to get the executable absolute path (PR #68091)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Use platform specific calls to get the executable absolute path (PR #68091)
@@ -331,6 +340,56 @@ static void SetInstallDir(SmallVectorImpl &argv, // path being a symlink. SmallString<128> InstalledPath(argv[0]); +#if defined(__linux__) ilg-ul wrote: > Can we use `getMainExecutable()` instead? Good point! > So is the problem here that SetInstallDir is called after the above (having > constructed TheDriver). Resulting in InstalledDir being changed into being > relative to the symlink? When I first encountered this problem I tried to run a debug session to understand how things work, but got confused and in the end I patched `SetInstallDir` and for my needs it was ok, but since `getMainExecutable()` already does the job I'll reconsider and try to understand where the problem comes from. https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Use platform specific calls to get the executable absolute path (PR #68091)
ilg-ul wrote: > I haven't checked closely Hi Martin, please check the #66704 bug report, the current behaviour is plainly wrong, clang does not pick the correct libraries and silently uses the system libraries, which leads to very subtle and hard to debug errors if the new version is several versions apart from the system. I'm open to any suggestions to make clang correctly identify the InstalledDir when invoked via a symbolic link. https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Use platform specific calls to get the executable absolute path (PR #68091)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/68091 >From f3812174546270051c4a2903b9a99408bf5b7ba0 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:07:48 +0300 Subject: [PATCH 1/3] [clang][driver] Use platform specific calls to get the executable absolute path (#66704) In clang/tools/driver/driver.cpp, the function SetInstallDir() tries to determine the location where the toolchain is installed, basically by taking the parent folder of the executable absolute path. This is not correct when the compiler is invoked via a link, since it returns the parent of the link. This leads to subtle errors, for example on macOS it silently picks the wrong system headers. --- clang/tools/driver/driver.cpp | 61 +++ 1 file changed, 61 insertions(+) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 531b5b4a61c1804..c8ad167cba0a423 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -57,6 +57,15 @@ using namespace clang; using namespace clang::driver; using namespace llvm::opt; +#if defined(__linux__) +#include +#elif defined(__APPLE__) +#include +#include +#elif defined(__MINGW32__) +#include +#endif + std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) { if (!CanonicalPrefixes) { SmallString<128> ExecutablePath(Argv0); @@ -331,6 +340,56 @@ static void SetInstallDir(SmallVectorImpl &argv, // path being a symlink. SmallString<128> InstalledPath(argv[0]); +#if defined(__linux__) + + char ProcessAbsolutePath[PATH_MAX]; + + int len = readlink("/proc/self/exe", ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__APPLE__) + + // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call + // fails with ENOMEM (12) - Cannot allocate memory. + // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + + int len = proc_pidpath(getpid(), ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: proc_pidpath() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__MINGW32__) + + char ProcessAbsolutePath[PATH_MAX]; + + len = GetModuleFileName(NULL, ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: GetModuleFileName() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#else + // Do a PATH lookup, if there are no directory components. if (llvm::sys::path::filename(InstalledPath) == InstalledPath) if (llvm::ErrorOr Tmp = llvm::sys::findProgramByName( @@ -341,6 +400,8 @@ static void SetInstallDir(SmallVectorImpl &argv, if (CanonicalPrefixes) llvm::sys::fs::make_absolute(InstalledPath); +#endif + StringRef InstalledPathParent(llvm::sys::path::parent_path(InstalledPath)); if (llvm::sys::fs::exists(InstalledPathParent)) TheDriver.setInstalledDir(InstalledPathParent); >From eb7b122d5e6ce160d3f0880aac44a2df720d182f Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:16:30 +0300 Subject: [PATCH 2/3] clang-format driver.cpp patch --- clang/tools/driver/driver.cpp | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index c8ad167cba0a423..45e1469aafca95e 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -346,7 +346,7 @@ static void SetInstallDir(SmallVectorImpl &argv, int len = readlink("/proc/self/exe", ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) { llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " << strerror(errno) << "\n"; exit(1); @@ -360,11 +360,11 @@ static void SetInstallDir(SmallVectorImpl &argv, // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call // fails with ENOMEM (12) - Cannot allocate memory. // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c - char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE + 1]; int len = proc_pidpath(getpid(), ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) { llvm::errs() << "Inte
[clang] [clang][driver] Use platform specific calls to get the executable absolute path (PR #68091)
ilg-ul wrote: > resolving this to the actual clang binary would indeed seem like the right > thing to do. Great! The new simplified patch does exactly this. > it makes me wonder if someone actually is relying on the current behaviour To rephrase your question, you ask if someone is using a configuration with a folder where various custom libraries are and bin folder with a link to the actual clang, and expects for clang to pick the custom libraries instead of the distributed ones? I obviously can't tell, but I would say that the chances are slim. And, in case someone wants such a configuration, the current patch, if `-no-canonical-prefixes` is used, reverts to the previous behaviour, when the symlink is not followed. > whatever the consequences are of using a different install dir is kinda of > irrelevant here Well, given how much time I spent trying to understand some weird errors caused by this, I would not call them irrelevant :-( > there should be a well defined logic for how that's calculated Right, but I would say that the current logic based on the folder where the clang executable is located worked fine till now, and I see no reasons to change it, just that till now there were no use cases with symlinks from different folders (a configuration specific to the npm/xpm ecosystem). https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Use platform specific calls to get the executable absolute path (PR #68091)
ilg-ul wrote: > You make it sound like nobody else might ever have used such symlinks before > - isn't that quite a big assumption? Ah, sorry for the confusion, I did not intend to make it sound like a big assumption. I'm convinced that such links were occasionally used before, just that the result was not necessarily an error, I would say that in most cases using the system libraries is functional, and this explains why such cases were not reported till now. However, in the npm/xpm ecosystem, by design, instead of adding lots of paths to the PATH, the installer add symlinks to a local .bin folder, and all binaries are invoked via these links (on windows things are more complicated, `.cmd` shims are used). This is why this issue hit me to the point of searching for a fix. And to make things worse, the xPack build environment (used to build all my binary tools) uses the latest compilers, but runs them on slightly older build machines (for example clang 16 on macOS 10.13), in other words the distance between the two clang versions is large, and builds fail with the system libraries, although clang 16 has much newer libraries. So, I don't want to minimise others experiences, I just wanted to explain how I got into this. > I agree that the current interpretation of symlinks here does seem weird and > that it probably would be right to change it. Ok, thank you. https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Use platform specific calls to get the executable absolute path (PR #68091)
ilg-ul wrote: I took some time to analyse the failed tests. I managed to reproduce them (except `rocm-detect.hip`) on my macOS, so I had more freedom to experiment. The failing tests are: - `Clang::Driver/mingw-sysroot.cpp` - `Clang::Driver/no-canonical-prefixes.c` - `Clang::Driver/program-path-priority.c` They all create folders where they place various symlinks to clang, then perform some checks on InstalledDir or other output lines. The question is whether the reliance of these tests on the path where the symlink is located is a feature that must be checked by the tests, or As Martin noticed, without a clear definition of the correct way to handle these symlinks, this is a tricky issue. Without any other reference, I checked the Apple clang, on my macOS: ``` ilg@wksi ~ % /usr/bin/clang -v Apple clang version 14.0.0 (clang-1400.0.29.202) Target: x86_64-apple-darwin21.6.0 Thread model: posix InstalledDir: /Library/Developer/CommandLineTools/usr/bin ilg@wksi ~ % mkdir tmp/a ilg@wksi ~ % ln -s /usr/bin/clang tmp/a ilg@wksi ~ % tmp/a/clang -v Apple clang version 14.0.0 (clang-1400.0.29.202) Target: x86_64-apple-darwin21.6.0 Thread model: posix InstalledDir: /Library/Developer/CommandLineTools/usr/bin ``` So Apple decided to **follow the links**, and, in my opinion, this is the less ambiguous and expected behaviour. I think that we first need an agreement from the clang maintainers that this is the correct behaviour, then find a way to fix the current tests, and possibly improve the implementation (for example i'm not sure if `InstalledDir` should be affected by `-no-canonical-prefixes`). https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Use TheDriver.ClangExecutable in SetInstallDir to point to the actual install folder (PR #68091)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Use TheDriver.ClangExecutable in SetInstallDir to point to the actual install folder (PR #68091)
ilg-ul wrote: @jansvoboda11, since the Apple clang already fixed this bug, could you suggest how to proceed? Could you backport the Apple patch to upstream? Please also take a look at the original bug report #66704, since it is related to macOS. https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Use TheDriver.ClangExecutable in SetInstallDir to point to the actual install folder (PR #68091)
ilg-ul wrote: Another way to rephrase the question: the current implementation that preserves the symlinks, was it an explicitly required feature, or rather a side effect that came into existence by accident when implementing the current tests? https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Use TheDriver.ClangExecutable in SetInstallDir to point to the actual install folder (PR #68091)
ilg-ul wrote: > it's quite possible that someone has ended up depending on the previous de > facto behaviour. Then we have to be super creative and find a better solution. How about changing the logic, and where `Driver.InstalledDir` is used, if the desired file/folder is not found (like header or library), to search again in `Driver.ClangExecutable`? I don't know how complicated the implementation might be, but, at first sight, it should maintain compatibility with curent use cases that create a full toolchain folder and also prevent silently falling back to system headers/libraries (like in my use case). https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Prevent clang pick the system headers/libraries when started via a symlink (PR #68091)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Prevent clang picking the system headers/libraries when started via a symlink (PR #68091)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Prevent clang picking the system headers/libraries when started via a symlink (PR #68091)
ilg-ul wrote: > I wonder if this would need to be somewhat target specific. I took a first look, and it seems target specific. The strategy was to search for `InstalledDir` and see how it is used. It looks like in many places both locations are used, which means things are not that bad: ```c++ getProgramPaths().push_back(getDriver().getInstalledDir()); if (getDriver().getInstalledDir() != D.Dir) getProgramPaths().push_back(D.Dir); ``` At a closer look, my specific issue is related only to picking the system headers, the libraries seem to be identified correctly. This now clearly explains the errors I encountered, compiling with the system headers and linking with the clang own libraries cannot work if the versions are too far apart. For example the code identifying the C++ headers is in Darwin.cpp, where only `InstalledDir` is checked: ``` // Get from '/bin' to '/include/c++/v1'. // Note that InstallBin can be relative, so we use '..' instead of // parent_path. llvm::SmallString<128> InstallBin = llvm::StringRef(getDriver().getInstalledDir()); // /bin llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1"); if (getVFS().exists(InstallBin)) { addSystemInclude(DriverArgs, CC1Args, InstallBin); return; } else if (DriverArgs.hasArg(options::OPT_v)) { llvm::errs() << "ignoring nonexistent directory \"" << InstallBin << "\"\n"; } ``` I'll extend the code to also check `Dir` and this should fix my problem. I'll take a look at the other targets too, but I'm not sure that I'll be able to provide fixes for all, since I don't know them, and I don't want to mess anything. https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Prevent clang picking the system headers when started via a symlink (PR #68091)
ilg-ul wrote: @nolange, English is not my first language, and I know sometimes I'm very confusing. In my understanding, the title suggest that if clang is not invoked directly but via a symlink, it does not behave correctly, i.e. instead of picking the headers from the folders where clang is installed, it picks the system headers. This is documented with examples in the bug report. If this is not what the title reads, please suggest a better phrasing. https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Prevent clang picking the system headers when started via a symlink (PR #68091)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Prevent clang picking the system headers when started via a symlink (PR #68091)
ilg-ul wrote: > ... I understand this is only a bug on MacOS? I'm using clang only to compile my binary packages on macOS (the Linux binaries are compiled with gcc), so I got bitten by this bug only on macOS. The other platforms/targets may or may not be affected, I don't know; but my initial search for InstalledDir showed multiple references not paired with references to Dir. https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Prevent clang picking the system headers when started via a symlink (PR #68091)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/68091 >From f3812174546270051c4a2903b9a99408bf5b7ba0 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:07:48 +0300 Subject: [PATCH 1/4] [clang][driver] Use platform specific calls to get the executable absolute path (#66704) In clang/tools/driver/driver.cpp, the function SetInstallDir() tries to determine the location where the toolchain is installed, basically by taking the parent folder of the executable absolute path. This is not correct when the compiler is invoked via a link, since it returns the parent of the link. This leads to subtle errors, for example on macOS it silently picks the wrong system headers. --- clang/tools/driver/driver.cpp | 61 +++ 1 file changed, 61 insertions(+) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 531b5b4a61c1804..c8ad167cba0a423 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -57,6 +57,15 @@ using namespace clang; using namespace clang::driver; using namespace llvm::opt; +#if defined(__linux__) +#include +#elif defined(__APPLE__) +#include +#include +#elif defined(__MINGW32__) +#include +#endif + std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) { if (!CanonicalPrefixes) { SmallString<128> ExecutablePath(Argv0); @@ -331,6 +340,56 @@ static void SetInstallDir(SmallVectorImpl &argv, // path being a symlink. SmallString<128> InstalledPath(argv[0]); +#if defined(__linux__) + + char ProcessAbsolutePath[PATH_MAX]; + + int len = readlink("/proc/self/exe", ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__APPLE__) + + // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call + // fails with ENOMEM (12) - Cannot allocate memory. + // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + + int len = proc_pidpath(getpid(), ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: proc_pidpath() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__MINGW32__) + + char ProcessAbsolutePath[PATH_MAX]; + + len = GetModuleFileName(NULL, ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: GetModuleFileName() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#else + // Do a PATH lookup, if there are no directory components. if (llvm::sys::path::filename(InstalledPath) == InstalledPath) if (llvm::ErrorOr Tmp = llvm::sys::findProgramByName( @@ -341,6 +400,8 @@ static void SetInstallDir(SmallVectorImpl &argv, if (CanonicalPrefixes) llvm::sys::fs::make_absolute(InstalledPath); +#endif + StringRef InstalledPathParent(llvm::sys::path::parent_path(InstalledPath)); if (llvm::sys::fs::exists(InstalledPathParent)) TheDriver.setInstalledDir(InstalledPathParent); >From eb7b122d5e6ce160d3f0880aac44a2df720d182f Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:16:30 +0300 Subject: [PATCH 2/4] clang-format driver.cpp patch --- clang/tools/driver/driver.cpp | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index c8ad167cba0a423..45e1469aafca95e 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -346,7 +346,7 @@ static void SetInstallDir(SmallVectorImpl &argv, int len = readlink("/proc/self/exe", ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) { llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " << strerror(errno) << "\n"; exit(1); @@ -360,11 +360,11 @@ static void SetInstallDir(SmallVectorImpl &argv, // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call // fails with ENOMEM (12) - Cannot allocate memory. // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c - char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE + 1]; int len = proc_pidpath(getpid(), ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) { llvm::errs() << "Inte
[clang] [clang][driver] Prevent clang picking the system headers when started via a symlink (PR #68091)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/68091 >From f3812174546270051c4a2903b9a99408bf5b7ba0 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:07:48 +0300 Subject: [PATCH 1/5] [clang][driver] Use platform specific calls to get the executable absolute path (#66704) In clang/tools/driver/driver.cpp, the function SetInstallDir() tries to determine the location where the toolchain is installed, basically by taking the parent folder of the executable absolute path. This is not correct when the compiler is invoked via a link, since it returns the parent of the link. This leads to subtle errors, for example on macOS it silently picks the wrong system headers. --- clang/tools/driver/driver.cpp | 61 +++ 1 file changed, 61 insertions(+) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 531b5b4a61c1804..c8ad167cba0a423 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -57,6 +57,15 @@ using namespace clang; using namespace clang::driver; using namespace llvm::opt; +#if defined(__linux__) +#include +#elif defined(__APPLE__) +#include +#include +#elif defined(__MINGW32__) +#include +#endif + std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) { if (!CanonicalPrefixes) { SmallString<128> ExecutablePath(Argv0); @@ -331,6 +340,56 @@ static void SetInstallDir(SmallVectorImpl &argv, // path being a symlink. SmallString<128> InstalledPath(argv[0]); +#if defined(__linux__) + + char ProcessAbsolutePath[PATH_MAX]; + + int len = readlink("/proc/self/exe", ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__APPLE__) + + // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call + // fails with ENOMEM (12) - Cannot allocate memory. + // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + + int len = proc_pidpath(getpid(), ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: proc_pidpath() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__MINGW32__) + + char ProcessAbsolutePath[PATH_MAX]; + + len = GetModuleFileName(NULL, ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: GetModuleFileName() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#else + // Do a PATH lookup, if there are no directory components. if (llvm::sys::path::filename(InstalledPath) == InstalledPath) if (llvm::ErrorOr Tmp = llvm::sys::findProgramByName( @@ -341,6 +400,8 @@ static void SetInstallDir(SmallVectorImpl &argv, if (CanonicalPrefixes) llvm::sys::fs::make_absolute(InstalledPath); +#endif + StringRef InstalledPathParent(llvm::sys::path::parent_path(InstalledPath)); if (llvm::sys::fs::exists(InstalledPathParent)) TheDriver.setInstalledDir(InstalledPathParent); >From eb7b122d5e6ce160d3f0880aac44a2df720d182f Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:16:30 +0300 Subject: [PATCH 2/5] clang-format driver.cpp patch --- clang/tools/driver/driver.cpp | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index c8ad167cba0a423..45e1469aafca95e 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -346,7 +346,7 @@ static void SetInstallDir(SmallVectorImpl &argv, int len = readlink("/proc/self/exe", ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) { llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " << strerror(errno) << "\n"; exit(1); @@ -360,11 +360,11 @@ static void SetInstallDir(SmallVectorImpl &argv, // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call // fails with ENOMEM (12) - Cannot allocate memory. // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c - char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE + 1]; int len = proc_pidpath(getpid(), ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) { llvm::errs() << "Inte
[clang] [clang][driver] Prevent clang picking the system headers when started via a symlink (PR #68091)
https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/68091 >From f3812174546270051c4a2903b9a99408bf5b7ba0 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:07:48 +0300 Subject: [PATCH 1/6] [clang][driver] Use platform specific calls to get the executable absolute path (#66704) In clang/tools/driver/driver.cpp, the function SetInstallDir() tries to determine the location where the toolchain is installed, basically by taking the parent folder of the executable absolute path. This is not correct when the compiler is invoked via a link, since it returns the parent of the link. This leads to subtle errors, for example on macOS it silently picks the wrong system headers. --- clang/tools/driver/driver.cpp | 61 +++ 1 file changed, 61 insertions(+) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 531b5b4a61c1804..c8ad167cba0a423 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -57,6 +57,15 @@ using namespace clang; using namespace clang::driver; using namespace llvm::opt; +#if defined(__linux__) +#include +#elif defined(__APPLE__) +#include +#include +#elif defined(__MINGW32__) +#include +#endif + std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) { if (!CanonicalPrefixes) { SmallString<128> ExecutablePath(Argv0); @@ -331,6 +340,56 @@ static void SetInstallDir(SmallVectorImpl &argv, // path being a symlink. SmallString<128> InstalledPath(argv[0]); +#if defined(__linux__) + + char ProcessAbsolutePath[PATH_MAX]; + + int len = readlink("/proc/self/exe", ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__APPLE__) + + // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call + // fails with ENOMEM (12) - Cannot allocate memory. + // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + + int len = proc_pidpath(getpid(), ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: proc_pidpath() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#elif defined(__MINGW32__) + + char ProcessAbsolutePath[PATH_MAX]; + + len = GetModuleFileName(NULL, ProcessAbsolutePath, + sizeof(ProcessAbsolutePath) - 1); + if ( len <= 0 ) { +llvm::errs() << "Internal error: GetModuleFileName() failed with " + << strerror(errno) << "\n"; +exit(1); + } + + ProcessAbsolutePath[len] = 0; + InstalledPath = ProcessAbsolutePath; + +#else + // Do a PATH lookup, if there are no directory components. if (llvm::sys::path::filename(InstalledPath) == InstalledPath) if (llvm::ErrorOr Tmp = llvm::sys::findProgramByName( @@ -341,6 +400,8 @@ static void SetInstallDir(SmallVectorImpl &argv, if (CanonicalPrefixes) llvm::sys::fs::make_absolute(InstalledPath); +#endif + StringRef InstalledPathParent(llvm::sys::path::parent_path(InstalledPath)); if (llvm::sys::fs::exists(InstalledPathParent)) TheDriver.setInstalledDir(InstalledPathParent); >From eb7b122d5e6ce160d3f0880aac44a2df720d182f Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Tue, 3 Oct 2023 14:16:30 +0300 Subject: [PATCH 2/6] clang-format driver.cpp patch --- clang/tools/driver/driver.cpp | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index c8ad167cba0a423..45e1469aafca95e 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -346,7 +346,7 @@ static void SetInstallDir(SmallVectorImpl &argv, int len = readlink("/proc/self/exe", ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) { llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with " << strerror(errno) << "\n"; exit(1); @@ -360,11 +360,11 @@ static void SetInstallDir(SmallVectorImpl &argv, // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call // fails with ENOMEM (12) - Cannot allocate memory. // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c - char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1]; + char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE + 1]; int len = proc_pidpath(getpid(), ProcessAbsolutePath, sizeof(ProcessAbsolutePath) - 1); - if ( len <= 0 ) { + if (len <= 0) { llvm::errs() << "Inte
[clang] [clang][driver] Fix an issue where clang does not correctly resolve the system header if invoked via symlink (on MacOS) (PR #68091)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix an issue where clang does not correctly resolve the system header if invoked via symlink (on MacOS) (PR #68091)
ilg-ul wrote: @nolange, thank you for rephrasing the title. https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix an issue where clang does not correctly resolve the system header if invoked via symlink (on MacOS) (PR #68091)
ilg-ul wrote: The latest patch fixes the issue #66704 without changing the meaning of InstalledDir. It is specific to macOS headers, the libraries were ok. I also checked the other places where `installedDir` is used, but I'm not familiar with them (`Hexagon.cpp`, `MipsLinux.cpp`, `OHOS.cpp`, `WebAssembly.cpp`). There are also several references in `MinGW.cpp`, but on Windows starting clang via a link is highly unlikely, so I'll leave them to Martin, to decide if they need any changes. One other file where checking for the path where the executable is located might be in `Gnu.cpp`, around line 2132, where the list of prefixes is calculated, but I don't know the logic well enough to suggest a change: ```cpp // Then look for gcc installed alongside clang. Prefixes.push_back(D.InstalledDir + "/.."); ``` > in principle I kinda agree that the fix itself, locating things based on the > actual executable, sounds like the right thing to do in any case. We're early > in the 18.x cycle, and if we could get the tests fixed to work despite that, > we'd have lots of time to see if someone is affected negatively by it, before > 18.x is released. Fully agree, but I suggest we first fix this specific issue in this PR, and do the rest in a separate PR. https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix an issue where clang does not correctly resolve the system header if invoked via symlink (on MacOS) (PR #68091)
ilg-ul wrote: The Linux tests were fine, but the Windows tests failed in `Interpreter/const.cpp`, which does not seem to be related to my patch. https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix an issue where clang does not correctly resolve the system header if invoked via symlink (on MacOS) (PR #68091)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix an issue where clang does not correctly resolve the system header if invoked via symlink (on MacOS) (PR #68091)
https://github.com/ilg-ul edited https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix an issue where clang does not correctly resolve the system header if invoked via symlink (on MacOS) (PR #68091)
ilg-ul wrote: Any suggestions how to proceed from here? https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix an issue where clang does not correctly resolve the system header if invoked via symlink (on MacOS) (PR #68091)
ilg-ul wrote: @jansvoboda11, as Martin showed before, Apple clang did not fix this, so this patch applies to it too. https://github.com/llvm/llvm-project/pull/68091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits