[Index] Make sure c-index-test finds libc++ on Mac

[ilya-biryukov]

Summary

Specifically, the version of libc++ that lives alongside the c-index-test
binary. After r348365 driver relies on a correct value of argv[0] to find
libc++ and 'c-index-test' did not pass proper argv[0] when calling the
driver.

[ilya-biryukov]

I'm not sure who the owners of c-index-test are, so adding people who touched the file recently or might be interested in this landing.
Suggestions for other reviewers are very welcome.

Still struggling to make one test pass: Index/index-file.cu fails with:

.../index-file.cu:8:20: error: CHECK-HOST-NOT: excluded string found in input
// CHECK-HOST-NOT: macro definition=__CUDA_ARCH__
                   ^
<stdin>:3631:48: note: found here
// CHECK: __clang_cuda_runtime_wrapper.h:77:9: macro definition=__CUDA_ARCH__ Extent=[77:9 - 77:26]

The catch is that I see this line both after and before my change when I run the corresponding invocation of c-index-test with --cuda-host-only by hand.
Any ideas why this might be happening? How do I emulate the environment of llvm-lit completely to get a repro?

[thakis]

Looking at D54630, it seems arphaman and dexonsmith are probalby good reviewers for this.

[arphaman]

The real problem is that clang_parseTranslationUnit2 is broken, so this just fixes one client, c-index-test. All other macOS clients that call that function will still be unable to find the libc++ headers.

clang_parseTranslationUnit2 should try to infer the binary directory based on the libclang path. Libclang already does that for -resource-dir, see (CIndexer::getClangResourceDir). I think that you should set the clang path to CINdexer::getClangToolchainPath()/bin/clang, and that should fix the issue.

[ilya-biryukov]

• Revert "changes to main"
• Compute the path in parseTranslationUnit2

[ilya-biryukov]

@arphaman, thanks for pointing this out. It's definitely nice to change stuff only in one place.
There are two issues still pending:

• index-file.cu still fails, I haven't looked into it closely yet, any idea why that might be happening?; repro is simple, just patch this in locally and run the test.
• I'm not sure it's safe to use temporary storage for argv[0] in clang_parseTranslationUnit2. The resulting translation units probably holds references to argv and is free to read the data later. A simple and safe alternative would be to store this string in CIndexer and return a reference here. Does that LG?

[arphaman]

In D57532#1380383, @ilya-biryukov wrote:

@arphaman, thanks for pointing this out. It's definitely nice to change stuff only in one place.
There are two issues still pending:

• index-file.cu still fails, I haven't looked into it closely yet, any idea why that might be happening?; repro is simple, just patch this in locally and run the test.

No idea why it fails, I'll try to take a look.

• I'm not sure it's safe to use temporary storage for argv[0] in clang_parseTranslationUnit2. The resulting translation units probably holds references to argv and is free to read the data later. A simple and safe alternative would be to store this string in CIndexer and return a reference here. Does that LG?

Yes, that would be better. Let's store the string in CIndexer like we do for toolchain path and the resource dir.

[ilya-biryukov]

• Move the string to CIndexer.

[ilya-biryukov]

• Simplify code

[arphaman]

LGTM.

[ilya-biryukov]

Still need to figure out why the test fails and fix it before submitting the change.

[thakis]

ilya-biryukov, ping? :-)

[ilya-biryukov]

Lost this one after vacation, sorry.
Still need to figure out why the test is failing. Any help appreaciated

clang/test/Index/record-completion-invocation.c

 // RUN: rm -rf %t
 // RUN: mkdir %t
 // RUN: env CINDEXTEST_INVOCATION_EMISSION_PATH=%t not c-index-test -code-completion-at=%s:10:1 "-remap-file=%s,%S/Inputs/record-parsing-invocation-remap.c" %s
 // RUN: cat %t/libclang-* | FileCheck %s
 
 // RUN: rm -rf %t
 // RUN: mkdir %t
 // RUN: env LIBCLANG_DISABLE_CRASH_RECOVERY=1 CINDEXTEST_INVOCATION_EMISSION_PATH=%t not --crash c-index-test -code-completion-at=%s:10:1 "-remap-file=%s,%S/Inputs/record-parsing-invocation-remap.c" %s
 // RUN: cat %t/libclang-* | FileCheck %s
 
