Skip to content

Commit 4095d2d

Browse files
cachemeifyoucancachemeifyoucan
committed
[CAS] Delay CAS initialization on server side after daemon starts
For auto CAS validation and recovery, it is important that server does not open the CAS until daemon replies so daemon has the chance to validate and recovery from broken CAS if needed. Otherwise, this is going to be a dead lock on daemon waiting for server to close the CAS before deletion. rdar://155342429
1 parent 8b67bf5 commit 4095d2d

File tree

3 files changed

+84
-64
lines changed

3 files changed

+84
-64
lines changed

‎clang/test/CAS/daemon-cas-recovery.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// REQUIRES: system-darwin, clang-cc1daemon
2+
3+
// RUN: rm -rf %t && mkdir -p %t
4+
5+
/// Construct a malformed CAS to recovery from.
6+
// RUN: echo "abc" | llvm-cas --cas %t/cas --make-blob --data -
7+
// RUN: rm %t/cas/v1.1/v9.data
8+
// RUN: not llvm-cas --cas %t/cas --validate --check-hash
9+
10+
// RUN: env LLVM_CACHE_CAS_PATH=%t/cas LLVM_CAS_FORCE_VALIDATION=1 %clang-cache \
11+
// RUN: %clang -fsyntax-only -x c %s
12+
13+
int func(void);

‎clang/test/CAS/depscan-cas-log.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
// CHECK: [[PID1:[0-9]*]] {{[0-9]*}}: mmap '{{.*}}v9.index'
1414
// CHECK: [[PID1]] {{[0-9]*}}: create subtrie
1515

16-
// CHECK: [[PID2:[0-9]*]] {{[0-9]*}}: mmap '{{.*}}v9.index'
1716
// Even a minimal compilation involves at least 9 records for the cache key.
18-
// CHECK-COUNT-9: [[PID2]] {{[0-9]*}}: create record
17+
// CHECK-COUNT-9: [[PID1]] {{[0-9]*}}: create record
1918

20-
// CHECK: [[PID1]] {{[0-9]*}}: close mmap '{{.*}}v9.index'
19+
// CHECK: [[PID2:[0-9]*]] {{[0-9]*}}: mmap '{{.*}}v9.index'
20+
// CHECK: [[PID2]] {{[0-9]*}}: close mmap '{{.*}}v9.index'

‎clang/tools/driver/cc1depscan_main.cpp

Lines changed: 68 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -355,13 +355,14 @@ makeDepscanDaemonPath(StringRef Mode, const DepscanSharing &Sharing) {
355355
return std::nullopt;
356356
}
357357

358-
static Expected<llvm::cas::CASID> scanAndUpdateCC1Inline(
359-
const char *Exec, ArrayRef<const char *> InputArgs,
360-
StringRef WorkingDirectory, SmallVectorImpl<const char *> &OutputArgs,
361-
bool ProduceIncludeTree, bool &DiagnosticErrorOccurred,
362-
llvm::function_ref<const char *(const Twine &)> SaveArg,
363-
const CASOptions &CASOpts, std::shared_ptr<llvm::cas::ObjectStore> DB,
364-
std::shared_ptr<llvm::cas::ActionCache> Cache);
358+
static int
359+
scanAndUpdateCC1Inline(const char *Exec, ArrayRef<const char *> InputArgs,
360+
StringRef WorkingDirectory,
361+
SmallVectorImpl<const char *> &OutputArgs,
362+
bool ProduceIncludeTree,
363+
llvm::function_ref<const char *(const Twine &)> SaveArg,
364+
const CASOptions &CASOpts, DiagnosticsEngine &Diag,
365+
std::optional<llvm::cas::CASID> &RootID);
365366

