Skip to content

Commit 74f27a4

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
1 parent a838e29 commit 74f27a4

File tree

4 files changed

+49
-40
lines changed

4 files changed

+49
-40
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
bool WriteOutputAsCASID = false, bool UseCASBackend = false,

clang/lib/Frontend/CompileJobCache.cpp

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "llvm/MCCAS/MCCASObjectV1.h"
2222
#include "llvm/RemoteCachingService/Client.h"
2323
#include "llvm/Support/FileOutputBuffer.h"
24+
#include "llvm/Support/Path.h"
2425
#include "llvm/Support/PrefixMapper.h"
2526
#include "llvm/Support/Process.h"
2627
#include "llvm/Support/ScopedDurationTimer.h"
@@ -36,8 +37,9 @@ class CompileJobCache::CachingOutputs {
3637
public:
3738
using OutputKind = clang::cas::CompileJobCacheResult::OutputKind;
3839

39-
CachingOutputs(CompilerInstance &Clang, llvm::PrefixMapper Mapper,
40-
bool WriteOutputAsCASID, bool UseCASBackend);
40+
CachingOutputs(CompilerInstance &Clang, StringRef Workingdir,
41+
llvm::PrefixMapper Mapper, bool WriteOutputAsCASID,
42+
bool UseCASBackend);
4143
virtual ~CachingOutputs() = default;
4244

4345
/// \returns true if result was found and replayed, false otherwise.
@@ -78,12 +80,13 @@ namespace {
7880
/// \p llvm::cas::ActionCache.
7981
class ObjectStoreCachingOutputs : public CompileJobCache::CachingOutputs {
8082
public:
81-
ObjectStoreCachingOutputs(CompilerInstance &Clang, llvm::PrefixMapper Mapper,
82-
bool WriteOutputAsCASID, bool UseCASBackend,
83+
ObjectStoreCachingOutputs(CompilerInstance &Clang, StringRef WorkingDir,
84+
llvm::PrefixMapper Mapper, bool WriteOutputAsCASID,
85+
bool UseCASBackend,
8386
std::optional<llvm::cas::CASID> &MCOutputID,
8487
std::shared_ptr<llvm::cas::ObjectStore> DB,
8588
std::shared_ptr<llvm::cas::ActionCache> Cache)
86-
: CachingOutputs(Clang, std::move(Mapper), WriteOutputAsCASID,
89+
: CachingOutputs(Clang, WorkingDir, std::move(Mapper), WriteOutputAsCASID,
8790
UseCASBackend),
8891
ComputedJobNeedsReplay(WriteOutputAsCASID || UseCASBackend),
8992
MCOutputID(MCOutputID), CAS(std::move(DB)), Cache(std::move(Cache)) {
@@ -178,9 +181,10 @@ class CollectingOutputBackend : public llvm::vfs::ProxyOutputBackend {
178181
/// and \p llvm::cas::KeyValueDBClient.
179182
class RemoteCachingOutputs : public CompileJobCache::CachingOutputs {
180183
public:
181-
RemoteCachingOutputs(CompilerInstance &Clang, llvm::PrefixMapper Mapper,
184+
RemoteCachingOutputs(CompilerInstance &Clang, StringRef WorkingDir,
185+
llvm::PrefixMapper Mapper,
182186
llvm::cas::remote::ClientServices Clients)
183-
: CachingOutputs(Clang, std::move(Mapper),
187+
: CachingOutputs(Clang, WorkingDir, std::move(Mapper),
184188
/*WriteOutputAsCASID*/ false,
185189
/*UseCASBackend*/ false) {
186190
RemoteKVClient = std::move(Clients.KVDB);
@@ -232,13 +236,23 @@ CompileJobCache::CachingOutputs::getPathForOutputKind(OutputKind Kind) {
232236
}
233237
}
234238

235-
static std::string fixupRelativePath(const std::string &Path, FileManager &FM) {
239+
static std::string fixupRelativePath(const std::string &Path, FileManager &FM,
240+
StringRef WorkingDir) {
241+
if (llvm::sys::path::is_absolute(Path) || Path.empty() || Path == "-")
242+
return Path;
243+
244+
// Apply -working-dir compiler option.
236245
// FIXME: this needs to stay in sync with createOutputFileImpl. Ideally, clang
237246
// would create output files by their "kind" rather than by path.
238-
if (!Path.empty() && Path != "-" && !llvm::sys::path::is_absolute(Path)) {
239-
SmallString<128> PathStorage(Path);
240-
if (FM.FixupRelativePath(PathStorage))
241-
return std::string(PathStorage);
247+
SmallString<128> PathStorage(Path);
248+
if (FM.FixupRelativePath(PathStorage))
249+
return std::string(PathStorage);
250+
251+
// Apply "normal" working directory.
252+
if (!WorkingDir.empty()) {
253+
SmallString<128> Tmp(Path);
254+
llvm::sys::fs::make_absolute(WorkingDir, Tmp);
255+
return std::string(Tmp);
242256
}
243257
return Path;
244258
}
@@ -311,17 +325,18 @@ std::optional<int> CompileJobCache::initialize(CompilerInstance &Clang) {
311325
"combination of options not rejected earlier?");
312326
assert(!UseCASBackend && "combination of options not rejected earlier?");
313327
CacheBackend = std::make_unique<RemoteCachingOutputs>(
314-
Clang, std::move(PrefixMapper), std::move(*Clients));
328+
Clang, /*WorkingDir=*/"", std::move(PrefixMapper), std::move(*Clients));
315329
} else {
316330
CacheBackend = std::make_unique<ObjectStoreCachingOutputs>(
317-
Clang, std::move(PrefixMapper), CacheOpts.WriteOutputAsCASID,
318-
UseCASBackend, MCOutputID, CAS, Cache);
331+
Clang, /*WorkingDir=*/"", std::move(PrefixMapper),
332+
CacheOpts.WriteOutputAsCASID, UseCASBackend, MCOutputID, CAS, Cache);
319333
}
320334

321335
return std::nullopt;
322336
}
323337

324338
CompileJobCache::CachingOutputs::CachingOutputs(CompilerInstance &Clang,
339+
StringRef WorkingDir,
325340
llvm::PrefixMapper Mapper,
326341
bool WriteOutputAsCASID,
327342
bool UseCASBackend)
@@ -332,9 +347,9 @@ CompileJobCache::CachingOutputs::CachingOutputs(CompilerInstance &Clang,
332347
if (!Clang.hasFileManager())
333348
Clang.createFileManager();
334349
FileManager &FM = Clang.getFileManager();
335-
OutputFile = fixupRelativePath(FrontendOpts.OutputFile, FM);
336-
DependenciesFile =
337-
fixupRelativePath(Invocation.getDependencyOutputOpts().OutputFile, FM);
350+
OutputFile = fixupRelativePath(FrontendOpts.OutputFile, FM, WorkingDir);
351+
DependenciesFile = fixupRelativePath(
352+
Invocation.getDependencyOutputOpts().OutputFile, FM, WorkingDir);
338353
DiagProcessor = std::make_unique<clang::cas::CachingDiagnosticsProcessor>(
339354
PrefixMapper, FM);
340355
}
@@ -524,10 +539,10 @@ bool CompileJobCache::finishComputedResult(CompilerInstance &Clang,
524539
}
525540

526541
Expected<std::optional<int>> CompileJobCache::replayCachedResult(
527-
std::shared_ptr<CompilerInvocation> Invok, const llvm::cas::CASID &CacheKey,
528-
cas::CompileJobCacheResult &CachedResult, SmallVectorImpl<char> &DiagText,
529-
bool WriteOutputAsCASID, bool UseCASBackend,
530-
std::optional<llvm::cas::CASID> *OutMCOutputID) {
542+
std::shared_ptr<CompilerInvocation> Invok, StringRef WorkingDir,
543+
const llvm::cas::CASID &CacheKey, cas::CompileJobCacheResult &CachedResult,
544+
SmallVectorImpl<char> &DiagText, bool WriteOutputAsCASID,
545+
bool UseCASBackend, std::optional<llvm::cas::CASID> *OutMCOutputID) {
531546
CompilerInstance Clang;
532547
Clang.setInvocation(std::move(Invok));
533548
llvm::raw_svector_ostream DiagOS(DiagText);
@@ -556,10 +571,10 @@ Expected<std::optional<int>> CompileJobCache::replayCachedResult(
556571
assert(!Clang.getDiagnostics().hasErrorOccurred());
557572

558573
std::optional<llvm::cas::CASID> MCOutputID;
559-
ObjectStoreCachingOutputs CachingOutputs(Clang, std::move(PrefixMapper),
560-
WriteOutputAsCASID, UseCASBackend,
561-
MCOutputID,
562-
/*CAS*/ nullptr, /*Cache*/ nullptr);
574+
ObjectStoreCachingOutputs CachingOutputs(
575+
Clang, WorkingDir, std::move(PrefixMapper), WriteOutputAsCASID,
576+
UseCASBackend, MCOutputID,
577+
/*CAS*/ nullptr, /*Cache*/ nullptr);
563578
if (OutMCOutputID)
564579
*OutMCOutputID = std::move(MCOutputID);
565580

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
@@ -418,10 +418,7 @@ CXCASReplayResult clang_experimental_cas_replayCompilation(
418418

419419
SmallVector<const char *, 256> Args(argv, argv + argc);
420420
llvm::BumpPtrAllocator Alloc;
421-
bool CLMode = driver::IsClangCL(
422-
driver::getDriverMode(argv[0], ArrayRef(Args).slice(1)));
423-
424-
if (llvm::Error E = driver::expandResponseFiles(Args, CLMode, Alloc)) {
421+
if (llvm::Error E = driver::expandResponseFiles(Args, /*CLMode=*/false, Alloc)) {
425422
Diags.Report(diag::err_drv_expand_response_file)
426423
<< llvm::toString(std::move(E));
427424
if (OutError)
@@ -438,16 +435,12 @@ CXCASReplayResult clang_experimental_cas_replayCompilation(
438435
return nullptr;
439436
}
440437

441-
StringRef WorkingDir = WorkingDirectory;
442-
if (!WorkingDir.empty())
443-
Invok->getFileSystemOpts().WorkingDir = WorkingDir;
444-
445438
SmallString<256> DiagText;
446439
std::optional<int> Ret;
447-
if (Error E =
448-
CompileJobCache::replayCachedResult(std::move(Invok), WComp.CacheKey,
449-
WComp.CachedResult, DiagText)
450-
.moveInto(Ret)) {
440+
if (Error E = CompileJobCache::replayCachedResult(
441+
std::move(Invok), WorkingDirectory, WComp.CacheKey,
442+
WComp.CachedResult, DiagText)
443+
.moveInto(Ret)) {
451444
if (OutError)
452445
*OutError = cxerror::create(std::move(E));
453446
else

0 commit comments

Comments
 (0)