Skip to content

Commit 74727a4

Browse files
committed
[dsymutil] Fix stdio data races (NFC)
When processing multiple architectures in parallel, we need to protect access to stdio with a mutex. We already have a mutex for that purpose, but it was only used in the DWARFLinker. This patch protects writes to stdio in driver.
1 parent 7ba37f4 commit 74727a4

File tree

1 file changed

+31
-13
lines changed

1 file changed

+31
-13
lines changed

llvm/tools/dsymutil/dsymutil.cpp

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -491,22 +491,25 @@ static Error createBundleDir(StringRef BundleBase) {
491491
}
492492

493493
static bool verifyOutput(StringRef OutputFile, StringRef Arch,
494-
DsymutilOptions Options) {
494+
DsymutilOptions Options, std::mutex &Mutex) {
495495

496496
if (OutputFile == "-") {
497+
std::lock_guard<std::mutex> Guard(Mutex);
497498
WithColor::warning() << "verification skipped for " << Arch
498499
<< " because writing to stdout.\n";
499500
return true;
500501
}
501502

502503
if (Options.LinkOpts.NoOutput) {
504+
std::lock_guard<std::mutex> Guard(Mutex);
503505
WithColor::warning() << "verification skipped for " << Arch
504506
<< " because --no-output was passed.\n";
505507
return true;
506508
}
507509

508510
Expected<OwningBinary<Binary>> BinOrErr = createBinary(OutputFile);
509511
if (!BinOrErr) {
512+
std::lock_guard<std::mutex> Guard(Mutex);
510513
WithColor::error() << OutputFile << ": " << toString(BinOrErr.takeError());
511514
return false;
512515
}
@@ -515,16 +518,27 @@ static bool verifyOutput(StringRef OutputFile, StringRef Arch,
515518
if (auto *Obj = dyn_cast<MachOObjectFile>(&Binary)) {
516519
std::unique_ptr<DWARFContext> DICtx = DWARFContext::create(*Obj);
517520
if (DICtx->getMaxVersion() >= 5) {
521+
std::lock_guard<std::mutex> Guard(Mutex);
518522
WithColor::warning() << "verification skipped for " << Arch
519523
<< " because DWARFv5 is not fully supported yet.\n";
520524
return true;
521525
}
522-
raw_ostream &os = Options.LinkOpts.Verbose ? errs() : nulls();
523-
os << "Verifying DWARF for architecture: " << Arch << "\n";
526+
527+
if (Options.LinkOpts.Verbose) {
528+
std::lock_guard<std::mutex> Guard(Mutex);
529+
errs() << "Verifying DWARF for architecture: " << Arch << "\n";
530+
}
531+
532+
std::string Buffer;
533+
raw_string_ostream OS(Buffer);
534+
524535
DIDumpOptions DumpOpts;
525-
bool success = DICtx->verify(os, DumpOpts.noImplicitRecursion());
526-
if (!success)
536+
bool success = DICtx->verify(OS, DumpOpts.noImplicitRecursion());
537+
if (!success) {
538+
std::lock_guard<std::mutex> Guard(Mutex);
539+
errs() << OS.str();
527540
WithColor::error() << "output verification failed for " << Arch << '\n';
541+
}
528542
return success;
529543
}
530544

@@ -730,16 +744,16 @@ int dsymutil_main(int argc, char **argv, const llvm::ToolContext &) {
730744
!Options.DumpDebugMap && (Options.OutputFile != "-") &&
731745
(DebugMapPtrsOrErr->size() != 1 || Options.LinkOpts.Update);
732746

733-
// Set up a crash recovery context.
734-
CrashRecoveryContext::Enable();
735-
CrashRecoveryContext CRC;
736-
CRC.DumpStackAndCleanupOnFailure = true;
737-
738747
std::atomic_char AllOK(1);
739748
SmallVector<MachOUtils::ArchAndFile, 4> TempFiles;
740749

741750
std::mutex ErrorHandlerMutex;
742751

752+
// Set up a crash recovery context.
753+
CrashRecoveryContext::Enable();
754+
CrashRecoveryContext CRC;
755+
CRC.DumpStackAndCleanupOnFailure = true;
756+
743757
const bool Crashed = !CRC.RunSafely([&]() {
744758
for (auto &Map : *DebugMapPtrsOrErr) {
745759
if (Options.LinkOpts.Verbose || Options.DumpDebugMap)
@@ -751,11 +765,13 @@ int dsymutil_main(int argc, char **argv, const llvm::ToolContext &) {
751765
if (!Options.SymbolMap.empty())
752766
Options.LinkOpts.Translator = SymMapLoader.Load(InputFile, *Map);
753767

754-
if (Map->begin() == Map->end())
768+
if (Map->begin() == Map->end()) {
769+
std::lock_guard<std::mutex> Guard(ErrorHandlerMutex);
755770
WithColor::warning()
756771
<< "no debug symbols in executable (-arch "
757772
<< MachOUtils::getArchName(Map->getTriple().getArchName())
758773
<< ")\n";
774+
}
759775

760776
// Using a std::shared_ptr rather than std::unique_ptr because move-only
761777
// types don't work with std::bind in the ThreadPool implementation.
@@ -767,6 +783,7 @@ int dsymutil_main(int argc, char **argv, const llvm::ToolContext &) {
767783

768784
auto E = TempFiles.back().createTempFile();
769785
if (E) {
786+
std::lock_guard<std::mutex> Guard(ErrorHandlerMutex);
770787
WithColor::error() << toString(std::move(E));
771788
AllOK.fetch_and(false);
772789
return;
@@ -797,8 +814,9 @@ int dsymutil_main(int argc, char **argv, const llvm::ToolContext &) {
797814
if (flagIsSet(Options.Verify, DWARFVerify::Output) ||
798815
(flagIsSet(Options.Verify, DWARFVerify::OutputOnValidInput) &&
799816
!Linker.InputVerificationFailed())) {
800-
AllOK.fetch_and(verifyOutput(
801-
OutputFile, Map->getTriple().getArchName(), Options));
817+
AllOK.fetch_and(verifyOutput(OutputFile,
818+
Map->getTriple().getArchName(),
819+
Options, ErrorHandlerMutex));
802820
}
803821
};
804822

0 commit comments

Comments
 (0)