[clangd] Preserve -nostdinc and --sysroot when calling query driver

Solves this issue: clangd/clangd#157

This is my first contribution to an llvm project, so I hope I'm doing it right!

Patch by @topisani (Tobias Pisani)!

Reviewers: kadircet, klimek

Differential Revision: https://reviews.llvm.org/D73811

(cherry picked from commit 6e8d6bc)

clang-tools-extra/clangd/QueryDriverDatabase.cpp

@@ -85,9 +85,10 @@ std::vector<std::string> parseDriverOutput(llvm::StringRef Output) {
   return SystemIncludes;
 }
 
-std::vector<std::string> extractSystemIncludes(PathRef Driver,
-                                               llvm::StringRef Lang,
-                                               llvm::Regex &QueryDriverRegex) {
+std::vector<std::string>
+extractSystemIncludes(PathRef Driver, llvm::StringRef Lang,
+                      llvm::ArrayRef<std::string> CommandLine,
+                      llvm::Regex &QueryDriverRegex) {
   trace::Span Tracer("Extract system includes");
   SPAN_ATTACH(Tracer, "driver", Driver);
   SPAN_ATTACH(Tracer, "lang", Lang);
@@ -120,14 +121,43 @@ std::vector<std::string> extractSystemIncludes(PathRef Driver,
   llvm::Optional<llvm::StringRef> Redirects[] = {
       {""}, {""}, llvm::StringRef(StdErrPath)};
 
-  // Should we also preserve flags like "-sysroot", "-nostdinc" ?
-  const llvm::StringRef Args[] = {Driver, "-E", "-x", Lang, "-", "-v"};
+  llvm::SmallVector<llvm::StringRef, 12> Args = {Driver, "-E", "-x",
+                                                 Lang,   "-",  "-v"};
+
+  // These flags will be preserved
+  const llvm::StringRef FlagsToPreserve[] = {
+      "-nostdinc", "--no-standard-includes", "-nostdinc++", "-nobuiltininc"};
+  // Preserves these flags and their values, either as separate args or with an
+  // equalsbetween them
+  const llvm::StringRef ArgsToPreserve[] = {"--sysroot", "-isysroot"};
+
+  for (size_t I = 0, E = CommandLine.size(); I < E; ++I) {
+    llvm::StringRef Arg = CommandLine[I];
+    if (llvm::any_of(FlagsToPreserve,
+                     [&Arg](llvm::StringRef S) { return S == Arg; })) {
+      Args.push_back(Arg);
+    } else {
+      const auto *Found =
+          llvm::find_if(ArgsToPreserve, [&Arg](llvm::StringRef S) {
+            return Arg.startswith(S);
+          });
+      if (Found == std::end(ArgsToPreserve))
+        continue;
+      Arg.consume_front(*Found);
+      if (Arg.empty() && I + 1 < E) {
+        Args.push_back(CommandLine[I]);
+        Args.push_back(CommandLine[++I]);
+      } else if (Arg.startswith("=")) {
+        Args.push_back(CommandLine[I]);
+      }
+    }
+  }
 
   if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None,
                                          Redirects)) {
     elog("System include extraction: driver execution failed with return code: "
-         "{0}",
-         llvm::to_string(RC));
+         "{0}. Args: ['{1}']",
+         llvm::to_string(RC), llvm::join(Args, "', '"));
     return {};
   }
 
@@ -237,7 +267,7 @@ class QueryDriverDatabase : public GlobalCompilationDatabase {
 
     llvm::SmallString<128> Driver(Cmd->CommandLine.front());
     llvm::sys::fs::make_absolute(Cmd->Directory, Driver);
-    auto Key = std::make_pair(Driver.str(), Lang);
+    auto Key = std::make_pair(Driver.str().str(), Lang.str());
 
     std::vector<std::string> SystemIncludes;
     {
@@ -247,8 +277,8 @@ class QueryDriverDatabase : public GlobalCompilationDatabase {
       if (It != DriverToIncludesCache.end())
         SystemIncludes = It->second;
       else
-        DriverToIncludesCache[Key] = SystemIncludes =
-            extractSystemIncludes(Key.first, Key.second, QueryDriverRegex);
+        DriverToIncludesCache[Key] = SystemIncludes = extractSystemIncludes(
+            Key.first, Key.second, Cmd->CommandLine, QueryDriverRegex);
     }
 
     return addSystemIncludes(*Cmd, SystemIncludes);
@@ -278,7 +308,7 @@ getQueryDriverDatabase(llvm::ArrayRef<std::string> QueryDriverGlobs,
   if (QueryDriverGlobs.empty())
     return Base;
   return std::make_unique<QueryDriverDatabase>(QueryDriverGlobs,
-                                                std::move(Base));
+                                               std::move(Base));
 }
 
 } // namespace clangd

clang-tools-extra/clangd/test/system-include-extractor.test

@@ -5,8 +5,12 @@
 
 # Generate a mock-driver that will print %temp_dir%/my/dir and
 # %temp_dir%/my/dir2 as include search paths.
-# RUN: echo '#!/bin/bash' >> %t.dir/my_driver.sh
+# RUN: echo '#!/bin/sh' >> %t.dir/my_driver.sh
 # RUN: echo '[ "$0" = "%t.dir/my_driver.sh" ] || exit' >> %t.dir/my_driver.sh
+# RUN: echo 'args="$*"' >> %t.dir/my_driver.sh
+# RUN: echo '[ -z "${args##*"-nostdinc"*}" ] || exit' >> %t.dir/my_driver.sh
+# RUN: echo '[ -z "${args##*"-isysroot=/isysroot"*}" ] || exit' >> %t.dir/my_driver.sh
+# RUN: echo 'echo " $* " | grep " --sysroot /my/sysroot/path " || exit' >> %t.dir/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo -e "#include <...> search starts here:\r" >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir/ >&2' >> %t.dir/my_driver.sh
@@ -22,7 +26,7 @@
 
 # Generate a compile_commands.json that will query the mock driver we've
 # created. Which should add a.h and b.h into include search path.
-# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/my_driver.sh the-file.cpp", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/my_driver.sh the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."