From 7a7dc5f15b35a72c2545c00e9d33296e0d1089be Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Tue, 24 May 2022 12:00:50 +0100 Subject: [PATCH] [Clang][Driver] Fix include paths for `--sysroot /` on Linux Currently if `--sysroot /` is passed to the Clang driver, the include paths generated by the Clang driver will start with a double slash: `//usr/include/...`. If VFS is used to inject files into the include paths (for example, the Swift compiler does this), VFS will get confused and the injected files won't be visible. This change makes sure that the include paths start with a single slash. Fixes #28283. Differential Revision: https://reviews.llvm.org/D126289 --- clang/include/clang/Driver/ToolChain.h | 3 ++ clang/lib/Driver/ToolChain.cpp | 8 +++ clang/lib/Driver/ToolChains/Gnu.cpp | 22 ++++----- clang/lib/Driver/ToolChains/Linux.cpp | 49 ++++++++++--------- .../basic_linux_libstdcxx_tree/usr/bin/.keep | 0 .../x86_64-unknown-linux-gnu/4.8/crtbegin.o | 0 clang/test/Driver/linux-header-search.cpp | 31 +++++++++++- 7 files changed, 77 insertions(+), 36 deletions(-) create mode 100644 clang/test/Driver/Inputs/basic_linux_libstdcxx_tree/usr/bin/.keep create mode 100644 clang/test/Driver/Inputs/basic_linux_libstdcxx_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.8/crtbegin.o diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h index 13094aabcdfa7..872a85611356e 100644 --- a/clang/include/clang/Driver/ToolChain.h +++ b/clang/include/clang/Driver/ToolChain.h @@ -213,6 +213,9 @@ class ToolChain { static void addSystemIncludes(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args, ArrayRef Paths); + + static std::string concat(StringRef Path, const Twine &A, const Twine &B = "", + const Twine &C = "", const Twine &D = ""); ///@} public: diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp index d45a90048dcbe..02baa558fbf6f 100644 --- a/clang/lib/Driver/ToolChain.cpp +++ b/clang/lib/Driver/ToolChain.cpp @@ -905,6 +905,14 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs, } } +/*static*/ std::string ToolChain::concat(StringRef Path, const Twine &A, + const Twine &B, const Twine &C, + const Twine &D) { + SmallString<128> Result(Path); + llvm::sys::path::append(Result, llvm::sys::path::Style::posix, A, B, C, D); + return std::string(Result); +} + std::string ToolChain::detectLibcxxVersion(StringRef IncludePath) const { std::error_code EC; int MaxVersion = 0; diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp index 5816b91994102..750b3c386753b 100644 --- a/clang/lib/Driver/ToolChains/Gnu.cpp +++ b/clang/lib/Driver/ToolChains/Gnu.cpp @@ -2021,7 +2021,7 @@ void Generic_GCC::GCCInstallationDetector::init( if (!VFS.exists(Prefix)) continue; for (StringRef Suffix : CandidateLibDirs) { - const std::string LibDir = Prefix + Suffix.str(); + const std::string LibDir = concat(Prefix, Suffix); if (!VFS.exists(LibDir)) continue; // Maybe filter out /gcc and /gcc-cross. @@ -2087,7 +2087,7 @@ void Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes( // so we need to find those /usr/gcc/*/lib/gcc libdirs and go with // /usr/gcc/ as a prefix. - std::string PrefixDir = SysRoot.str() + "/usr/gcc"; + std::string PrefixDir = concat(SysRoot, "/usr/gcc"); std::error_code EC; for (llvm::vfs::directory_iterator LI = D.getVFS().dir_begin(PrefixDir, EC), LE; @@ -2122,7 +2122,7 @@ void Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes( Prefixes.push_back("/opt/rh/devtoolset-3/root/usr"); Prefixes.push_back("/opt/rh/devtoolset-2/root/usr"); } - Prefixes.push_back(SysRoot.str() + "/usr"); + Prefixes.push_back(concat(SysRoot, "/usr")); } /*static*/ void Generic_GCC::GCCInstallationDetector::CollectLibDirsAndTriples( @@ -2645,7 +2645,7 @@ bool Generic_GCC::GCCInstallationDetector::ScanGentooConfigs( const llvm::Triple &TargetTriple, const ArgList &Args, const SmallVectorImpl &CandidateTriples, const SmallVectorImpl &CandidateBiarchTriples) { - if (!D.getVFS().exists(D.SysRoot + GentooConfigDir)) + if (!D.getVFS().exists(concat(D.SysRoot, GentooConfigDir))) return false; for (StringRef CandidateTriple : CandidateTriples) { @@ -2664,8 +2664,8 @@ bool Generic_GCC::GCCInstallationDetector::ScanGentooGccConfig( const llvm::Triple &TargetTriple, const ArgList &Args, StringRef CandidateTriple, bool NeedsBiarchSuffix) { llvm::ErrorOr> File = - D.getVFS().getBufferForFile(D.SysRoot + GentooConfigDir + "/config-" + - CandidateTriple.str()); + D.getVFS().getBufferForFile(concat(D.SysRoot, GentooConfigDir, + "/config-" + CandidateTriple.str())); if (File) { SmallVector Lines; File.get()->getBuffer().split(Lines, "\n"); @@ -2676,8 +2676,8 @@ bool Generic_GCC::GCCInstallationDetector::ScanGentooGccConfig( continue; // Process the config file pointed to by CURRENT. llvm::ErrorOr> ConfigFile = - D.getVFS().getBufferForFile(D.SysRoot + GentooConfigDir + "/" + - Line.str()); + D.getVFS().getBufferForFile( + concat(D.SysRoot, GentooConfigDir, "/" + Line)); std::pair ActiveVersion = Line.rsplit('-'); // List of paths to scan for libraries. SmallVector GentooScanPaths; @@ -2710,7 +2710,7 @@ bool Generic_GCC::GCCInstallationDetector::ScanGentooGccConfig( // Scan all paths for GCC libraries. for (const auto &GentooScanPath : GentooScanPaths) { - std::string GentooPath = D.SysRoot + std::string(GentooScanPath); + std::string GentooPath = concat(D.SysRoot, GentooScanPath); if (D.getVFS().exists(GentooPath + "/crtbegin.o")) { if (!ScanGCCForMultilibs(TargetTriple, Args, GentooPath, NeedsBiarchSuffix)) @@ -3003,9 +3003,9 @@ Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs, // If this is a development, non-installed, clang, libcxx will // not be found at ../include/c++ but it likely to be found at // one of the following two locations: - if (AddIncludePath(SysRoot + "/usr/local/include")) + if (AddIncludePath(concat(SysRoot, "/usr/local/include"))) return; - if (AddIncludePath(SysRoot + "/usr/include")) + if (AddIncludePath(concat(SysRoot, "/usr/include"))) return; } diff --git a/clang/lib/Driver/ToolChains/Linux.cpp b/clang/lib/Driver/ToolChains/Linux.cpp index 6be2e801d8361..926da8fac80cb 100644 --- a/clang/lib/Driver/ToolChains/Linux.cpp +++ b/clang/lib/Driver/ToolChains/Linux.cpp @@ -97,9 +97,9 @@ std::string Linux::getMultiarchTriple(const Driver &D, case llvm::Triple::mips64: { std::string MT = std::string(IsMipsR6 ? "mipsisa64r6" : "mips64") + "-linux-" + (IsMipsN32Abi ? "gnuabin32" : "gnuabi64"); - if (D.getVFS().exists(SysRoot + "/lib/" + MT)) + if (D.getVFS().exists(concat(SysRoot, "/lib", MT))) return MT; - if (D.getVFS().exists(SysRoot + "/lib/mips64-linux-gnu")) + if (D.getVFS().exists(concat(SysRoot, "/lib/mips64-linux-gnu"))) return "mips64-linux-gnu"; break; } @@ -108,14 +108,14 @@ std::string Linux::getMultiarchTriple(const Driver &D, return "mips64el-linux-android"; std::string MT = std::string(IsMipsR6 ? "mipsisa64r6el" : "mips64el") + "-linux-" + (IsMipsN32Abi ? "gnuabin32" : "gnuabi64"); - if (D.getVFS().exists(SysRoot + "/lib/" + MT)) + if (D.getVFS().exists(concat(SysRoot, "/lib", MT))) return MT; - if (D.getVFS().exists(SysRoot + "/lib/mips64el-linux-gnu")) + if (D.getVFS().exists(concat(SysRoot, "/lib/mips64el-linux-gnu"))) return "mips64el-linux-gnu"; break; } case llvm::Triple::ppc: - if (D.getVFS().exists(SysRoot + "/lib/powerpc-linux-gnuspe")) + if (D.getVFS().exists(concat(SysRoot, "/lib/powerpc-linux-gnuspe"))) return "powerpc-linux-gnuspe"; return "powerpc-linux-gnu"; case llvm::Triple::ppcle: @@ -269,13 +269,13 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args) // used. We need add both libo32 and /lib. if (Arch == llvm::Triple::mips || Arch == llvm::Triple::mipsel) { Generic_GCC::AddMultilibPaths(D, SysRoot, "libo32", MultiarchTriple, Paths); - addPathIfExists(D, SysRoot + "/libo32", Paths); - addPathIfExists(D, SysRoot + "/usr/libo32", Paths); + addPathIfExists(D, concat(SysRoot, "/libo32"), Paths); + addPathIfExists(D, concat(SysRoot, "/usr/libo32"), Paths); } Generic_GCC::AddMultilibPaths(D, SysRoot, OSLibDir, MultiarchTriple, Paths); - addPathIfExists(D, SysRoot + "/lib/" + MultiarchTriple, Paths); - addPathIfExists(D, SysRoot + "/lib/../" + OSLibDir, Paths); + addPathIfExists(D, concat(SysRoot, "/lib", MultiarchTriple), Paths); + addPathIfExists(D, concat(SysRoot, "/lib/..", OSLibDir), Paths); if (IsAndroid) { // Android sysroots contain a library directory for each supported OS @@ -283,24 +283,24 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args) // directory. addPathIfExists( D, - SysRoot + "/usr/lib/" + MultiarchTriple + "/" + - llvm::to_string(Triple.getEnvironmentVersion().getMajor()), + concat(SysRoot, "/usr/lib", MultiarchTriple, + llvm::to_string(Triple.getEnvironmentVersion().getMajor())), Paths); } - addPathIfExists(D, SysRoot + "/usr/lib/" + MultiarchTriple, Paths); + addPathIfExists(D, concat(SysRoot, "/usr/lib", MultiarchTriple), Paths); // 64-bit OpenEmbedded sysroots may not have a /usr/lib dir. So they cannot // find /usr/lib64 as it is referenced as /usr/lib/../lib64. So we handle // this here. if (Triple.getVendor() == llvm::Triple::OpenEmbedded && Triple.isArch64Bit()) - addPathIfExists(D, SysRoot + "/usr/" + OSLibDir, Paths); + addPathIfExists(D, concat(SysRoot, "/usr", OSLibDir), Paths); else - addPathIfExists(D, SysRoot + "/usr/lib/../" + OSLibDir, Paths); + addPathIfExists(D, concat(SysRoot, "/usr/lib/..", OSLibDir), Paths); if (IsRISCV) { StringRef ABIName = tools::riscv::getRISCVABI(Args, Triple); - addPathIfExists(D, SysRoot + "/" + OSLibDir + "/" + ABIName, Paths); - addPathIfExists(D, SysRoot + "/usr/" + OSLibDir + "/" + ABIName, Paths); + addPathIfExists(D, concat(SysRoot, "/", OSLibDir, ABIName), Paths); + addPathIfExists(D, concat(SysRoot, "/usr", OSLibDir, ABIName), Paths); } Generic_GCC::AddMultiarchPaths(D, SysRoot, OSLibDir, Paths); @@ -312,8 +312,8 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args) D.getVFS().exists(D.Dir + "/../lib/libc++.so")) addPathIfExists(D, D.Dir + "/../lib", Paths); - addPathIfExists(D, SysRoot + "/lib", Paths); - addPathIfExists(D, SysRoot + "/usr/lib", Paths); + addPathIfExists(D, concat(SysRoot, "/lib"), Paths); + addPathIfExists(D, concat(SysRoot, "/usr/lib"), Paths); } ToolChain::RuntimeLibType Linux::GetDefaultRuntimeLibType() const { @@ -586,7 +586,7 @@ void Linux::AddClangSystemIncludeArgs(const ArgList &DriverArgs, return; // LOCAL_INCLUDE_DIR - addSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/local/include"); + addSystemInclude(DriverArgs, CC1Args, concat(SysRoot, "/usr/local/include")); // TOOL_INCLUDE_DIR AddMultilibIncludeArgs(DriverArgs, CC1Args); @@ -607,9 +607,10 @@ void Linux::AddClangSystemIncludeArgs(const ArgList &DriverArgs, // /usr/include. std::string MultiarchIncludeDir = getMultiarchTriple(D, getTriple(), SysRoot); if (!MultiarchIncludeDir.empty() && - D.getVFS().exists(SysRoot + "/usr/include/" + MultiarchIncludeDir)) - addExternCSystemInclude(DriverArgs, CC1Args, - SysRoot + "/usr/include/" + MultiarchIncludeDir); + D.getVFS().exists(concat(SysRoot, "/usr/include", MultiarchIncludeDir))) + addExternCSystemInclude( + DriverArgs, CC1Args, + concat(SysRoot, "/usr/include", MultiarchIncludeDir)); if (getTriple().getOS() == llvm::Triple::RTEMS) return; @@ -617,9 +618,9 @@ void Linux::AddClangSystemIncludeArgs(const ArgList &DriverArgs, // Add an include of '/include' directly. This isn't provided by default by // system GCCs, but is often used with cross-compiling GCCs, and harmless to // add even when Clang is acting as-if it were a system compiler. - addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include"); + addExternCSystemInclude(DriverArgs, CC1Args, concat(SysRoot, "/include")); - addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include"); + addExternCSystemInclude(DriverArgs, CC1Args, concat(SysRoot, "/usr/include")); if (!DriverArgs.hasArg(options::OPT_nobuiltininc) && getTriple().isMusl()) addSystemInclude(DriverArgs, CC1Args, ResourceDirInclude); diff --git a/clang/test/Driver/Inputs/basic_linux_libstdcxx_tree/usr/bin/.keep b/clang/test/Driver/Inputs/basic_linux_libstdcxx_tree/usr/bin/.keep new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/clang/test/Driver/Inputs/basic_linux_libstdcxx_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.8/crtbegin.o b/clang/test/Driver/Inputs/basic_linux_libstdcxx_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.8/crtbegin.o new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/clang/test/Driver/linux-header-search.cpp b/clang/test/Driver/linux-header-search.cpp index ced20ed0ae853..e95973a180517 100644 --- a/clang/test/Driver/linux-header-search.cpp +++ b/clang/test/Driver/linux-header-search.cpp @@ -16,6 +16,22 @@ // CHECK-BASIC-LIBCXX-SYSROOT: "-internal-isystem" "[[SYSROOT]]/usr/include/x86_64-unknown-linux-gnu/c++/v1" // CHECK-BASIC-LIBCXX-SYSROOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" // CHECK-BASIC-LIBCXX-SYSROOT: "-internal-isystem" "[[SYSROOT]]/usr/local/include" + +// Test include paths when the sysroot path ends with `/`. +// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: --target=x86_64-unknown-linux-gnu \ +// RUN: -stdlib=libc++ \ +// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin \ +// RUN: -resource-dir=%S/Inputs/resource_dir \ +// RUN: --sysroot=%S/Inputs/basic_linux_libcxx_tree/ \ +// RUN: --gcc-toolchain="" \ +// RUN: | FileCheck --check-prefix=CHECK-BASIC-LIBCXX-SYSROOT-SLASH %s +// CHECK-BASIC-LIBCXX-SYSROOT-SLASH: "-cc1" +// CHECK-BASIC-LIBCXX-SYSROOT-SLASH-SAME: "-isysroot" "[[SYSROOT:[^"]+/]]" +// CHECK-BASIC-LIBCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/include/x86_64-unknown-linux-gnu/c++/v1" +// CHECK-BASIC-LIBCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/include/c++/v1" +// CHECK-BASIC-LIBCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/local/include" + // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ // RUN: -target x86_64-unknown-linux-gnu \ // RUN: -stdlib=libc++ \ @@ -56,7 +72,20 @@ // CHECK-BASIC-LIBCXXV2-INSTALL: "-internal-isystem" "[[SYSROOT]]/usr/bin/../include/x86_64-unknown-linux-gnu/c++/v2" // CHECK-BASIC-LIBCXXV2-INSTALL: "-internal-isystem" "[[SYSROOT]]/usr/bin/../include/c++/v2" // CHECK-BASIC-LIBCXXV2-INSTALL: "-internal-isystem" "[[SYSROOT]]/usr/local/include" -// + +// Test Linux with libstdc++ when the sysroot path ends with `/`. +// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: --target=x86_64-unknown-linux-gnu \ +// RUN: -stdlib=libstdc++ \ +// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin \ +// RUN: -resource-dir=%S/Inputs/resource_dir \ +// RUN: --sysroot=%S/Inputs/basic_linux_libstdcxx_tree/ \ +// RUN: --gcc-toolchain="" \ +// RUN: | FileCheck --check-prefix=CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH %s +// CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH: "-cc1" +// CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH-SAME: "-isysroot" "[[SYSROOT:[^"]+/]]" +// CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/lib/gcc/x86_64-unknown-linux-gnu/4.8/../../../../x86_64-unknown-linux-gnu/include" + // Test Linux with both libc++ and libstdc++ installed. // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ // RUN: -target x86_64-unknown-linux-gnu \