-// CHECK: {"toolchain":"{{.*}}","libclang.operation":"complete","libclang.opts":1,"args":["clang","-fno-spell-checking","{{.*}}record-completion-invocation.c","-Xclang","-detailed-preprocessing-record","-fallow-editor-placeholders"],"invocation-args":["-code-completion-at={{.*}}record-completion-invocation.c:10:1"],"unsaved_file_hashes":[{"name":"{{.*}}record-completion-invocation.c","md5":"aee23773de90e665992b48209351d70e"}]}
+// CHECK: {"toolchain":"{{.*}}","libclang.operation":"complete","libclang.opts":1,"args":["{{.*}}clang","-fno-spell-checking","{{.*}}record-completion-invocation.c","-Xclang","-detailed-preprocessing-record","-fallow-editor-placeholders"],"invocation-args":["-code-completion-at={{.*}}record-completion-invocation.c:10:1"],"unsaved_file_hashes":[{"name":"{{.*}}record-completion-invocation.c","md5":"aee23773de90e665992b48209351d70e"}]}

clang/test/Index/record-parsing-invocation.c

 // RUN: mkdir %t
 // RUN: env CINDEXTEST_INVOCATION_EMISSION_PATH=%t not c-index-test -test-load-source all "-remap-file=%s,%S/Inputs/record-parsing-invocation-remap.c" %s
 // RUN: cat %t/libclang-* | FileCheck --check-prefix=CHECK-UNSAVED %s
 
 #ifndef AVOID_CRASH
 #  pragma clang __debug parser_crash
 #endif
 
-// CHECK: {"toolchain":"{{.*}}","libclang.operation":"parse","libclang.opts":1,"args":["clang","-fno-spell-checking","{{.*}}record-parsing-invocation.c","-Xclang","-detailed-preprocessing-record","-fallow-editor-placeholders"]}
-// CHECK-UNSAVED: {"toolchain":"{{.*}}","libclang.operation":"parse","libclang.opts":1,"args":["clang","-fno-spell-checking","{{.*}}record-parsing-invocation.c","-Xclang","-detailed-preprocessing-record","-fallow-editor-placeholders"],"unsaved_file_hashes":[{"name":"{{.*}}record-parsing-invocation.c","md5":"aee23773de90e665992b48209351d70e"}]}
+// CHECK: {"toolchain":"{{.*}}","libclang.operation":"parse","libclang.opts":1,"args":["{{.*}}clang","-fno-spell-checking","{{.*}}record-parsing-invocation.c","-Xclang","-detailed-preprocessing-record","-fallow-editor-placeholders"]}
+// CHECK-UNSAVED: {"toolchain":"{{.*}}","libclang.operation":"parse","libclang.opts":1,"args":["{{.*}}clang","-fno-spell-checking","{{.*}}record-parsing-invocation.c","-Xclang","-detailed-preprocessing-record","-fallow-editor-placeholders"],"unsaved_file_hashes":[{"name":"{{.*}}record-parsing-invocation.c","md5":"aee23773de90e665992b48209351d70e"}]}

clang/tools/libclang/CIndex.cpp