366367
static Expected<llvm::cas::CASID> scanAndUpdateCC1InlineWithTool(
367368
tooling::dependencies::DependencyScanningTool &Tool,
@@ -370,14 +371,17 @@ static Expected<llvm::cas::CASID> scanAndUpdateCC1InlineWithTool(
370371
SmallVectorImpl<const char *> &OutputArgs, llvm::cas::ObjectStore &DB,
371372
llvm::function_ref<const char *(const Twine &)> SaveArg);
372373

373-
static llvm::Expected<llvm::cas::CASID> scanAndUpdateCC1UsingDaemon(
374+
static int scanAndUpdateCC1UsingDaemon(
374375
const char *Exec, ArrayRef<const char *> OldArgs,
375376
StringRef WorkingDirectory, SmallVectorImpl<const char *> &NewArgs,
376-
std::string &DiagnosticOutput, StringRef Path,
377-
const DepscanSharing &Sharing,
377+
StringRef Path, const DepscanSharing &Sharing, DiagnosticsEngine &Diag,
378378
llvm::function_ref<const char *(const Twine &)> SaveArg,
379-
llvm::cas::ObjectStore &CAS) {
379+
const CASOptions &CASOpts, std::optional<llvm::cas::CASID> &Root) {
380380
using namespace clang::cc1depscand;
381+
auto reportScanFailure = [&](Error E) {
382+
Diag.Report(diag::err_cas_depscan_failed) << std::move(E);
383+
return 1;
384+
};
381385

382386
// FIXME: Skip some of this if -fcas-fs has been passed.
383387

@@ -387,12 +391,12 @@ static llvm::Expected<llvm::cas::CASID> scanAndUpdateCC1UsingDaemon(
387391
? ScanDaemon::connectToDaemonAndShakeHands(Path)
388392
: ScanDaemon::constructAndShakeHands(Path, Exec, Sharing);
389393
if (!Daemon)
390-
return Daemon.takeError();
394+
return reportScanFailure(Daemon.takeError());
391395
CC1DepScanDProtocol Comms(*Daemon);
392396

393397
// llvm::dbgs() << "sending request...\n";
394398
if (auto E = Comms.putCommand(WorkingDirectory, OldArgs))
395-
return std::move(E);
399+
return reportScanFailure(std::move(E));
396400

397401
llvm::BumpPtrAllocator Alloc;
398402
llvm::StringSaver Saver(Alloc);
@@ -401,23 +405,32 @@ static llvm::Expected<llvm::cas::CASID> scanAndUpdateCC1UsingDaemon(
401405
StringRef FailedReason;
402406
StringRef RootID;
403407
StringRef DiagOut;
404-
if (auto E = Comms.getScanResult(Saver, Result, FailedReason, RootID,
405-
RawNewArgs, DiagOut)) {
406-
DiagnosticOutput = DiagOut;
407-
return std::move(E);
408-
}
409-
DiagnosticOutput = DiagOut;
408+
auto E = Comms.getScanResult(Saver, Result, FailedReason, RootID, RawNewArgs,
409+
DiagOut);
410+
// Send the diagnostics to std::err.
411+
llvm::errs() << DiagOut;
412+
if (E)
413+
return reportScanFailure(std::move(E));
410414

411415
if (Result != CC1DepScanDProtocol::SuccessResult)
412-
return llvm::createStringError(llvm::inconvertibleErrorCode(),
413-
"depscan daemon failed: " + FailedReason);
416+
return reportScanFailure(
417+
llvm::createStringError("depscan daemon failed: " + FailedReason));
414418

415419
// FIXME: Avoid this duplication.
416420
NewArgs.resize(RawNewArgs.size());
417421
for (int I = 0, E = RawNewArgs.size(); I != E; ++I)
418422
NewArgs[I] = SaveArg(RawNewArgs[I]);
419423

420-
return CAS.parseID(RootID);
424+
// Create CAS after daemon returns the result so daemon can perform corrupted
425+
// CAS recovery.
426+
auto [CAS, _] = CASOpts.getOrCreateDatabases(Diag);
427+
if (!CAS)
428+
return 1;
429+
430+
if (auto E = CAS->parseID(RootID).moveInto(Root))
431+
return reportScanFailure(std::move(E));
432+
433+
return 0;
421434
}
422435

423436
// FIXME: This is a copy of Command::writeResponseFile. Command is too deeply
@@ -444,8 +457,6 @@ static int scanAndUpdateCC1(const char *Exec, ArrayRef<const char *> OldArgs,
444457
DiagnosticsEngine &Diag,
445458
const llvm::opt::ArgList &Args,
446459
const CASOptions &CASOpts,
447-
std::shared_ptr<llvm::cas::ObjectStore> DB,
448-
std::shared_ptr<llvm::cas::ActionCache> Cache,
449460
std::optional<llvm::cas::CASID> &RootID) {
450461
using namespace clang::driver;
451462

@@ -511,25 +522,14 @@ static int scanAndUpdateCC1(const char *Exec, ArrayRef<const char *> OldArgs,
511522
if (ProduceIncludeTree)
512523
Sharing.CASArgs.push_back("-fdepscan-include-tree");
513524

514-
std::string DiagnosticOutput;
515-
bool DiagnosticErrorOccurred = false;
516-
auto ScanAndUpdate = [&]() {
517-
if (std::optional<std::string> DaemonPath =
518-
makeDepscanDaemonPath(Mode, Sharing))
519-
return scanAndUpdateCC1UsingDaemon(Exec, OldArgs, WorkingDirectory,
520-
NewArgs, DiagnosticOutput, *DaemonPath,
521-
Sharing, SaveArg, *DB);
522-
return scanAndUpdateCC1Inline(Exec, OldArgs, WorkingDirectory, NewArgs,
523-
ProduceIncludeTree, DiagnosticErrorOccurred,
524-
SaveArg, CASOpts, DB, Cache);
525-
};
526-
if (llvm::Error E = ScanAndUpdate().moveInto(RootID)) {
527-
Diag.Report(diag::err_cas_depscan_failed) << std::move(E);
528-
if (!DiagnosticOutput.empty())
529-
llvm::errs() << DiagnosticOutput;
530-
return 1;
531-
}
532-
return DiagnosticErrorOccurred;
525+
if (auto DaemonPath = makeDepscanDaemonPath(Mode, Sharing))
526+
return scanAndUpdateCC1UsingDaemon(Exec, OldArgs, WorkingDirectory, NewArgs,
527+
*DaemonPath, Sharing, Diag, SaveArg,
528+
CASOpts, RootID);
529+
530+
return scanAndUpdateCC1Inline(Exec, OldArgs, WorkingDirectory, NewArgs,
531+
ProduceIncludeTree, SaveArg, CASOpts, Diag,
532+
RootID);
533533
}
534534

535535
int cc1depscan_main(ArrayRef<const char *> Argv, const char *Argv0,
@@ -590,12 +590,8 @@ int cc1depscan_main(ArrayRef<const char *> Argv, const char *Argv0,
590590
CompilerInvocation::ParseCASArgs(CASOpts, ParsedCC1Args, Diags);
591591
CASOpts.ensurePersistentCAS();
592592

593-
auto [CAS, Cache] = CASOpts.getOrCreateDatabases(Diags);
594-
if (!CAS || !Cache)
595-
return 1;
596-
597593
if (int Ret = scanAndUpdateCC1(Argv0, CC1Args->getValues(), NewArgs, Diags,
598-
Args, CASOpts, CAS, Cache, RootID))
594+
Args, CASOpts, RootID))
599595
return Ret;
600596

601597
// FIXME: Use OutputBackend to OnDisk only now.
@@ -841,7 +837,8 @@ void ScanServer::start(bool Exclusive, ArrayRef<const char *> CASArgs) {
841837
ExitOnErr(llvm::cas::validateOnDiskUnifiedCASDatabasesIfNeeded(
842838
CASPath, /*CheckHash=*/true,
843839
/*AllowRecovery=*/true,
844-
/*Force=*/false, findLLVMCasBinary(Argv0, LLVMCasStorage)));
840+
/*Force=*/getenv("LLVM_CAS_FORCE_VALIDATION"),
841+
findLLVMCasBinary(Argv0, LLVMCasStorage)));
845842
});
846843

847844
// Check the pidfile.
@@ -1108,13 +1105,18 @@ static Expected<llvm::cas::CASID> scanAndUpdateCC1InlineWithTool(
11081105
return *Root;
11091106
}
11101107

1111-
static Expected<llvm::cas::CASID> scanAndUpdateCC1Inline(
1112-
const char *Exec, ArrayRef<const char *> InputArgs,
1113-
StringRef WorkingDirectory, SmallVectorImpl<const char *> &OutputArgs,
1114-
bool ProduceIncludeTree, bool &DiagnosticErrorOccurred,
1115-
llvm::function_ref<const char *(const Twine &)> SaveArg,
1116-
const CASOptions &CASOpts, std::shared_ptr<llvm::cas::ObjectStore> DB,
1117-
std::shared_ptr<llvm::cas::ActionCache> Cache) {
1108+
static int
1109+
scanAndUpdateCC1Inline(const char *Exec, ArrayRef<const char *> InputArgs,
1110+
StringRef WorkingDirectory,
1111+
SmallVectorImpl<const char *> &OutputArgs,
1112+
bool ProduceIncludeTree,
1113+
llvm::function_ref<const char *(const Twine &)> SaveArg,
1114+
const CASOptions &CASOpts, DiagnosticsEngine &Diag,
1115+
std::optional<llvm::cas::CASID> &RootID) {
1116+
auto [DB, Cache] = CASOpts.getOrCreateDatabases(Diag);
1117+
if (!DB || !Cache)
1118+
return 1;
1119+
11181120
IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> FS;
11191121
if (!ProduceIncludeTree)
11201122
FS = llvm::cantFail(llvm::cas::createCachingOnDiskFileSystem(*DB));
@@ -1138,10 +1140,15 @@ static Expected<llvm::cas::CASID> scanAndUpdateCC1Inline(
11381140
auto DiagsConsumer =
11391141
std::make_unique<TextDiagnosticPrinter>(llvm::errs(), *DiagOpts, false);
11401142

1141-
auto Result = scanAndUpdateCC1InlineWithTool(
1142-
Tool, *DiagsConsumer, /*VerboseOS*/ nullptr, Exec, InputArgs,
1143-
WorkingDirectory, OutputArgs, *DB, SaveArg);
1144-
DiagnosticErrorOccurred = DiagsConsumer->getNumErrors() != 0;
1145-
return Result;
1143+
auto E = scanAndUpdateCC1InlineWithTool(
1144+
Tool, *DiagsConsumer, /*VerboseOS*/ nullptr, Exec, InputArgs,
1145+
WorkingDirectory, OutputArgs, *DB, SaveArg)
1146+
.moveInto(RootID);
1147+
if (E) {
1148+
Diag.Report(diag::err_cas_depscan_failed) << std::move(E);
1149+
return 1;
1150+
}
1151+
1152+
return DiagsConsumer->getNumErrors() != 0;
11461153
}
11471154
#endif /* LLVM_ON_UNIX */

0 commit comments

Comments
 (0)