Skip to content

Commit fc56069

Browse files
JDevliegherefelipepiovezan
authored andcommitted
[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. (cherry picked from commit 74727a4)
1 parent 7c71610 commit fc56069

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
@@ -486,22 +486,25 @@ static Error createBundleDir(StringRef BundleBase) {
486486
}
487487

488488
static bool verifyOutput(StringRef OutputFile, StringRef Arch,
489-
DsymutilOptions Options) {
489+
DsymutilOptions Options, std::mutex &Mutex) {
490490

491491
if (OutputFile == "-") {
492+
std::lock_guard<std::mutex> Guard(Mutex);
492493
WithColor::warning() << "verification skipped for " << Arch
493494
<< " because writing to stdout.\n";
494495
return true;
495496
}
496497

497498
if (Options.LinkOpts.NoOutput) {
499+
std::lock_guard<std::mutex> Guard(Mutex);
498500
WithColor::warning() << "verification skipped for " << Arch
499501
<< " because --no-output was passed.\n";
500502
return true;
501503
}
502504

503505
Expected<OwningBinary<Binary>> BinOrErr = createBinary(OutputFile);
504506
if (!BinOrErr) {
507+
std::lock_guard<std::mutex> Guard(Mutex);
505508
WithColor::error() << OutputFile << ": " << toString(BinOrErr.takeError());
506509
return false;
507510
}
@@ -510,16 +513,27 @@ static bool verifyOutput(StringRef OutputFile, StringRef Arch,
510513
if (auto *Obj = dyn_cast<MachOObjectFile>(&Binary)) {
511514
std::unique_ptr<DWARFContext> DICtx = DWARFContext::create(*Obj);
512515
if (DICtx->getMaxVersion() >= 5) {
516+
std::lock_guard<std::mutex> Guard(Mutex);
513517
WithColor::warning() << "verification skipped for " << Arch
514518
<< " because DWARFv5 is not fully supported yet.\n";
515519
return true;
516520
}
517-
raw_ostream &os = Options.LinkOpts.Verbose ? errs() : nulls();
518-
os << "Verifying DWARF for architecture: " << Arch << "\n";
521+
522+
if (Options.LinkOpts.Verbose) {
523+
std::lock_guard<std::mutex> Guard(Mutex);
524+
errs() << "Verifying DWARF for architecture: " << Arch << "\n";
525+
}
526+
527+
std::string Buffer;
528+
raw_string_ostream OS(Buffer);
529+
519530
DIDumpOptions DumpOpts;
520-
bool success = DICtx->verify(os, DumpOpts.noImplicitRecursion());
521-
if (!success)
531+
bool success = DICtx->verify(OS, DumpOpts.noImplicitRecursion());
532+
if (!success) {
533+
std::lock_guard<std::mutex> Guard(Mutex);
534+
errs() << OS.str();
522535
WithColor::error() << "output verification failed for " << Arch << '\n';
536+
}
523537
return success;
524538
}
525539

@@ -728,16 +742,16 @@ int main(int argc, char **argv) {
728742
!Options.DumpDebugMap && (Options.OutputFile != "-") &&
729743
(DebugMapPtrsOrErr->size() != 1 || Options.LinkOpts.Update);
730744

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

739748
std::mutex ErrorHandlerMutex;
740749

750+
// Set up a crash recovery context.
751+
CrashRecoveryContext::Enable();
752+
CrashRecoveryContext CRC;
753+
CRC.DumpStackAndCleanupOnFailure = true;
754+
741755
const bool Crashed = !CRC.RunSafely([&]() {
742756
for (auto &Map : *DebugMapPtrsOrErr) {
743757
if (Options.LinkOpts.Verbose || Options.DumpDebugMap)
@@ -749,11 +763,13 @@ int main(int argc, char **argv) {
749763
if (!Options.SymbolMap.empty())
750764
Options.LinkOpts.Translator = SymMapLoader.Load(InputFile, *Map);
751765

752-
if (Map->begin() == Map->end())
766+
if (Map->begin() == Map->end()) {
767+
std::lock_guard<std::mutex> Guard(ErrorHandlerMutex);
753768
WithColor::warning()
754769
<< "no debug symbols in executable (-arch "
755770
<< MachOUtils::getArchName(Map->getTriple().getArchName())
756771
<< ")\n";
772+
}
757773

758774
// Using a std::shared_ptr rather than std::unique_ptr because move-only
759775
// types don't work with std::bind in the ThreadPool implementation.
@@ -765,6 +781,7 @@ int main(int argc, char **argv) {
765781

766782
auto E = TempFiles.back().createTempFile();
767783
if (E) {
784+
std::lock_guard<std::mutex> Guard(ErrorHandlerMutex);
768785
WithColor::error() << toString(std::move(E));
769786
AllOK.fetch_and(false);
770787
return;
@@ -795,8 +812,9 @@ int main(int argc, char **argv) {
795812
if (flagIsSet(Options.Verify, DWARFVerify::Output) ||
796813
(flagIsSet(Options.Verify, DWARFVerify::OutputOnValidInput) &&
797814
!Linker.InputVerificationFailed())) {
798-
AllOK.fetch_and(verifyOutput(
799-
OutputFile, Map->getTriple().getArchName(), Options));
815+
AllOK.fetch_and(verifyOutput(OutputFile,
816+
Map->getTriple().getArchName(),
817+
Options, ErrorHandlerMutex));
800818
}
801819
};
802820

0 commit comments

Comments
 (0)