egorzhdan created this revision.
Herald added subscribers: pmatos, asb, jgravelle-google, sbc100, dschuff.
Herald added a project: All.
egorzhdan requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, aheejin.
Herald added a project: clang.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126289

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Hurd.h
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/Linux.h
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/lib/Driver/ToolChains/WebAssembly.h
  clang/test/Driver/Inputs/basic_linux_libstdcxx_tree/usr/bin/.keep
  
clang/test/Driver/Inputs/basic_linux_libstdcxx_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.8/crtbegin.o
  clang/test/Driver/linux-header-search.cpp

Index: clang/test/Driver/linux-header-search.cpp
===================================================================
--- clang/test/Driver/linux-header-search.cpp
+++ 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: "-isysroot" "[[SYSROOT:[^"]+/]]"
+// CHECK-BASIC-LIBCXX-SYSROOT-SLASH: "-internal-isystem" "[[SYSROOT]]usr/include/x86_64-unknown-linux-gnu/c++/v1"
+// CHECK-BASIC-LIBCXX-SYSROOT-SLASH: "-internal-isystem" "[[SYSROOT]]usr/include/c++/v1"
+// CHECK-BASIC-LIBCXX-SYSROOT-SLASH: "-internal-isystem" "[[SYSROOT]]usr/local/include"
+//
 // RUN: %clang -### %s -fsyntax-only 2>&1 \
 // RUN:     --target=x86_64-unknown-linux-gnu \
 // RUN:     -stdlib=libc++ \
@@ -56,6 +72,32 @@
 // 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++.
+// 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 %s
+// CHECK-BASIC-LIBSTDCXX-SYSROOT: "-cc1"
+// CHECK-BASIC-LIBSTDCXX-SYSROOT: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-BASIC-LIBSTDCXX-SYSROOT: "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/x86_64-unknown-linux-gnu/4.8/../../../../x86_64-unknown-linux-gnu/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: "-isysroot" "[[SYSROOT:[^"]+/]]"
+// CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH: "-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 -### %s -fsyntax-only 2>&1 \
Index: clang/lib/Driver/ToolChains/WebAssembly.h
===================================================================
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -78,7 +78,7 @@
 
   std::string getMultiarchTriple(const Driver &D,
                                  const llvm::Triple &TargetTriple,
-                                 StringRef SysRoot) const override;
+                                 const std::string &SysRoot) const override;
 
   void addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
                              llvm::opt::ArgStringList &CC1Args) const;
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===================================================================
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -30,7 +30,7 @@
 /// we remove the vendor field to form the multiarch triple.
 std::string WebAssembly::getMultiarchTriple(const Driver &D,
                                             const llvm::Triple &TargetTriple,
-                                            StringRef SysRoot) const {
+                                            const std::string &SysRoot) const {
     return (TargetTriple.getArchName() + "-" +
             TargetTriple.getOSAndEnvironmentName()).str();
 }
