Make clang-based tools find libc++ on MacOS

[ilya-biryukov]

Summary

When they read compiler args from compile_commands.json.
This change allows to run clang-based tools, like clang-tidy or clangd,
built from head using the compile_commands.json file produced for XCode
toolchains.

On MacOS clang can find the C++ standard library relative to the
compiler installation dir.

The logic to do this was based on resource dir as an approximation of
where the compiler is installed. This broke the tools that read
'compile_commands.json' and don't ship with the compiler, as they
typically change resource dir.

To workaround this, we now use compiler install dir detected by the driver
to better mimic the behavior of the original compiler when replaying the
compilations using other tools.

[sammccall]

This is hacky, but solves an important problem. If you know of a good reviewer for this, you may want a second opinion!

include/clang/Lex/HeaderSearchOptions.h

111 ↗

(On Diff #173318)

Could you add "typically contains bin/clang, lib/libclang_rt, and include/<builtins>"?

lib/Frontend/CompilerInvocation.cpp

1776 ↗

(On Diff #173318)

(is this needed? I guess you fall back to this if you don't create the CI from the command line?)

[ilya-biryukov]

• Update the comment.

[ilya-biryukov]

In D54310#1292924, @sammccall wrote:

If you know of a good reviewer for this, you may want a second opinion!

This would definitely be nice, but I also don't know who'd be the proper person to review this.
I'll take a pause until Monday, maybe @arphaman or someone else from the community can suggest a better way of fixing this or proper reviewers for this change.

include/clang/Lex/HeaderSearchOptions.h

111 ↗

(On Diff #173318)

This is actually the 'bin' subdirectory, I've added a comment about that but you see a better way to communicate this, please let me know! E.g. actually mentioning the parallel lib/ and include/ subdirs.

lib/Frontend/CompilerInvocation.cpp

1776 ↗

(On Diff #173318)

Thanks for noticing this!
This is a leftover from a previous iteration, it's actually incorrect now since we rely on InstallDir pointing to a different place from the resource dir.
Removed it.

[ilya-biryukov]

• Added a test with a compiler path relative to the working dir

[JDevlieghere]

I reverted this because it broke the LLDB bots on GreenDragon: http://green.lab.llvm.org/green/view/LLDB/

[ilya-biryukov]

Will take a look, thanks for the revert.

[george.karpenkov]

I don't quite understand the need for this patch.
If we are talking about a binary built from source, wouldn't it make more sense to build libcxx from source as well?
If libcxx is built from source in the same repo, clang-based tools do find it.

[george.karpenkov]

I suspect it broke http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/14090/console as well.

[george.karpenkov]

In the future, for macOS-specific changes I think it would be better to wait for a sign-off from at least one maintainer who is an expert on Apple tools.

[llvm-commits]

• msg-17782-346.txt162 BDownload

[arphaman]

Apologies for not seeing this earlier.
I agree with George, this behavior doesn't seem right to me.

The logic to do this was based on resource dir as an approximation of
where the compiler is installed. This broke the tools that read
'compile_commands.json' and don't ship with the compiler, as they
typically change resource dir.

In that case I think the tool should adjust the -resource-dir to point to the appropriate -resource-dir for the compiler in the toolchain. However, that comes with its own problems, as the builtin headers might be incompatible between the tool and the compiler in the toolchain. In that case I think it would be preferable for the tool to ship with its own libc++ headers in addition to the builtin headers rather than implementing this behavior. If that's unacceptable I think we should adjust libTooling to add a search path for the libc++ headers when the tool detects that it's running outside of the toolchain on Darwin (i.e. 'clang' doesn't exist in the same directory as the tool, and '../include/c++' doesn't exist as well) instead of changing this behavior.

[ilya-biryukov]

In D54310#1296080, @arphaman wrote:

Apologies for not seeing this earlier.

No worries, thanks for the input!

The logic to do this was based on resource dir as an approximation of
where the compiler is installed. This broke the tools that read
'compile_commands.json' and don't ship with the compiler, as they
typically change resource dir.

In that case I think the tool should adjust the -resource-dir to point to the appropriate -resource-dir for the compiler in the toolchain. However, that comes with its own problems, as the builtin headers might be incompatible between the tool and the compiler in the toolchain.

Exactly the problem we're facing. Since the -resource-dir is primarily used to find the builtin headers, we have to override it in the tools to avoid breaking them.

In that case I think it would be preferable for the tool to ship with its own libc++ headers in addition to the builtin headers rather than implementing this behavior.

There's value in providing the users with warnings/errors from exactly the same version of the C++ standard library that's used by the compiler, so I would avoid going down this path if possible.

If that's unacceptable I think we should adjust libTooling to add a search path for the libc++ headers when the tool detects that it's running outside of the toolchain on Darwin (i.e. 'clang' doesn't exist in the same directory as the tool, and '../include/c++' doesn't exist as well) instead of changing this behavior.

This was the original intention of the patch, but I don't think we should limit this to libTooling.
I'll have a look at what exactly lldb broke and will come up with a fix to this behavior.

cfe/trunk/include/clang/Lex/HeaderSearchOptions.h

public:
 
   /// User-specified system header prefixes.
   std::vector<SystemHeaderPrefix> SystemHeaderPrefixes;
 
   /// The directory which holds the compiler resource files (builtin includes,
   /// etc.).
   std::string ResourceDir;
 
+  /// Compiler install dir as detected by the Driver.
+  /// This is typically the directory that contains the clang executable, i.e.
+  /// the 'bin/' subdir of a clang distribution.
+  /// Only used to add include dirs for libc++ on Darwin. Please avoid relying
+  /// on this field for other purposes.
+  std::string InstallDir;
+
   /// The directory used for the module cache.
   std::string ModuleCachePath;
 
   /// The directory used for a user build.
   std::string ModuleUserBuildPath;
 
   /// The mapping of module names to prebuilt module files.
   std::map<std::string, std::string> PrebuiltModuleFiles;

cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp

 //===--- CreateInvocationFromCommandLine.cpp - CompilerInvocation from Args ==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Construct a compiler invocation object for command line driver arguments
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/Frontend/Utils.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Driver/Action.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
-#include "clang/Driver/Action.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
+#include "clang/Frontend/Utils.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/Path.h"
 using namespace clang;
 using namespace llvm::opt;
 
 /// createInvocationFromCommandLine - Construct a compiler invocation object for
 /// a command line argument vector.
 ///
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
   const ArgStringList &CCArgs = Cmd.getArguments();
   auto CI = llvm::make_unique<CompilerInvocation>();
   if (!CompilerInvocation::CreateFromArgs(*CI,
                                      const_cast<const char **>(CCArgs.data()),
                                      const_cast<const char **>(CCArgs.data()) +
                                      CCArgs.size(),
                                      *Diags))
     return nullptr;
+  // Patch up the install dir, so we find the same standard library as the
+  // original compiler on MacOS.
+  CI->getHeaderSearchOpts().InstallDir = TheDriver.getInstalledDir();
   return CI;
 }

cfe/trunk/lib/Frontend/InitHeaderSearch.cpp

void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang,
   }
 
   if (Lang.CPlusPlus && !Lang.AsmPreprocessor &&
       HSOpts.UseStandardCXXIncludes && HSOpts.UseStandardSystemIncludes) {
     if (HSOpts.UseLibcxx) {
       if (triple.isOSDarwin()) {
         // On Darwin, libc++ may be installed alongside the compiler in
         // include/c++/v1.
-        if (!HSOpts.ResourceDir.empty()) {
-          // Remove version from foo/lib/clang/version
-          StringRef NoVer = llvm::sys::path::parent_path(HSOpts.ResourceDir);
-          // Remove clang from foo/lib/clang
-          StringRef Lib = llvm::sys::path::parent_path(NoVer);
-          // Remove lib from foo/lib
-          SmallString<128> P = llvm::sys::path::parent_path(Lib);
-
+        if (!HSOpts.InstallDir.empty()) {
+          // Get from foo/bin to foo.
+          SmallString<128> P(llvm::sys::path::parent_path(HSOpts.InstallDir));
           // Get foo/include/c++/v1
           llvm::sys::path::append(P, "include", "c++", "v1");
           AddUnmappedPath(P, CXXSystem, false);
         }
       }
       AddPath("/usr/include/c++/v1", CXXSystem, false);
     } else {
       AddDefaultCPlusPlusIncludePaths(Lang, triple, HSOpts);

cfe/trunk/lib/Tooling/Tooling.cpp

bool ToolInvocation::run() {
   // FIXME: remove this when all users have migrated!
   for (const auto &It : MappedFileContents) {
     // Inject the code as the given file name into the preprocessor options.
     std::unique_ptr<llvm::MemoryBuffer> Input =
         llvm::MemoryBuffer::getMemBuffer(It.getValue());
     Invocation->getPreprocessorOpts().addRemappedFile(It.getKey(),
                                                       Input.release());
   }
+  // Patch up the install dir, so we find the same standard library as the
+  // original compiler on MacOS.
+  Invocation->getHeaderSearchOpts().InstallDir = Driver->getInstalledDir();
   return runInvocation(BinaryName, Compilation.get(), std::move(Invocation),
                        std::move(PCHContainerOps));
 }
 
 bool ToolInvocation::runInvocation(
     const char *BinaryName, driver::Compilation *Compilation,
     std::shared_ptr<CompilerInvocation> Invocation,
     std::shared_ptr<PCHContainerOperations> PCHContainerOps) {

cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector

+class vector {};

cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp

+// Clang on MacOS can find libc++ living beside the installed compiler.
+// This test makes sure our libTooling-based tools emulate this properly.
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+//
+// Install the mock libc++ (simulates the libc++ directory structure).
+// RUN: cp -r %S/Inputs/mock-libcxx %t/
+//
+// Pretend clang is installed beside the mock library that we provided.
+// RUN: echo '[{"directory":"%t","command":"%t/mock-libcxx/bin/clang++ -stdlib=libc++ -target x86_64-apple-darwin -c test.cpp","file":"test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// clang-check will produce an error code if the mock library is not found.
+// RUN: clang-check -p "%t" "%t/test.cpp"
+
+#include <mock_vector>
+vector v;

cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp

+// Clang on MacOS can find libc++ living beside the installed compiler.
+// This test makes sure our libTooling-based tools emulate this properly.
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+//
+// Install the mock libc++ (simulates the libc++ directory structure).
+// RUN: cp -r %S/Inputs/mock-libcxx %t/
+//
+// Pretend clang is installed beside the mock library that we provided.
+// RUN: echo '[{"directory":"%t","command":"mock-libcxx/bin/clang++ -stdlib=libc++ -target x86_64-apple-darwin -c test.cpp","file":"test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// clang-check will produce an error code if the mock library is not found.
+// RUN: clang-check -p "%t" "%t/test.cpp"
+
+#include <mock_vector>
+vector v;