[clang] [Driver] Fix detection of libc++ with empty sysroot. (PR #66947)

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 22 04:40:15 PDT 2023


https://github.com/sam-mccall updated https://github.com/llvm/llvm-project/pull/66947

>From 00ef242c40dd453157ee3b31e367fd6240d68a5a Mon Sep 17 00:00:00 2001
From: Sam McCall <sam.mccall at gmail.com>
Date: Wed, 20 Sep 2023 21:19:03 +0200
Subject: [PATCH 1/3] [Driver] Fix detection of libc++ with empty sysroot.

b1e3cd1d79443603dc003441e07cdd8d30bb7f26 dropped the leading slash here,
presumably unintentionally.

(I don't understand Driver well enough to know how/where this is supposed
to be tested, but it broke clangd on any project that uses the
system-installed -stdlib=libc++ on a standard debian install)
---
 clang/lib/Driver/ToolChains/Gnu.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index e909549d20708fe..f2fd85021a777b4 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -3139,11 +3139,11 @@ Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
   // not be found at ../include/c++ but it likely to be found at
   // one of the following two locations:
   SmallString<128> UsrLocalIncludeDir(SysRoot);
-  llvm::sys::path::append(UsrLocalIncludeDir, "usr", "local", "include");
+  llvm::sys::path::append(UsrLocalIncludeDir, "/usr", "local", "include");
   if (AddIncludePath(UsrLocalIncludeDir))
     return;
   SmallString<128> UsrIncludeDir(SysRoot);
-  llvm::sys::path::append(UsrIncludeDir, "usr", "include");
+  llvm::sys::path::append(UsrIncludeDir, "/usr", "include");
   if (AddIncludePath(UsrIncludeDir))
     return;
 }

>From a7f9a86ac30bcb93b255e9dd6f4b1f27bb99c502 Mon Sep 17 00:00:00 2001
From: Sam McCall <sam.mccall at gmail.com>
Date: Fri, 22 Sep 2023 12:03:39 +0200
Subject: [PATCH 2/3] use / locally for default sysroot, add test

---
 clang/lib/Driver/ToolChains/Gnu.cpp      |  6 ++--
 clang/unittests/Driver/ToolChainTest.cpp | 46 ++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index f2fd85021a777b4..61f5100f1385a13 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -3100,6 +3100,8 @@ Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
                                    llvm::opt::ArgStringList &CC1Args) const {
   const Driver &D = getDriver();
   std::string SysRoot = computeSysRoot();
+  if (SysRoot.empty())
+    SysRoot = llvm::sys::path::get_separator();
 
   auto AddIncludePath = [&](StringRef Path, bool TargetDirRequired = false) {
     std::string Version = detectLibcxxVersion(Path);
@@ -3139,11 +3141,11 @@ Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
   // not be found at ../include/c++ but it likely to be found at
   // one of the following two locations:
   SmallString<128> UsrLocalIncludeDir(SysRoot);
-  llvm::sys::path::append(UsrLocalIncludeDir, "/usr", "local", "include");
+  llvm::sys::path::append(UsrLocalIncludeDir, "usr", "local", "include");
   if (AddIncludePath(UsrLocalIncludeDir))
     return;
   SmallString<128> UsrIncludeDir(SysRoot);
-  llvm::sys::path::append(UsrIncludeDir, "/usr", "include");
+  llvm::sys::path::append(UsrIncludeDir, "usr", "include");
   if (AddIncludePath(UsrIncludeDir))
     return;
 }
diff --git a/clang/unittests/Driver/ToolChainTest.cpp b/clang/unittests/Driver/ToolChainTest.cpp
index ae567abb81a9bb4..85beea4e94ca252 100644
--- a/clang/unittests/Driver/ToolChainTest.cpp
+++ b/clang/unittests/Driver/ToolChainTest.cpp
@@ -19,10 +19,12 @@
 #include "clang/Driver/Driver.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <memory>
 
@@ -316,6 +318,50 @@ TEST(ToolChainTest, VFSSolarisMultiGCCInstallation) {
   }
 }
 
+MATCHER_P(jobHasArgs, Substr, "") {
+  const driver::Command &C = arg;
+  std::string Args = "";
+  llvm::ListSeparator Sep(" ");
+  for (const char *Arg : C.getArguments()) {
+    Args += Sep;
+    Args += Arg;
+  }
+  if (llvm::StringRef(Args).contains(Substr))
+    return true;
+  *result_listener << "whose args are '" << Args << "'";
+  return false;
+}
+
+TEST(ToolChainTest, VFSGnuLibcxxPathNoSysroot) {
+  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+
+  IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
+  struct TestDiagnosticConsumer : public DiagnosticConsumer {};
+  IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
+      new llvm::vfs::InMemoryFileSystem);
+
+  const char *EmptyFiles[] = {
+      "foo.cpp",
+      "/bin/clang",
+      "/usr/include/c++/v1/cstdio",
+  };
+
+  for (const char *Path : EmptyFiles)
+    InMemoryFileSystem->addFile(Path, 0,
+                                llvm::MemoryBuffer::getMemBuffer("\n"));
+
+  {
+    DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+    Driver TheDriver("/bin/clang", "x86_64-unknown-linux-gnu", Diags,
+                     "clang LLVM compiler", InMemoryFileSystem);
+    std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(
+        {"/bin/clang", "-fsyntax-only", "-stdlib=libc++", "foo.cpp"}));
+    ASSERT_TRUE(C);
+    EXPECT_THAT(C->getJobs(), testing::ElementsAre(jobHasArgs(
+                                  "-internal-isystem /usr/include/c++/v1")));
+  }
+}
+
 TEST(ToolChainTest, DefaultDriverMode) {
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
 

>From 0f7f2f05a04b14e55170da058ede74639838600d Mon Sep 17 00:00:00 2001
From: Sam McCall <sam.mccall at gmail.com>
Date: Fri, 22 Sep 2023 13:39:48 +0200
Subject: [PATCH 3/3] fix backslash problem when testing on windows

---
 clang/unittests/Driver/ToolChainTest.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clang/unittests/Driver/ToolChainTest.cpp b/clang/unittests/Driver/ToolChainTest.cpp
index 85beea4e94ca252..731b6c98c67929c 100644
--- a/clang/unittests/Driver/ToolChainTest.cpp
+++ b/clang/unittests/Driver/ToolChainTest.cpp
@@ -326,6 +326,8 @@ MATCHER_P(jobHasArgs, Substr, "") {
     Args += Sep;
     Args += Arg;
   }
+  if (is_style_windows(llvm::sys::path::Style::native))
+    std::replace(Args.begin(), Args.end(), '\\', '/');
   if (llvm::StringRef(Args).contains(Substr))
     return true;
   *result_listener << "whose args are '" << Args << "'";



More information about the cfe-commits mailing list