Skip to content

Commit 0abcafd

Browse files
committed
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
1 parent 8f6182f commit 0abcafd

File tree

5 files changed

+46
-30
lines changed

5 files changed

+46
-30
lines changed

clang/include/clang/Driver/Driver.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,12 @@ class Driver {
277277
SmallString<128> &CrashDiagDir);
278278

279279
public:
280+
281+
/// Takes the path to a binary that's either in bin/ or lib/ and returns
282+
/// the path to clang's resource directory.
283+
static std::string GetResourcesPath(StringRef BinaryPath,
284+
StringRef CustomResourceDir = "");
285+
280286
Driver(StringRef ClangExecutable, StringRef TargetTriple,
281287
DiagnosticsEngine &Diags,
282288
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr);

clang/lib/Driver/Driver.cpp

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,33 @@ using namespace clang::driver;
8989
using namespace clang;
9090
using namespace llvm::opt;
9191

92+
// static
93+
std::string Driver::GetResourcesPath(StringRef BinaryPath,
94+
StringRef CustomResourceDir) {
95+
// Since the resource directory is embedded in the module hash, it's important
96+
// that all places that need it call this function, so that they get the
97+
// exact same string ("a/../b/" and "b/" get different hashes, for example).
98+
99+
// Dir is bin/ or lib/, depending on where BinaryPath is.
100+
std::string Dir = llvm::sys::path::parent_path(BinaryPath);
101+
102+
SmallString<128> P(Dir);
103+
if (CustomResourceDir != "") {
104+
llvm::sys::path::append(P, CustomResourceDir);
105+
} else {
106+
// On Windows, libclang.dll is in bin/.
107+
// On non-Windows, libclang.so/.dylib is in lib/.
108+
// With a static-library build of libclang, LibClangPath will contain the
109+
// path of the embedding binary, which for LLVM binaries will be in bin/.
110+
// ../lib gets us to lib/ in both cases.
111+
P = llvm::sys::path::parent_path(Dir);
112+
llvm::sys::path::append(P, Twine("lib") + CLANG_LIBDIR_SUFFIX, "clang",
113+
CLANG_VERSION_STRING);
114+
}
115+
116+
return P.str();
117+
}
118+
92119
Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
93120
DiagnosticsEngine &Diags,
94121
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS)
@@ -119,17 +146,7 @@ Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
119146
#endif
120147

121148
// Compute the path to the resource directory.
122-
StringRef ClangResourceDir(CLANG_RESOURCE_DIR);
123-
SmallString<128> P(Dir);
124-
if (ClangResourceDir != "") {
125-
llvm::sys::path::append(P, ClangResourceDir);
126-
} else {
127-
StringRef ClangLibdirSuffix(CLANG_LIBDIR_SUFFIX);
128-
P = llvm::sys::path::parent_path(Dir);
129-
llvm::sys::path::append(P, Twine("lib") + ClangLibdirSuffix, "clang",
130-
CLANG_VERSION_STRING);
131-
}
132-
ResourceDir = P.str();
149+
ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
133150
}
134151

135152
void Driver::ParseDriverMode(StringRef ProgramName,

clang/lib/Frontend/CompilerInvocation.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "clang/Basic/Visibility.h"
2727
#include "clang/Basic/XRayInstr.h"
2828
#include "clang/Config/config.h"
29+
#include "clang/Driver/Driver.h"
2930
#include "clang/Driver/DriverDiagnostic.h"
3031
#include "clang/Driver/Options.h"
3132
#include "clang/Frontend/CommandLineSourceLoc.h"
@@ -1893,18 +1894,7 @@ std::string CompilerInvocation::GetResourcesPath(const char *Argv0,
18931894
void *MainAddr) {
18941895
std::string ClangExecutable =
18951896
llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
1896-
StringRef Dir = llvm::sys::path::parent_path(ClangExecutable);
1897-
1898-
// Compute the path to the resource directory.
1899-
StringRef ClangResourceDir(CLANG_RESOURCE_DIR);
1900-
SmallString<128> P(Dir);
1901-
if (ClangResourceDir != "")
1902-
llvm::sys::path::append(P, ClangResourceDir);
1903-
else
1904-
llvm::sys::path::append(P, "..", Twine("lib") + CLANG_LIBDIR_SUFFIX,
1905-
"clang", CLANG_VERSION_STRING);
1906-
1907-
return P.str();
1897+
return Driver::GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
19081898
}
19091899

19101900
static void ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
// Check that clang can use a PCH created from libclang.
22

3-
// 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.
4-
// REQUIRES: system-darwin
3+
// This test doesn't use -fdisable-module-hash and hence requires that
4+
// CompilerInvocation::getModuleHash() computes exactly the same hash
5+
// for c-index-test and clang, which in turn requires that the both use
6+
// exactly the same resource-dir, even without calling realpath() on it:
7+
// - a/../b/ and b/ are not considered the same
8+
// - on Windows, c:\ and C:\ (only different in case) are not the same
59

610
// RUN: %clang_cc1 -fsyntax-only %s -verify
711
// RUN: c-index-test -write-pch %t.h.pch %s -fmodules -fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin

clang/tools/libclang/CIndexer.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "CXString.h"
1515
#include "clang/Basic/LLVM.h"
1616
#include "clang/Basic/Version.h"
17+
#include "clang/Driver/Driver.h"
1718
#include "llvm/ADT/STLExtras.h"
1819
#include "llvm/ADT/SmallString.h"
1920
#include "llvm/Support/MD5.h"
@@ -62,21 +63,19 @@ const std::string &CIndexer::getClangResourcesPath() {
6263
#endif
6364
#endif
6465

65-
LibClangPath += llvm::sys::path::parent_path(path);
66+
LibClangPath += path;
6667
#else
6768
// This silly cast below avoids a C++ warning.
6869
Dl_info info;
6970
if (dladdr((void *)(uintptr_t)clang_createTranslationUnit, &info) == 0)
7071
llvm_unreachable("Call to dladdr() failed");
7172

7273
// We now have the CIndex directory, locate clang relative to it.
73-
LibClangPath += llvm::sys::path::parent_path(info.dli_fname);
74+
LibClangPath += info.dli_fname;
7475
#endif
7576

76-
llvm::sys::path::append(LibClangPath, "clang", CLANG_VERSION_STRING);
77-
7877
// Cache our result.
79-
ResourcesPath = LibClangPath.str();
78+
ResourcesPath = driver::Driver::GetResourcesPath(LibClangPath);
8079
return ResourcesPath;
8180
}
8281

0 commit comments

Comments
 (0)