[clangd] Try harder to find a plausible `clang` as argv0, particularly on Mac.

[sammccall]

Summary

Fixes https://github.com/clangd/clangd/issues/211
Fixes https://github.com/clangd/clangd/issues/178

No tests - this is hard to test, and basically impossible to verify what we want
(this produces compile commands that work on a real mac with recent toolchain)

(Need someone on mac to verify it actually fixes these!)

[ilya-biryukov]

The code LG, but my mac is at home, so I could not verify it.
@kbobyrev, would you mind running this on your machine?

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

266

NIT: maybe add braces to the for statement?

[kadircet]

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

266

did you mean "cc" instead of "ccc"? or is that really a think ?

[ilya-biryukov]

Another interesting consideration: we choose to ask users to whitelists compilers we might run from compile_commands.json that we can.
We are in a better position here, since we're not running the binaries based on user input.

Technically, we could consider using the same mechanism for running xcrun. It will probably never be used in practice, though (and we'll have to whitelist some common xcrun binaries anyway).

[kbobyrev]

The current version of the patch fixes one of the related issues. When running on a simple file like this

#include <iostream>

int main() {
        std::cout << "Hello, world" << std::endl;
        return 0;
}

Clangd would fail to find iostream header and produce errors for std::cout and std::endl which are also not found before the patch. After the patch, this is no longer an issue. Hence, I believe that standard library headers can be identified correctly now.

However, after the patch wchar.h from #include_next is not found and this produces another error. wchar.h there is AFAIK not a part of the standard library and should be found in the SDKROOT.

The following is a log of Clangd failing to find wchar.h:

kbobyrev$ ./bin/clangd -sync < /tmp/mirror                                                                                                                                    
clangd is a language server that provides IDE-like features to editors.

It should be used via an editor plugin rather than invoked directly. For more information, see:
        https://clang.llvm.org/extra/clangd/
        https://microsoft.github.io/language-server-protocol/

clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment variable.

I[11:18:01.774] clangd version 10.0.0 (https://github.com/llvm/llvm-project bd23859f390aa81ddb1bf0b16684cce50ad9d66d)                                                                                          
I[11:18:01.775] PID: 92552
I[11:18:01.775] Working directory: /Users/kbobyrev/dev/build/llvm-release
I[11:18:01.775] argv[0]: ./bin/clangd
I[11:18:01.775] argv[1]: -sync
I[11:18:01.775] Starting LSP over stdin/stdout
I[11:18:01.776] <-- initialize(0)
I[11:18:01.776] --> reply:initialize(0) 0 ms
Content-Length: 800

{"id":0,"jsonrpc":"2.0","result":{"capabilities":{"codeActionProvider":{"codeActionKinds":["quickfix","refactor","info"]},"completionProvider":{"resolveProvider":false,"triggerCharacters":[".",">",":"]},"declarationProvider":true,"definitionProvider":true,"documentFormattingProvider":true,"documentHighlightProvider":true,"documentOnTypeFormattingProvider":{"firstTriggerCharacter":"\n","moreTriggerCharacter":[]},"documentRangeFormattingProvider":true,"documentSymbolProvider":true,"executeCommandProvider":{"commands":["clangd.applyFix","clangd.applyTweak"]},"hoverProvider":true,"referencesProvider":true,"renameProvider":true,"selectionRangeProvider":true,"signatureHelpProvider":{"triggerCharacters":["(",","]},"textDocumentSync":2,"typeHierarchyProvider":true,"workspaceSymbolProvider":true}}}I[11:18:01.776] <-- textDocument/didOpen
I[11:18:01.789] Failed to find compilation database for /private/tmp/test.cpp
I[11:18:01.789] Updating file /private/tmp/test.cpp with command clangd fallback
[/private/tmp]
/Library/Developer/CommandLineTools/usr/bin/clang /private/tmp/test.cpp -fsyntax-only -resource-dir=/Users/kbobyrev/dev/build/llvm-release/lib/clang/10.0.0                                                    
I[11:18:02.155] --> textDocument/publishDiagnostics
Content-Length: 819

{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"code":"pp_file_not_found","message":"In included file: 'wchar.h' file not found","range":{"end":{"character":10,"line":0},"start":{"character":9,"line":0}},"relatedInformation":[{"location":{"range":{"end":{"character":23,"line":118},"start":{"character":14,"line":118}},"uri":"file:///Library/Developer/CommandLineTools/usr/include/c%2B%2B/v1/wchar.h"},"message":"Error occurred here"}],"severity":1,"source":"clang"},{"code":"typecheck_invalid_operands","message":"Invalid operands to binary expression ('std::__1::ostream' (aka 'int') and 'const char [13]')","range":{"end":{"character":13,"line":3},"start":{"character":11,"line":3}},"relatedInformation":[],"severity":1,"source":"clang"}],"uri":"file:///private/tmp/test.cpp"}}I[11:18:02.155] Warning: Missing Content-Length header, or zero-length message.
E[11:18:02.155] Transport error: Input/output error
I[11:18:02.155] LSP finished, exiting with status 1

