[clangd] Exclude builtin headers from system include extraction

[sammccall]

Summary

Most headers that we discover are likely to be fairly portable across at least
clang/gcc, which is why we get away with adding them to clangd's search path.

This is not the case for the compiler builtin headers, which are shipped with
the parser and rely on parser builtins/features. We're better off *not* using
the the scanned builtin headers, and hoping our own intrinsics provide the
interface the code is relying on.

Fixes https://github.com/clangd/clangd/issues/1695

[sammccall]

remove (broken) support for -nobuiltininc

[sammccall]

fix incomplete cleanup of dead option

[madscientist]

I built latest main with this patch applied and it does fix the issues in my environment. My headers that include xmmintrin.h etc. don't throw errors in clangd. Thanks!

[kadircet]

thanks a lot!

[madscientist]

(just a note that another way to do this check is to look for some common intrinsic file in each directory generated by the compiler driver, and remove any directory containing those files. But, if the -print-file-name=include trick works reliably that's good too).

clang-tools-extra/clangd/SystemIncludeExtractor.cpp

 };
 
 struct DriverArgs {
   // Name of the driver program to execute or absolute path to it.
   std::string Driver;
   // Whether certain includes should be part of query.
   bool StandardIncludes = true;
   bool StandardCXXIncludes = true;
-  bool BuiltinIncludes = true;
   // Language to use while querying.
   std::string Lang;
   std::string Sysroot;
   std::string ISysroot;
 
   bool operator==(const DriverArgs &RHS) const {
-    return std::tie(Driver, StandardIncludes, StandardCXXIncludes,
-                    BuiltinIncludes, Lang, Sysroot, ISysroot) ==
+    return std::tie(Driver, StandardIncludes, StandardCXXIncludes, Lang,
+                    Sysroot, ISysroot) ==
            std::tie(RHS.Driver, RHS.StandardIncludes, RHS.StandardCXXIncludes,
-                    RHS.BuiltinIncludes, RHS.Lang, RHS.Sysroot, ISysroot);
+                    RHS.Lang, RHS.Sysroot, ISysroot);
   }
 
   DriverArgs(const tooling::CompileCommand &Cmd, llvm::StringRef File) {
     llvm::SmallString<128> Driver(Cmd.CommandLine.front());
     // Driver is a not a single executable name but instead a path (either
     // relative or absolute).
     if (llvm::any_of(Driver,
                      [](char C) { return llvm::sys::path::is_separator(C); })) {
for (size_t I = 0, E = Cmd.CommandLine.size(); I < E; ++I) {
         else
           Lang = Arg.str();
       }
       // Look for standard/builtin includes.
       else if (Arg == "-nostdinc" || Arg == "--no-standard-includes")
         StandardIncludes = false;
       else if (Arg == "-nostdinc++")
         StandardCXXIncludes = false;
-      else if (Arg == "-nobuiltininc")
-        BuiltinIncludes = false;
       // Figure out sysroot
       else if (Arg.consume_front("--sysroot")) {
         if (Arg.consume_front("="))
           Sysroot = Arg.str();
         else if (Arg.empty() && I + 1 < E)
           Sysroot = Cmd.CommandLine[I + 1];
       } else if (Arg.consume_front("-isysroot")) {
         if (Arg.empty() && I + 1 < E)
struct DriverArgs {
   llvm::SmallVector<llvm::StringRef> render() const {
     // FIXME: Don't treat lang specially?
     assert(!Lang.empty());
     llvm::SmallVector<llvm::StringRef> Args = {"-x", Lang};
     if (!StandardIncludes)
       Args.push_back("-nostdinc");
     if (!StandardCXXIncludes)
       Args.push_back("-nostdinc++");
-    if (!BuiltinIncludes)
-      Args.push_back("-nobuiltininc++");
     if (!Sysroot.empty())
       Args.append({"--sysroot", Sysroot});
     if (!ISysroot.empty())
       Args.append({"-isysroot", ISysroot});
     return Args;
   }
 
   static DriverArgs getEmpty() { return {}; }
static DriverArgs getTombstoneKey() {
     Driver.Driver = "TOMBSTONE_KEY";
     return Driver;
   }
   static unsigned getHashValue(const DriverArgs &Val) {
     return llvm::hash_value(std::tuple{
         Val.Driver,
         Val.StandardIncludes,
         Val.StandardCXXIncludes,
-        Val.BuiltinIncludes,
         Val.Lang,
         Val.Sysroot,
         Val.ISysroot,
     });
   }
   static bool isEqual(const DriverArgs &LHS, const DriverArgs &RHS) {
     return LHS == RHS;
   }
std::optional<DriverInfo> parseDriverOutput(llvm::StringRef Output) {
   }
   if (State == IncludesExtracting) {
     elog("System include extraction: end marker missing: {0}", Output);
     return std::nullopt;
   }
   return std::move(Info);
 }
 
+std::optional<std::string> run(llvm::ArrayRef<llvm::StringRef> Argv,
+                               bool OutputIsStderr) {
+  llvm::SmallString<128> OutputPath;
+  if (auto EC = llvm::sys::fs::createTemporaryFile("system-includes", "clangd",
+                                                   OutputPath)) {
+    elog("System include extraction: failed to create temporary file with "
+         "error {0}",
+         EC.message());
+    return std::nullopt;
+  }
+  auto CleanUp = llvm::make_scope_exit(
+      [&OutputPath]() { llvm::sys::fs::remove(OutputPath); });
+
+  std::optional<llvm::StringRef> Redirects[] = {{""}, {""}, {""}};
+  Redirects[OutputIsStderr ? 2 : 1] = OutputPath.str();
+
+  std::string ErrMsg;
+  if (int RC =
+          llvm::sys::ExecuteAndWait(Argv.front(), Argv, /*Env=*/std::nullopt,
+                                    Redirects, /*SecondsToWait=*/0,
+                                    /*MemoryLimit=*/0, &ErrMsg)) {
+    elog("System include extraction: driver execution failed with return code: "
+         "{0} - '{1}'. Args: [{2}]",
+         llvm::to_string(RC), ErrMsg, printArgv(Argv));
+    return std::nullopt;
+  }
+
+  auto BufOrError = llvm::MemoryBuffer::getFile(OutputPath);
+  if (!BufOrError) {
+    elog("System include extraction: failed to read {0} with error {1}",
+         OutputPath, BufOrError.getError().message());
+    return std::nullopt;
+  }
+  return BufOrError.get().get()->getBuffer().str();
+}
+
 std::optional<DriverInfo>
 extractSystemIncludesAndTarget(const DriverArgs &InputArgs,
                                const llvm::Regex &QueryDriverRegex) {
   trace::Span Tracer("Extract system includes and target");
 
   std::string Driver = InputArgs.Driver;
   if (!llvm::sys::path::is_absolute(Driver)) {
     auto DriverProgram = llvm::sys::findProgramByName(Driver);
extractSystemIncludesAndTarget(const DriverArgs &InputArgs,
   SPAN_ATTACH(Tracer, "driver", Driver);
   SPAN_ATTACH(Tracer, "lang", InputArgs.Lang);
 
   if (!QueryDriverRegex.match(Driver)) {
     vlog("System include extraction: not allowed driver {0}", Driver);
     return std::nullopt;
   }
 
-  llvm::SmallString<128> StdErrPath;
-  if (auto EC = llvm::sys::fs::createTemporaryFile("system-includes", "clangd",
-                                                   StdErrPath)) {
-    elog("System include extraction: failed to create temporary file with "
-         "error {0}",
-         EC.message());
-    return std::nullopt;
-  }
-  auto CleanUp = llvm::make_scope_exit(
-      [&StdErrPath]() { llvm::sys::fs::remove(StdErrPath); });
-
-  std::optional<llvm::StringRef> Redirects[] = {{""}, {""}, StdErrPath.str()};
-
   llvm::SmallVector<llvm::StringRef> Args = {Driver, "-E", "-v"};
   Args.append(InputArgs.render());
   // Input needs to go after Lang flags.
   Args.push_back("-");
-
-  std::string ErrMsg;
-  if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/std::nullopt,
-                                         Redirects, /*SecondsToWait=*/0,
-                                         /*MemoryLimit=*/0, &ErrMsg)) {
-    elog("System include extraction: driver execution failed with return code: "
-         "{0} - '{1}'. Args: [{2}]",
-         llvm::to_string(RC), ErrMsg, printArgv(Args));
+  auto Output = run(Args, /*OutputIsStderr=*/true);
+  if (!Output)
     return std::nullopt;
-  }
 
-  auto BufOrError = llvm::MemoryBuffer::getFile(StdErrPath);
-  if (!BufOrError) {
-    elog("System include extraction: failed to read {0} with error {1}",
-         StdErrPath, BufOrError.getError().message());
+  std::optional<DriverInfo> Info = parseDriverOutput(*Output);
+  if (!Info)
     return std::nullopt;
+
+  // The built-in headers are tightly coupled to parser builtins.
+  // (These are clang's "resource dir", GCC's GCC_INCLUDE_DIR.)
+  // We should keep using clangd's versions, so exclude the queried builtins.
+  // They're not specially marked in the -v output, but we can get the path
+  // with `$DRIVER -print-file-name=include`.
+  if (auto BuiltinHeaders =
+          run({Driver, "-print-file-name=include"}, /*OutputIsStderr=*/false)) {
+    auto Path = llvm::StringRef(*BuiltinHeaders).trim();
+    if (!Path.empty() && llvm::sys::path::is_absolute(Path)) {
+      auto Size = Info->SystemIncludes.size();
+      llvm::erase_if(Info->SystemIncludes,
+                     [&](llvm::StringRef Entry) { return Path == Entry; });
+      vlog("System includes extractor: builtin headers {0} {1}", Path,
+           (Info->SystemIncludes.size() != Size)
+               ? "excluded"
+               : "not found in driver's response");
+    }
   }
 
-  std::optional<DriverInfo> Info =
-      parseDriverOutput(BufOrError->get()->getBuffer());
-  if (!Info)
-    return std::nullopt;
   log("System includes extractor: successfully executed {0}\n\tgot includes: "
       "\"{1}\"\n\tgot target: \"{2}\"",
       Driver, llvm::join(Info->SystemIncludes, ", "), Info->Target);
   return Info;
 }
 
 tooling::CompileCommand &
 addSystemIncludes(tooling::CompileCommand &Cmd,

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

 # RUN: rm -rf %t.dir && mkdir -p %t.dir
 
 # The mock driver below is a shell script:
 # REQUIRES: shell
 
 # Create a bin directory to store the mock-driver and add it to the path
 # RUN: mkdir -p %t.dir/bin
 # RUN: export PATH=%t.dir/bin:$PATH
 # Generate a mock-driver that will print %temp_dir%/my/dir and
 # %temp_dir%/my/dir2 as include search paths.
 # RUN: echo '#!/bin/sh' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ "$0" = "%t.dir/bin/my_driver.sh" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ "$1" = "-print-file-name=include" ] && echo "%t.dir/builtin" && exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'args="$*"' >> %t.dir/bin/my_driver.sh
 # Check that clangd preserves certain flags like `-nostdinc` from
 # original invocation in compile_commands.json.
 # RUN: echo '[ -z "${args##*"-nostdinc"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"--sysroot /my/sysroot/path"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"-isysroot /isysroot"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "Target: arm-linux-gnueabihf\r\n" >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir/ >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir2/ >&2' >> %t.dir/bin/my_driver.sh
+# RUN: echo 'echo %t.dir/builtin >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "End of search list.\r\n" >&2' >> %t.dir/bin/my_driver.sh
 # RUN: chmod +x %t.dir/bin/my_driver.sh
 
 # Create header files my/dir/a.h and my/dir2/b.h
 # RUN: mkdir -p %t.dir/my/dir
 # RUN: touch %t.dir/my/dir/a.h
 # RUN: mkdir -p %t.dir/my/dir2
 # RUN: touch %t.dir/my/dir2/b.h
+# RUN: mkdir -p %t.dir/builtin
+# RUN: touch %t.dir/builtin/c.h
 
 # 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": "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:/..."
 # (with the extra slash in the front), so we add it here.
 # RUN: sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' %t.test.1 > %t.test
 
 # Bless the mock driver we've just created so that clangd can execute it.
 # Note: include clangd's stderr in the FileCheck input with "2>&1" so that we
 # can match output lines like "ASTWorker building file"
 # RUN: clangd -lit-test -query-driver="**.test,**.sh" < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
 ---
 {
   "jsonrpc":"2.0",
   "method":"textDocument/didOpen",
   "params": {
     "textDocument": {
       "uri": "file://INPUT_DIR/the-file.cpp",
       "languageId":"cpp",
       "version":1,
-      "text":"#include <a.h>\n#include <b.h>\n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n"
+      "text":"#include <a.h>\n#include <b.h>\n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n#if __has_include(<c.h>)\n#error \"wrong-toolchain builtins\"\n#endif\n"
     }
   }
 }
 # Look for the "ASTWorker building file" line so that the subsequent diagnostics
 # that are matches are for the C++ source file and not a config file.
 # CHECK: ASTWorker building file
 # CHECK:   "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:   "params": {