Make clang/test/Index/pch-from-libclang.c pass in more places

[thakis]

Summary

• fixes the test on macOS with LLVM_ENABLE_PIC=OFF
• together with D57343, gets the test to pass on Windows
• makes it run everywhere (it seems to just pass on Linux)

The main change is to pull out the resource directory computation into a function shared by all 3 places that do it. In CIndexer.cpp, this now works no matter if libclang is in lib/ or bin/ or statically linked to a binary in bin/.

[thakis]

arphaman: ping?

[thakis]

erik.pilkington, can you take a look? Looks like arphaman isn't around. Since this touches libclang, I figure it might make sense if one of you takes a look.

[erik.pilkington]

Sorry, this really seems more like @arphaman's wheelhouse. I'll ping him to take a look though.

[arphaman]

Makes sense. The comments aren't really enforceable though, can we just factor this out into one function in lib/Basic that can be shared between all sites? It seems like there's nothing that would prevent that.

[thakis]

Could it be somewhere in lib/Driver? libclang depends on lib/Frontend which depends on lib/Driver -- is there any reason libclang can't depend on lib/Driver?

[arphaman]

Yes, that should work.

[thakis]

shared function

[arphaman]

LGTM

[sbc100]

I think this broke the BUILD_SHARED_LIBS build: https://reviews.llvm.org/D57556

clang/include/clang/Driver/Driver.h

private:
   ///                           in case the search or copy fails.
   ///
   /// \returns If the .crash is found and successfully copied return true,
   /// otherwise false and return the suggested directory in \p CrashDiagDir.
   bool getCrashDiagnosticFile(StringRef ReproCrashFilename,
                               SmallString<128> &CrashDiagDir);
 
 public:
+
+  /// Takes the path to a binary that's either in bin/ or lib/ and returns
+  /// the path to clang's resource directory.
+  static std::string GetResourcesPath(StringRef BinaryPath,
+                                      StringRef CustomResourceDir = "");
+
   Driver(StringRef ClangExecutable, StringRef TargetTriple,
          DiagnosticsEngine &Diags,
          IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr);
 
   /// @name Accessors
   /// @{
 
   /// Name to use when invoking gcc/g++.

clang/lib/Driver/Driver.cpp

 #include <unistd.h> // getpid
 #include <sysexits.h> // EX_IOERR
 #endif
 
 using namespace clang::driver;
 using namespace clang;
 using namespace llvm::opt;
 
+// static
+std::string Driver::GetResourcesPath(StringRef BinaryPath,
+                                     StringRef CustomResourceDir) {
+  // Since the resource directory is embedded in the module hash, it's important
+  // that all places that need it call this function, so that they get the
+  // exact same string ("a/../b/" and "b/" get different hashes, for example).
+
+  // Dir is bin/ or lib/, depending on where BinaryPath is.
+  std::string Dir = llvm::sys::path::parent_path(BinaryPath);
+
+  SmallString<128> P(Dir);
+  if (CustomResourceDir != "") {
+    llvm::sys::path::append(P, CustomResourceDir);
+  } else {
+    // On Windows, libclang.dll is in bin/.
+    // On non-Windows, libclang.so/.dylib is in lib/.
+    // With a static-library build of libclang, LibClangPath will contain the
+    // path of the embedding binary, which for LLVM binaries will be in bin/.
+    // ../lib gets us to lib/ in both cases.
+    P = llvm::sys::path::parent_path(Dir);
+    llvm::sys::path::append(P, Twine("lib") + CLANG_LIBDIR_SUFFIX, "clang",
+                            CLANG_VERSION_STRING);
+  }
+
+  return P.str();
+}
+
 Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
                DiagnosticsEngine &Diags,
                IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS)
     : Opts(createDriverOptTable()), Diags(Diags), VFS(std::move(VFS)),
       Mode(GCCMode), SaveTemps(SaveTempsNone), BitcodeEmbed(EmbedNone),
       LTOMode(LTOK_None), ClangExecutable(ClangExecutable),
       SysRoot(DEFAULT_SYSROOT), DriverTitle("clang LLVM compiler"),
       CCPrintOptionsFilename(nullptr), CCPrintHeadersFilename(nullptr),

 #if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
   SystemConfigDir = CLANG_CONFIG_FILE_SYSTEM_DIR;
 #endif
 #if defined(CLANG_CONFIG_FILE_USER_DIR)
   UserConfigDir = CLANG_CONFIG_FILE_USER_DIR;
 #endif
 
   // Compute the path to the resource directory.
