Skip to content

Commit 2db61c1

Browse files
committed
[clang][cas] Fix replay with include-tree relative paths
The include-tree embeds the relative path, not the path including the working directory, which may differ from the original invocation. Input paths should remain relative while only making output paths absolute based on the working directory. Fixes a crash when emitting diagnostics in a relative path during libclang replay. rdar://116013181 (cherry picked from commit 74f27a4)
1 parent 3097792 commit 2db61c1

File tree

4 files changed

+44
-33
lines changed

4 files changed

+44
-33
lines changed

clang/include/clang/Frontend/CompileJobCache.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class CompileJobCache {
8282
bool finishComputedResult(CompilerInstance &Clang, bool Success);
8383

8484
static llvm::Expected<std::optional<int>> replayCachedResult(
85-
std::shared_ptr<CompilerInvocation> Invok,
85+
std::shared_ptr<CompilerInvocation> Invok, StringRef WorkingDir,
8686
const llvm::cas::CASID &CacheKey,
8787
cas::CompileJobCacheResult &CachedResult, SmallVectorImpl<char> &DiagText);
8888

clang/lib/Frontend/CompileJobCache.cpp

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "llvm/CAS/CASOutputBackend.h"
2020
#include "llvm/RemoteCachingService/Client.h"
2121
#include "llvm/Support/FileOutputBuffer.h"
22+
#include "llvm/Support/Path.h"
2223
#include "llvm/Support/PrefixMapper.h"
2324
#include "llvm/Support/Process.h"
2425
#include "llvm/Support/ScopedDurationTimer.h"
@@ -34,7 +35,8 @@ class CompileJobCache::CachingOutputs {
3435
public:
3536
using OutputKind = clang::cas::CompileJobCacheResult::OutputKind;
3637

37-
CachingOutputs(CompilerInstance &Clang, llvm::PrefixMapper Mapper);
38+
CachingOutputs(CompilerInstance &Clang, StringRef Workingdir,
39+
llvm::PrefixMapper Mapper);
3840
virtual ~CachingOutputs() = default;
3941

4042
/// \returns true if result was found and replayed, false otherwise.
@@ -73,10 +75,11 @@ namespace {
7375
/// \p llvm::cas::ActionCache.
7476
class ObjectStoreCachingOutputs : public CompileJobCache::CachingOutputs {
7577
public:
76-
ObjectStoreCachingOutputs(CompilerInstance &Clang, llvm::PrefixMapper Mapper,
78+
ObjectStoreCachingOutputs(CompilerInstance &Clang, StringRef WorkingDir,
79+
llvm::PrefixMapper Mapper,
7780
std::shared_ptr<llvm::cas::ObjectStore> DB,
7881
std::shared_ptr<llvm::cas::ActionCache> Cache)
79-
: CachingOutputs(Clang, std::move(Mapper)), CAS(std::move(DB)),
82+
: CachingOutputs(Clang, WorkingDir, std::move(Mapper)), CAS(std::move(DB)),
8083
Cache(std::move(Cache)) {
8184
if (CAS)
8285
CASOutputs = llvm::makeIntrusiveRefCnt<llvm::cas::CASOutputBackend>(*CAS);
@@ -167,9 +170,10 @@ class CollectingOutputBackend : public llvm::vfs::ProxyOutputBackend {
167170
/// and \p llvm::cas::KeyValueDBClient.
168171
class RemoteCachingOutputs : public CompileJobCache::CachingOutputs {
169172
public:
170-
RemoteCachingOutputs(CompilerInstance &Clang, llvm::PrefixMapper Mapper,
173+
RemoteCachingOutputs(CompilerInstance &Clang, StringRef WorkingDir,
174+
llvm::PrefixMapper Mapper,
171175
llvm::cas::remote::ClientServices Clients)
172-
: CachingOutputs(Clang, std::move(Mapper)) {
176+
: CachingOutputs(Clang, WorkingDir, std::move(Mapper)) {
173177
RemoteKVClient = std::move(Clients.KVDB);
174178
RemoteCASClient = std::move(Clients.CASDB);
175179
CollectingOutputs = llvm::makeIntrusiveRefCnt<CollectingOutputBackend>();
@@ -219,13 +223,23 @@ CompileJobCache::CachingOutputs::getPathForOutputKind(OutputKind Kind) {
219223
}
220224
}
221225

222-
static std::string fixupRelativePath(const std::string &Path, FileManager &FM) {
226+
static std::string fixupRelativePath(const std::string &Path, FileManager &FM,
227+
StringRef WorkingDir) {
228+
if (llvm::sys::path::is_absolute(Path) || Path.empty() || Path == "-")
229+
return Path;
230+
231+
// Apply -working-dir compiler option.
223232
// FIXME: this needs to stay in sync with createOutputFileImpl. Ideally, clang
224233
// would create output files by their "kind" rather than by path.
225-
if (!Path.empty() && Path != "-" && !llvm::sys::path::is_absolute(Path)) {
226-
SmallString<128> PathStorage(Path);
227-
if (FM.FixupRelativePath(PathStorage))
228-
return std::string(PathStorage);
234+
SmallString<128> PathStorage(Path);
235+
if (FM.FixupRelativePath(PathStorage))
236+
return std::string(PathStorage);
237+
238+
// Apply "normal" working directory.
239+
if (!WorkingDir.empty()) {
240+
SmallString<128> Tmp(Path);
241+
llvm::sys::fs::make_absolute(WorkingDir, Tmp);
242+
return std::string(Tmp);
229243
}
230244
return Path;
231245
}
@@ -290,26 +304,27 @@ std::optional<int> CompileJobCache::initialize(CompilerInstance &Clang) {
290304
return reportCachingBackendError(Clang.getDiagnostics(),
291305
Clients.takeError());
292306
CacheBackend = std::make_unique<RemoteCachingOutputs>(
293-
Clang, std::move(PrefixMapper), std::move(*Clients));
307+
Clang, /*WorkingDir=*/"", std::move(PrefixMapper), std::move(*Clients));
294308
} else {
295309
CacheBackend = std::make_unique<ObjectStoreCachingOutputs>(
296-
Clang, std::move(PrefixMapper), CAS, Cache);
310+
Clang, /*WorkingDir=*/"", std::move(PrefixMapper), CAS, Cache);
297311
}
298312

299313
return std::nullopt;
300314
}
301315

302316
CompileJobCache::CachingOutputs::CachingOutputs(CompilerInstance &Clang,
317+
StringRef WorkingDir,
303318
llvm::PrefixMapper Mapper)
304319
: Clang(Clang), PrefixMapper(std::move(Mapper)) {
305320
CompilerInvocation &Invocation = Clang.getInvocation();
306321
FrontendOptions &FrontendOpts = Invocation.getFrontendOpts();
307322
if (!Clang.hasFileManager())
308323
Clang.createFileManager();
309324
FileManager &FM = Clang.getFileManager();
310-
OutputFile = fixupRelativePath(FrontendOpts.OutputFile, FM);
311-
DependenciesFile =
312-
fixupRelativePath(Invocation.getDependencyOutputOpts().OutputFile, FM);
325+
OutputFile = fixupRelativePath(FrontendOpts.OutputFile, FM, WorkingDir);
326+
DependenciesFile = fixupRelativePath(
327+
Invocation.getDependencyOutputOpts().OutputFile, FM, WorkingDir);
313328
DiagProcessor = std::make_unique<clang::cas::CachingDiagnosticsProcessor>(
314329
PrefixMapper, FM);
315330
}
@@ -491,8 +506,9 @@ bool CompileJobCache::finishComputedResult(CompilerInstance &Clang,
491506
}
492507

493508
Expected<std::optional<int>> CompileJobCache::replayCachedResult(
494-
std::shared_ptr<CompilerInvocation> Invok, const llvm::cas::CASID &CacheKey,
495-
cas::CompileJobCacheResult &CachedResult, SmallVectorImpl<char> &DiagText) {
509+
std::shared_ptr<CompilerInvocation> Invok, StringRef WorkingDir,
510+
const llvm::cas::CASID &CacheKey, cas::CompileJobCacheResult &CachedResult,
511+
SmallVectorImpl<char> &DiagText) {
496512
CompilerInstance Clang;
497513
Clang.setInvocation(std::move(Invok));
498514
llvm::raw_svector_ostream DiagOS(DiagText);
@@ -520,8 +536,9 @@ Expected<std::optional<int>> CompileJobCache::replayCachedResult(
520536

521537
assert(!Clang.getDiagnostics().hasErrorOccurred());
522538

523-
ObjectStoreCachingOutputs CachingOutputs(Clang, std::move(PrefixMapper),
524-
/*CAS*/ nullptr, /*Cache*/ nullptr);
539+
ObjectStoreCachingOutputs CachingOutputs(
540+
Clang, WorkingDir, std::move(PrefixMapper),
541+
/*CAS*/ nullptr, /*Cache*/ nullptr);
525542

526543
std::optional<int> Ret;
527544
if (Error E = CachingOutputs

clang/test/CAS/libclang-replay-job.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,11 @@
6868
// RUN: diff -u %t/t1.d %t/a/b/rel.d
6969
// FIXME: Get clang's `-working-directory` to affect relative path for serialized diagnostics.
7070

71+
// Use relative path to inputs and outputs.
7172
//--- cdb.json.template
7273
[{
7374
"directory": "DIR",
74-
"command": "clang -c DIR/main.c -target x86_64-apple-macos11 -MD -MF t1.d -MT deps -o output1.o",
75+
"command": "clang -c main.c -target x86_64-apple-macos11 -MD -MF t1.d -MT deps -o output1.o",
7576
"file": "DIR/main.c"
7677
}]
7778

clang/tools/libclang/CCAS.cpp

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -422,10 +422,7 @@ CXCASReplayResult clang_experimental_cas_replayCompilation(
422422

423423
SmallVector<const char *, 256> Args(argv, argv + argc);
424424
llvm::BumpPtrAllocator Alloc;
425-
bool CLMode = driver::IsClangCL(
426-
driver::getDriverMode(argv[0], ArrayRef(Args).slice(1)));
427-
428-
if (llvm::Error E = driver::expandResponseFiles(Args, CLMode, Alloc)) {
425+
if (llvm::Error E = driver::expandResponseFiles(Args, /*CLMode=*/false, Alloc)) {
429426
Diags.Report(diag::err_drv_expand_response_file)
430427
<< llvm::toString(std::move(E));
431428
if (OutError)
@@ -442,16 +439,12 @@ CXCASReplayResult clang_experimental_cas_replayCompilation(
442439
return nullptr;
443440
}
444441

445-
StringRef WorkingDir = WorkingDirectory;
446-
if (!WorkingDir.empty())
447-
Invok->getFileSystemOpts().WorkingDir = WorkingDir;
448-
449442
SmallString<256> DiagText;
450443
std::optional<int> Ret;
451-
if (Error E =
452-
CompileJobCache::replayCachedResult(std::move(Invok), WComp.CacheKey,
453-
WComp.CachedResult, DiagText)
454-
.moveInto(Ret)) {
444+
if (Error E = CompileJobCache::replayCachedResult(
445+
std::move(Invok), WorkingDirectory, WComp.CacheKey,
446+
WComp.CachedResult, DiagText)
447+
.moveInto(Ret)) {
455448
if (OutError)
456449
*OutError = cxerror::create(std::move(E));
457450
else

0 commit comments

Comments
 (0)