This can be fixed manually by appending env SDKROOT=$(xcrun --show-sdk-path) to the clangd invocation (or simply defining the environment variable for all invocations), but I believe we might want to deal with it on the Clangd side, too.

[sammccall]

@kbobyrev tested this and it turns out we also have to set $SDKROOT. And we probably want to fix clang in compile_commands.json too.

In D70863#1764785, @ilya-biryukov wrote:

Another interesting consideration: we choose to ask users to whitelists compilers we might run from compile_commands.json that we can.
We are in a better position here, since we're not running the binaries based on user input.

Interesting idea. Wouldn't mix it with this patch as the purposes don't overlap much...

• apple clang in practice won't report the required info to the driver query until the next major xcode release I think (with your driver patch)
• the motivating case for this patch is the fallback compile command

Technically, we could consider using the same mechanism for running xcrun. It will probably never be used in practice, though (and we'll have to whitelist some common xcrun binaries anyway).

You mean the whitelist? The security risk we were worried about with --query_driver is that compile_commands.json is easily attacker-controlled. The string xcrun is fixed, and the attack "put a different xcrun on the user's PATH" requires way more privileges - generally you're owned at that point anyway. I don't think it's worth guarding.

[sammccall]

Also add detected -isysroot on mac unless $SDKROOT is set.
Also add path to driver when bare driver name comes from CDB.
While here, address -resource-dir fixme.

[sammccall]

@kbobyrev would you mind testing this again?

Dropping an empty compile_flags.txt should also work (and test the other codepath)

[kbobyrev]

In D70863#1764977, @sammccall wrote:

@kbobyrev would you mind testing this again?

Dropping an empty compile_flags.txt should also work (and test the other codepath)

I've tested again (and also with empty compile_flags.txt) and this works now.

[sammccall]

In D70863#1765042, @kbobyrev wrote:

In D70863#1764977, @sammccall wrote:

@kbobyrev would you mind testing this again?

Dropping an empty compile_flags.txt should also work (and test the other codepath)

I've tested again (and also with empty compile_flags.txt) and this works now.

Woohoo, thanks!

[arphaman]

@sammccall Sam, it looks like the tests are failing on the darwin bots:

http://lab.llvm.org:8080/green/job/clang-stage1-RA/4243/consoleFull

Value of: CDB.getFallbackCommand(testPath("bar.cc")).CommandLine
Expected: has 6 elements where
element #0 ends with "clang",
element #1 is equal to "-DA=2",
element #2 is equal to "/clangd-test/bar.cc",
element #3 is equal to "-DA=4",
element #4 is equal to "-fsyntax-only",
element #5 starts with "-resource-dir"
  Actual: { "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang", "-DA=2", "/clangd-test/bar.cc", "-DA=4", "-fsyntax-only", "-resource-dir=/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/tools/clang/tools/extra/clangd/lib/clang/10.0.0", "-isysroot", "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk" }, which has 8 elements

Can you please take a look?

[sammccall]

In D70863#1765668, @arphaman wrote:

@sammccall Sam, it looks like the tests are failing on the darwin bots:

http://lab.llvm.org:8080/green/job/clang-stage1-RA/4243/consoleFull

Thanks, and sorry! Fingers crossed that 82039cbc8d2a0f6fb5995f54e0e42919999bcfd0 will fix it...

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include <string>
 #include <tuple>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 namespace {
 
+// Query apple's `xcrun` launcher, which is the source of truth for "how should"
+// clang be invoked on this system.
+llvm::Optional<std::string> queryXcrun(llvm::ArrayRef<llvm::StringRef> Argv) {
+  auto Xcrun = llvm::sys::findProgramByName("xcrun");
+  if (!Xcrun) {
+    log("Couldn't find xcrun. Hopefully you have a non-apple toolchain...");
+    return llvm::None;
+  }
+  llvm::SmallString<64> OutFile;
+  llvm::sys::fs::createTemporaryFile("clangd-xcrun", "", OutFile);
+  llvm::FileRemover OutRemover(OutFile);
+  llvm::Optional<llvm::StringRef> Redirects[3] = {
+      /*stdin=*/{""}, /*stdout=*/{OutFile}, /*stderr=*/{""}};
+  vlog("Invoking {0} to find clang installation", *Xcrun);
+  int Ret = llvm::sys::ExecuteAndWait(*Xcrun, Argv,
+                                      /*Env=*/llvm::None, Redirects,
+                                      /*SecondsToWait=*/10);
+  if (Ret != 0) {
+    log("xcrun exists but failed with code {0}. "
+        "If you have a non-apple toolchain, this is OK. "
+        "Otherwise, try xcode-select --install.",
+        Ret);
+    return llvm::None;
+  }
+
+  auto Buf = llvm::MemoryBuffer::getFile(OutFile);
+  if (!Buf) {
+    log("Can't read xcrun output: {0}", Buf.getError().message());
+    return llvm::None;
+  }
+  StringRef Path = Buf->get()->getBuffer().trim();
+  if (Path.empty()) {
+    log("xcrun produced no output");
+    return llvm::None;
+  }
+  return Path.str();
+}
+
+// On Mac, `which clang` is /usr/bin/clang. It runs `xcrun clang`, which knows
+// where the real clang is kept. We need to do the same thing,
+// because cc1 (not the driver!) will find libc++ relative to argv[0].
+llvm::Optional<std::string> queryMacClangPath() {
+#ifndef __APPLE__
+  return llvm::None;
+#endif
+
+  return queryXcrun({"xcrun", "--find", "clang"});
+}
+
+// Resolve symlinks if possible.
+std::string resolve(std::string Path) {
+  llvm::SmallString<128> Resolved;
+  if (llvm::sys::fs::real_path(Path, Resolved))
+    return Path; // On error;
+  return Resolved.str();
+}
+
+// Get a plausible full `clang` path.
+// This is used in the fallback compile command, or when the CDB returns a
+// generic driver with no path.
+llvm::StringRef getFallbackClangPath() {
+  static const std::string &MemoizedFallbackPath = [&]() -> std::string {
+    // The driver and/or cc1 sometimes depend on the binary name to compute
+    // useful things like the standard library location.
+    // We need to emulate what clang on this system is likely to see.
+    // cc1 in particular looks at the "real path" of the running process, and
+    // so if /usr/bin/clang is a symlink, it sees the resolved path.
+    // clangd doesn't have that luxury, so we resolve symlinks ourselves.
+
+    // /usr/bin/clang on a mac is a program that redirects to the right clang.
+    // We resolve it as if it were a symlink.
+    if (auto MacClang = queryMacClangPath())
+      return resolve(std::move(*MacClang));
+    // On other platforms, just look for compilers on the PATH.
+    for (const char* Name : {"clang", "gcc", "cc"})
+      if (auto PathCC = llvm::sys::findProgramByName(Name))
+        return resolve(std::move(*PathCC));
+    // Fallback: a nonexistent 'clang' binary next to clangd.
+    static int Dummy;
+    std::string ClangdExecutable =
+        llvm::sys::fs::getMainExecutable("clangd", (void *)&Dummy);
+    SmallString<128> ClangPath;
+    ClangPath = llvm::sys::path::parent_path(ClangdExecutable);
+    llvm::sys::path::append(ClangPath, "clang");
+    return ClangPath.str();
+  }();
+  return MemoizedFallbackPath;
+}
+
+// On mac, /usr/bin/clang sets SDKROOT and then invokes the real clang.
+// The effect of this is to set -isysroot correctly. We do the same.
+const std::string *getMacSysroot() {
+#ifndef __APPLE__
+  return nullptr;
+#endif
+
+  // SDKROOT overridden in environment, respect it. Driver will set isysroot.
+  if (::getenv("SDKROOT"))
+    return nullptr;
+  static const llvm::Optional<std::string> &Sysroot =
+      queryXcrun({"xcrun", "--show-sdk-path"});
+  return Sysroot ? Sysroot.getPointer() : nullptr;
+}
+
+// Transform a command into the form we want to send to the driver.
+// The command was originally either from the CDB or is the fallback command,
+// and may have been modified by OverlayCDB.
 void adjustArguments(tooling::CompileCommand &Cmd,
                      llvm::StringRef ResourceDir) {
   tooling::ArgumentsAdjuster ArgsAdjuster = tooling::combineAdjusters(
       // clangd should not write files to disk, including dependency files
       // requested on the command line.
       tooling::getClangStripDependencyFileAdjuster(),
       // Strip plugin related command line arguments. Clangd does
       // not support plugins currently. Therefore it breaks if
       // compiler tries to load plugins.
       tooling::combineAdjusters(tooling::getStripPluginsAdjuster(),
                                 tooling::getClangSyntaxOnlyAdjuster()));
 
   Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, Cmd.Filename);
+  // Check whether the flag exists, either as -flag or -flag=*
+  auto Has = [&](llvm::StringRef Flag) {
+    for (llvm::StringRef Arg : Cmd.CommandLine) {
+      if (Arg.consume_front(Flag) && (Arg.empty() || Arg[0] == '='))
+        return true;
+    }
+    return false;
+  };
   // Inject the resource dir.
-  // FIXME: Don't overwrite it if it's already there.
-  if (!ResourceDir.empty())
+  if (!ResourceDir.empty() && !Has("-resource-dir"))
     Cmd.CommandLine.push_back(("-resource-dir=" + ResourceDir).str());
+  if (!Has("-isysroot"))
+    if (const std::string *MacSysroot = getMacSysroot()) {
+      Cmd.CommandLine.push_back("-isysroot");
+      Cmd.CommandLine.push_back(*MacSysroot);
+    }
+
+  // If the driver is a generic name like "g++" with no path, add a clang path.
+  // This makes it easier for us to find the standard libraries on mac.
+  if (!Cmd.CommandLine.empty()) {
+    std::string &Driver = Cmd.CommandLine.front();
+    if (Driver == "clang" || Driver == "clang++" || Driver == "gcc" ||
+        Driver == "g++" || Driver == "cc" || Driver == "c++") {
+      llvm::SmallString<128> QualifiedDriver =
+          llvm::sys::path::parent_path(getFallbackClangPath());
+      llvm::sys::path::append(QualifiedDriver, Driver);
+      Driver = QualifiedDriver.str();
+    }
+  }
 }
 
 std::string getStandardResourceDir() {
   static int Dummy; // Just an address in this process.
   return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
 }
 
 // Runs the given action on all parent directories of filename, starting from
 // deepest directory and going up to root. Stops whenever action succeeds.
 void actOnAllParentDirectories(PathRef FileName,
                                llvm::function_ref<bool(PathRef)> Action) {
   for (auto Path = llvm::sys::path::parent_path(FileName);
        !Path.empty() && !Action(Path);
        Path = llvm::sys::path::parent_path(Path))
     ;
 }
 
 } // namespace
 