-  StringRef ClangResourceDir(CLANG_RESOURCE_DIR);
-  SmallString<128> P(Dir);
-  if (ClangResourceDir != "") {
-    llvm::sys::path::append(P, ClangResourceDir);
-  } else {
-    StringRef ClangLibdirSuffix(CLANG_LIBDIR_SUFFIX);
-    P = llvm::sys::path::parent_path(Dir);
-    llvm::sys::path::append(P, Twine("lib") + ClangLibdirSuffix, "clang",
-                            CLANG_VERSION_STRING);
-  }
-  ResourceDir = P.str();
+  ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
 }
 
 void Driver::ParseDriverMode(StringRef ProgramName,
                              ArrayRef<const char *> Args) {
   if (ClangNameParts.isEmpty())
     ClangNameParts = ToolChain::getTargetAndModeFromProgramName(ProgramName);
   setDriverModeFromOption(ClangNameParts.DriverMode);
 

clang/lib/Frontend/CompilerInvocation.cpp

 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Sanitizers.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Basic/Version.h"
 #include "clang/Basic/Visibility.h"
 #include "clang/Basic/XRayInstr.h"
 #include "clang/Config/config.h"
+#include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/CommandLineSourceLoc.h"
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
 #include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
static InputKind ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
 
   return DashX;
 }
 
 std::string CompilerInvocation::GetResourcesPath(const char *Argv0,
                                                  void *MainAddr) {
   std::string ClangExecutable =
       llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
-  StringRef Dir = llvm::sys::path::parent_path(ClangExecutable);
-
-  // Compute the path to the resource directory.
-  StringRef ClangResourceDir(CLANG_RESOURCE_DIR);
-  SmallString<128> P(Dir);
-  if (ClangResourceDir != "")
-    llvm::sys::path::append(P, ClangResourceDir);
-  else
-    llvm::sys::path::append(P, "..", Twine("lib") + CLANG_LIBDIR_SUFFIX,
-                            "clang", CLANG_VERSION_STRING);
-
-  return P.str();
+  return Driver::GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
 }
 
 static void ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
                                   const std::string &WorkingDir) {
   Opts.Sysroot = Args.getLastArgValue(OPT_isysroot, "/");
   Opts.Verbose = Args.hasArg(OPT_v);
   Opts.UseBuiltinIncludes = !Args.hasArg(OPT_nobuiltininc);
   Opts.UseStandardSystemIncludes = !Args.hasArg(OPT_nostdsysteminc);

clang/test/Index/pch-from-libclang.c

 // Check that clang can use a PCH created from libclang.
 
-// FIXME: Non-darwin bots fail. Would need investigation using -module-file-info to see what is the difference in modules generated from libclang vs the compiler invocation, in those systems.
-// REQUIRES: system-darwin
+// This test doesn't use -fdisable-module-hash and hence requires that
+// CompilerInvocation::getModuleHash() computes exactly the same hash
+// for c-index-test and clang, which in turn requires that the both use
+// exactly the same resource-dir, even without calling realpath() on it:
+// - a/../b/ and b/ are not considered the same
+// - on Windows, c:\ and C:\ (only different in case) are not the same
 
 // RUN: %clang_cc1 -fsyntax-only %s -verify
 // RUN: c-index-test -write-pch %t.h.pch %s -fmodules -fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin
 // RUN: %clang -fsyntax-only -include %t.h %s -Xclang -verify -fmodules -fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -Xclang -triple -Xclang x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors
 // RUN: %clang -x c-header %s -o %t.clang.h.pch -fmodules -fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -Xclang -triple -Xclang x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors -Xclang -verify
 // RUN: c-index-test -test-load-source local %s -include %t.clang.h -fmodules -fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin | FileCheck %s
 
 #ifndef HEADER

clang/tools/libclang/CIndexer.cpp

 //===- CIndex.cpp - Clang-C Source Indexing Library -----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the Clang-C Source Indexing library.
 //
 //===----------------------------------------------------------------------===//
 
 #include "CIndexer.h"
 #include "CXString.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/Version.h"
+#include "clang/Driver/Driver.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/MutexGuard.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/YAMLParser.h"
 #include <cstdio>
#ifdef __CYGWIN__
   strcpy(w32path, path);
 #if CYGWIN_VERSION_API_MAJOR > 0 || CYGWIN_VERSION_API_MINOR >= 181
   cygwin_conv_path(CCP_WIN_A_TO_POSIX, w32path, path, MAX_PATH);
 #else
   cygwin_conv_to_full_posix_path(w32path, path);
 #endif
 #endif
 
-  LibClangPath += llvm::sys::path::parent_path(path);
+  LibClangPath += path;
 #else
   // This silly cast below avoids a C++ warning.
   Dl_info info;
   if (dladdr((void *)(uintptr_t)clang_createTranslationUnit, &info) == 0)
     llvm_unreachable("Call to dladdr() failed");
 
   // We now have the CIndex directory, locate clang relative to it.
-  LibClangPath += llvm::sys::path::parent_path(info.dli_fname);
+  LibClangPath += info.dli_fname;
 #endif
 
-  llvm::sys::path::append(LibClangPath, "clang", CLANG_VERSION_STRING);
-
   // Cache our result.
-  ResourcesPath = LibClangPath.str();
+  ResourcesPath = driver::Driver::GetResourcesPath(LibClangPath);
   return ResourcesPath;
 }
 
 StringRef CIndexer::getClangToolchainPath() {
   if (!ToolchainPath.empty())
     return ToolchainPath;
   StringRef ResourcePath = getClangResourcesPath();
   ToolchainPath = llvm::sys::path::parent_path(