clang_parseTranslationUnit(CXIndex CIdx,
   return TU;
 }
 
 enum CXErrorCode clang_parseTranslationUnit2(
     CXIndex CIdx, const char *source_filename,
     const char *const *command_line_args, int num_command_line_args,
     struct CXUnsavedFile *unsaved_files, unsigned num_unsaved_files,
     unsigned options, CXTranslationUnit *out_TU) {
+  if (!CIdx)
+    return CXError_InvalidArguments;
   SmallVector<const char *, 4> Args;
-  Args.push_back("clang");
+  Args.push_back(static_cast<CIndexer *>(CIdx)->getClangBinaryPath().c_str());
   Args.append(command_line_args, command_line_args + num_command_line_args);
   return clang_parseTranslationUnit2FullArgv(
       CIdx, source_filename, Args.data(), Args.size(), unsaved_files,
       num_unsaved_files, options, out_TU);
 }
 
 enum CXErrorCode clang_parseTranslationUnit2FullArgv(
     CXIndex CIdx, const char *source_filename,

clang/tools/libclang/CIndexer.h

class CIndexer {
   bool OnlyLocalDecls;
   bool DisplayDiagnostics;
   unsigned Options; // CXGlobalOptFlags.
 
   std::string ResourcesPath;
   std::shared_ptr<PCHContainerOperations> PCHContainerOps;
 
   std::string ToolchainPath;
+  std::string ClangBinaryPath;
 
   std::string InvocationEmissionPath;
 
 public:
   CIndexer(std::shared_ptr<PCHContainerOperations> PCHContainerOps =
                std::make_shared<PCHContainerOperations>())
       : OnlyLocalDecls(false), DisplayDiagnostics(false),
         Options(CXGlobalOpt_None), PCHContainerOps(std::move(PCHContainerOps)) {
   }
 
   /// Whether we only want to see "local" declarations (that did not
   /// come from a previous precompiled header). If false, we want to see all
   /// declarations.
   bool getOnlyLocalDecls() const { return OnlyLocalDecls; }
   void setOnlyLocalDecls(bool Local = true) { OnlyLocalDecls = Local; }
-  
   bool getDisplayDiagnostics() const { return DisplayDiagnostics; }
   void setDisplayDiagnostics(bool Display = true) {
     DisplayDiagnostics = Display;
   }
 
   std::shared_ptr<PCHContainerOperations> getPCHContainerOperations() const {
     return PCHContainerOps;
   }
 
   unsigned getCXGlobalOptFlags() const { return Options; }
   void setCXGlobalOptFlags(unsigned options) { Options = options; }
 
   bool isOptEnabled(CXGlobalOptFlags opt) const {
     return Options & opt;
   }
 
   /// Get the path of the clang resource files.
   const std::string &getClangResourcesPath();
 
   StringRef getClangToolchainPath();
 
+  /// Returns <ToolchainPath>/bin/clang.
+  const std::string &getClangBinaryPath();
+
   void setInvocationEmissionPath(StringRef Str) {
     InvocationEmissionPath = Str;
   }
 
   StringRef getInvocationEmissionPath() const { return InvocationEmissionPath; }
 };
 
 /// Logs information about a particular libclang operation like parsing to

clang/tools/libclang/CIndexer.cpp

StringRef CIndexer::getClangToolchainPath() {
   if (!ToolchainPath.empty())
     return ToolchainPath;
   StringRef ResourcePath = getClangResourcesPath();
   ToolchainPath = llvm::sys::path::parent_path(
       llvm::sys::path::parent_path(llvm::sys::path::parent_path(ResourcePath)));
   return ToolchainPath;
 }
 
+const std::string &CIndexer::getClangBinaryPath() {
+  if (!ClangBinaryPath.empty())
+    return ClangBinaryPath;
+  llvm::SmallString<128> Path;
+  llvm::sys::path::append(Path, getClangToolchainPath(), "bin", "clang");
+  ClangBinaryPath = Path.str();
+  return ClangBinaryPath;
+}
+
 LibclangInvocationReporter::LibclangInvocationReporter(
     CIndexer &Idx, OperationKind Op, unsigned ParseOptions,
     llvm::ArrayRef<const char *> Args,
     llvm::ArrayRef<std::string> InvocationArgs,
     llvm::ArrayRef<CXUnsavedFile> UnsavedFiles) {
   StringRef Path = Idx.getInvocationEmissionPath();
   if (Path.empty())
     return;