-static std::string getFallbackClangPath() {
-  static int Dummy;
-  std::string ClangdExecutable =
-      llvm::sys::fs::getMainExecutable("clangd", (void *)&Dummy);
-  SmallString<128> ClangPath;
-  ClangPath = llvm::sys::path::parent_path(ClangdExecutable);
-  llvm::sys::path::append(ClangPath, "clang");
-  return ClangPath.str();
-}
-
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
-  std::vector<std::string> Argv = {getFallbackClangPath()};
+  std::vector<std::string> Argv = {"clang"};
   // Clang treats .h files as C by default and files without extension as linker
   // input, resulting in unhelpful diagnostics.
   // Parsing as Objective C++ is friendly to more cases.
   auto FileExtension = llvm::sys::path::extension(File);
   if (FileExtension.empty() || FileExtension == ".h")
     Argv.push_back("-xobjective-c++-header");
   Argv.push_back(File);
   tooling::CompileCommand Cmd(llvm::sys::path::parent_path(File),

 static bool pathEqual(PathRef A, PathRef B) {
 #if defined(_WIN32) || defined(__APPLE__)
   return A.equals_lower(B);
 #else
   return A == B;
 #endif
 }
 
 DirectoryBasedGlobalCompilationDatabase::CachedCDB &
 DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const {
   // FIXME(ibiryukov): Invalidate cached compilation databases on changes
   // FIXME(sammccall): this function hot, avoid copying key when hitting cache.
   auto Key = maybeCaseFoldPath(Dir);
   auto R = CompilationDatabases.try_emplace(Key);
   if (R.second) { // Cache miss, try to load CDB.
     CachedCDB &Entry = R.first->second;
     std::string Error = "";

clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp

TEST_F(BackgroundIndexTest, CmdLineHash) {
   BackgroundIndex Idx(Context::empty(), FS, CDB,
                       [&](llvm::StringRef) { return &MSS; });
 
   tooling::CompileCommand Cmd;
   FS.Files[testPath("A.cc")] = "#include \"A.h\"";
   FS.Files[testPath("A.h")] = "";
   Cmd.Filename = "../A.cc";
   Cmd.Directory = testPath("build");
-  Cmd.CommandLine = {"clang++", "../A.cc", "-fsyntax-only"};
+  Cmd.CommandLine = {"/bin/clang++", "../A.cc", "-fsyntax-only"};
   CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
 
   EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc"), testPath("A.h")));
   // Make sure we only store the Cmd for main file.
   EXPECT_FALSE(MSS.loadShard(testPath("A.h"))->Cmd);
 
   {
     tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd;
     EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);
     EXPECT_EQ(CmdStored.Directory, Cmd.Directory);
   }
 
   // FIXME: Changing compile commands should be enough to invalidate the cache.
   FS.Files[testPath("A.cc")] = " ";
-  Cmd.CommandLine = {"clang++", "../A.cc", "-Dfoo", "-fsyntax-only"};
+  Cmd.CommandLine = {"/bin/clang++", "../A.cc", "-Dfoo", "-fsyntax-only"};
   CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
 
   EXPECT_FALSE(MSS.loadShard(testPath("A.h"))->Cmd);
 
   {
     tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd;
     EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);

clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

TEST_F(OverlayCDBTest, GetCompileCommand) {
   CDB.setCompileCommand(testPath("missing.cc"), Override);
   EXPECT_THAT(CDB.getCompileCommand(testPath("missing.cc"))->CommandLine,
               Contains("-DA=3"));
 }
 
 TEST_F(OverlayCDBTest, GetFallbackCommand) {
   OverlayCDB CDB(Base.get(), {"-DA=4"});
   EXPECT_THAT(CDB.getFallbackCommand(testPath("bar.cc")).CommandLine,
-              ElementsAre("clang", "-DA=2", testPath("bar.cc"), "-DA=4",
-                          "-fsyntax-only", StartsWith("-resource-dir")));
+              ElementsAre(EndsWith("clang"), "-DA=2", testPath("bar.cc"),
+                          "-DA=4", "-fsyntax-only",
+                          StartsWith("-resource-dir")));
 }
 
 TEST_F(OverlayCDBTest, NoBase) {
   OverlayCDB CDB(nullptr, {"-DA=6"}, std::string(""));
   EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), None);
   auto Override = cmd(testPath("bar.cc"), "-DA=5");
   CDB.setCompileCommand(testPath("bar.cc"), Override);
   EXPECT_THAT(CDB.getCompileCommand(testPath("bar.cc"))->CommandLine,