Skip to content

Commit 861fcbd

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 (cherry picked from commit 4095d2d)
1 parent b6943cf commit 861fcbd

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
@@ -357,13 +357,14 @@ makeDepscanDaemonPath(StringRef Mode, const DepscanSharing &Sharing) {
357357
return std::nullopt;
358358
}
359359

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

368369
static Expected<llvm::cas::CASID> scanAndUpdateCC1InlineWithTool(
369370
tooling::dependencies::DependencyScanningTool &Tool,
@@ -372,14 +373,17 @@ static Expected<llvm::cas::CASID> scanAndUpdateCC1InlineWithTool(
372373
SmallVectorImpl<const char *> &OutputArgs, llvm::cas::ObjectStore &DB,
373374
llvm::function_ref<const char *(const Twine &)> SaveArg);
374375

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

384388
// FIXME: Skip some of this if -fcas-fs has been passed.
385389

@@ -389,12 +393,12 @@ static llvm::Expected<llvm::cas::CASID> scanAndUpdateCC1UsingDaemon(
389393
? ScanDaemon::connectToDaemonAndShakeHands(Path)
390394
: ScanDaemon::constructAndShakeHands(Path, Exec, Sharing);
391395
if (!Daemon)
392-
return Daemon.takeError();
396+
return reportScanFailure(Daemon.takeError());
393397
CC1DepScanDProtocol Comms(*Daemon);
394398

395399
// llvm::dbgs() << "sending request...\n";
396400
if (auto E = Comms.putCommand(WorkingDirectory, OldArgs))
397-
return std::move(E);
401+
return reportScanFailure(std::move(E));
398402

399403
llvm::BumpPtrAllocator Alloc;
400404
llvm::StringSaver Saver(Alloc);
@@ -403,23 +407,32 @@ static llvm::Expected<llvm::cas::CASID> scanAndUpdateCC1UsingDaemon(
403407
StringRef FailedReason;
404408
StringRef RootID;
405409
StringRef DiagOut;
406-
if (auto E = Comms.getScanResult(Saver, Result, FailedReason, RootID,
407-
RawNewArgs, DiagOut)) {
408-
DiagnosticOutput = DiagOut;
409-
return std::move(E);
410-
}
411-
DiagnosticOutput = DiagOut;
410+
auto E = Comms.getScanResult(Saver, Result, FailedReason, RootID, RawNewArgs,
411+
DiagOut);
412+
// Send the diagnostics to std::err.
413+
llvm::errs() << DiagOut;
414+
if (E)
415+
return reportScanFailure(std::move(E));
412416

413417
if (Result != CC1DepScanDProtocol::SuccessResult)
414-
return llvm::createStringError(llvm::inconvertibleErrorCode(),
415-
"depscan daemon failed: " + FailedReason);
418+
return reportScanFailure(
419+
llvm::createStringError("depscan daemon failed: " + FailedReason));
416420

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

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

425438
// FIXME: This is a copy of Command::writeResponseFile. Command is too deeply
@@ -446,8 +459,6 @@ static int scanAndUpdateCC1(const char *Exec, ArrayRef<const char *> OldArgs,
446459
DiagnosticsEngine &Diag,
447460
const llvm::opt::ArgList &Args,
448461
const CASOptions &CASOpts,
449-
std::shared_ptr<llvm::cas::ObjectStore> DB,
450-
std::shared_ptr<llvm::cas::ActionCache> Cache,
451462
std::optional<llvm::cas::CASID> &RootID) {
452463
using namespace clang::driver;
453464

@@ -513,25 +524,14 @@ static int scanAndUpdateCC1(const char *Exec, ArrayRef<const char *> OldArgs,
513524
if (ProduceIncludeTree)
514525
Sharing.CASArgs.push_back("-fdepscan-include-tree");
515526

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

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

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

602598
// 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.
@@ -1106,13 +1103,18 @@ static Expected<llvm::cas::CASID> scanAndUpdateCC1InlineWithTool(
11061103
return *Root;
11071104
}
11081105

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

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

0 commit comments

Comments
 (0)