Index: clang/lib/Driver/ToolChains/Linux.h
===================================================================
--- clang/lib/Driver/ToolChains/Linux.h
+++ clang/lib/Driver/ToolChains/Linux.h
@@ -25,7 +25,7 @@
 
   std::string getMultiarchTriple(const Driver &D,
                                  const llvm::Triple &TargetTriple,
-                                 StringRef SysRoot) const override;
+                                 const std::string &SysRoot) const override;
 
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
Index: clang/lib/Driver/ToolChains/Linux.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -39,7 +39,7 @@
 /// so we provide a rough mapping here.
 std::string Linux::getMultiarchTriple(const Driver &D,
                                       const llvm::Triple &TargetTriple,
-                                      StringRef SysRoot) const {
+                                      const std::string &SysRoot) const {
   llvm::Triple::EnvironmentType TargetEnvironment =
       TargetTriple.getEnvironment();
   bool IsAndroid = TargetTriple.isAndroid();
@@ -97,9 +97,9 @@
   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 @@
       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 @@
   // 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 @@
     // 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 @@
       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 @@
     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 @@
   // /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 @@
   // 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);
Index: clang/lib/Driver/ToolChains/Hurd.h
===================================================================
--- clang/lib/Driver/ToolChains/Hurd.h
+++ clang/lib/Driver/ToolChains/Hurd.h
@@ -42,7 +42,7 @@
 
   std::string getMultiarchTriple(const Driver &D,
                                  const llvm::Triple &TargetTriple,
-                                 StringRef SysRoot) const override;
+                                 const std::string &SysRoot) const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/Hurd.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -29,7 +29,7 @@
 /// so we provide a rough mapping here.
 std::string Hurd::getMultiarchTriple(const Driver &D,
                                      const llvm::Triple &TargetTriple,
-                                     StringRef SysRoot) const {
+                                     const std::string &SysRoot) const {
   if (TargetTriple.getArch() == llvm::Triple::x86) {
     // We use the existence of '/lib/<triple>' as a directory to detect some
     // common hurd triples that don't quite match the Clang triple for both
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2059,7 +2059,7 @@
     if (!VFS.exists(Prefix))
       continue;
     for (StringRef Suffix : CandidateLibDirs) {
-      const std::string LibDir = Prefix + Suffix.str();
+      const std::string LibDir = concat(Prefix, Suffix.str());
       if (!VFS.exists(LibDir))
         continue;
       // Maybe filter out <libdir>/gcc and <libdir>/gcc-cross.
@@ -2125,7 +2125,7 @@
     // so we need to find those /usr/gcc/*/lib/gcc libdirs and go with
     // /usr/gcc/<version> as a prefix.
 
-    std::string PrefixDir = SysRoot.str() + "/usr/gcc";
+    std::string PrefixDir = concat(SysRoot.str(), "/usr/gcc");
     std::error_code EC;
     for (llvm::vfs::directory_iterator LI = D.getVFS().dir_begin(PrefixDir, EC),
                                        LE;
@@ -2160,7 +2160,7 @@
     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.str(), "/usr"));
 }
 
 /*static*/ void Generic_GCC::GCCInstallationDetector::CollectLibDirsAndTriples(
@@ -2683,7 +2683,7 @@
     const llvm::Triple &TargetTriple, const ArgList &Args,
     const SmallVectorImpl<StringRef> &CandidateTriples,
     const SmallVectorImpl<StringRef> &CandidateBiarchTriples) {
-  if (!D.getVFS().exists(D.SysRoot + GentooConfigDir))
+  if (!D.getVFS().exists(concat(D.SysRoot, GentooConfigDir)))
     return false;
 
   for (StringRef CandidateTriple : CandidateTriples) {
@@ -2702,8 +2702,8 @@
     const llvm::Triple &TargetTriple, const ArgList &Args,
     StringRef CandidateTriple, bool NeedsBiarchSuffix) {
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> File =
-      D.getVFS().getBufferForFile(D.SysRoot + GentooConfigDir + "/config-" +
-                                  CandidateTriple.str());
+      D.getVFS().getBufferForFile(concat(D.SysRoot, GentooConfigDir,
+                                         "/config-" + CandidateTriple.str()));
   if (File) {
     SmallVector<StringRef, 2> Lines;
     File.get()->getBuffer().split(Lines, "\n");
@@ -2714,8 +2714,8 @@
         continue;
       // Process the config file pointed to by CURRENT.
       llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> ConfigFile =
-          D.getVFS().getBufferForFile(D.SysRoot + GentooConfigDir + "/" +
-                                      Line.str());
+          D.getVFS().getBufferForFile(
+              concat(D.SysRoot, GentooConfigDir, "/" + Line.str()));
       std::pair<StringRef, StringRef> ActiveVersion = Line.rsplit('-');
       // List of paths to scan for libraries.
       SmallVector<StringRef, 4> GentooScanPaths;
@@ -2748,7 +2748,7 @@
 
       // 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))
@@ -3041,9 +3041,9 @@
   // 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;
 }
 
Index: clang/lib/Driver/ToolChain.cpp
===================================================================
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -914,6 +914,14 @@
   }
 }
 
+/*static*/ std::string ToolChain::concat(const std::string &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;
Index: clang/include/clang/Driver/ToolChain.h
===================================================================
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -215,6 +215,10 @@
   static void addSystemIncludes(const llvm::opt::ArgList &DriverArgs,
                                 llvm::opt::ArgStringList &CC1Args,
                                 ArrayRef<StringRef> Paths);
+
+  static std::string concat(const std::string &Path, const Twine &A,
+                            const Twine &B = "", const Twine &C = "",
+                            const Twine &D = "");
   ///@}
 
 public:
@@ -562,7 +566,7 @@
 
   virtual std::string getMultiarchTriple(const Driver &D,
                                          const llvm::Triple &TargetTriple,
-                                         StringRef SysRoot) const {
+                                         const std::string &SysRoot) const {
     return TargetTriple.str();
   }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to