From 0abcafd8a48ddfcf80b41c20810d6f44de58fc24 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 31 Jan 2019 22:15:32 +0000 Subject: [PATCH] Make clang/test/Index/pch-from-libclang.c pass in more places - 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/. Differential Revision: https://reviews.llvm.org/D57345 llvm-svn: 352803 --- clang/include/clang/Driver/Driver.h | 6 ++++ clang/lib/Driver/Driver.cpp | 39 ++++++++++++++++------- clang/lib/Frontend/CompilerInvocation.cpp | 14 ++------ clang/test/Index/pch-from-libclang.c | 8 +++-- clang/tools/libclang/CIndexer.cpp | 9 +++--- 5 files changed, 46 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 0551230dad61..03e6458a5e5d 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -277,6 +277,12 @@ private: 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 VFS = nullptr); diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index e93d3e057754..0a89e2c45e19 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -89,6 +89,33 @@ 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 VFS) @@ -119,17 +146,7 @@ Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple, #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, diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index dbc29e24aee3..05cce3c90941 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -26,6 +26,7 @@ #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" @@ -1893,18 +1894,7 @@ 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, diff --git a/clang/test/Index/pch-from-libclang.c b/clang/test/Index/pch-from-libclang.c index 349fcac01eca..e71b61cddc31 100644 --- a/clang/test/Index/pch-from-libclang.c +++ b/clang/test/Index/pch-from-libclang.c @@ -1,7 +1,11 @@ // 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 diff --git a/clang/tools/libclang/CIndexer.cpp b/clang/tools/libclang/CIndexer.cpp index 28ec7abb37ae..03907884ac1a 100644 --- a/clang/tools/libclang/CIndexer.cpp +++ b/clang/tools/libclang/CIndexer.cpp @@ -14,6 +14,7 @@ #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" @@ -62,7 +63,7 @@ const std::string &CIndexer::getClangResourcesPath() { #endif #endif - LibClangPath += llvm::sys::path::parent_path(path); + LibClangPath += path; #else // This silly cast below avoids a C++ warning. Dl_info info; @@ -70,13 +71,11 @@ const std::string &CIndexer::getClangResourcesPath() { 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; }