Index: llvm/test/tools/dsymutil/X86/reproducer.test =================================================================== --- llvm/test/tools/dsymutil/X86/reproducer.test +++ llvm/test/tools/dsymutil/X86/reproducer.test @@ -14,7 +14,7 @@ RUN: dsymutil -f -o - -oso-prepend-path=%t %t/Inputs/basic.macho.x86_64 | llvm-dwarfdump -a - | FileCheck %s # Create a reproducer. -RUN: env DSYMUTIL_REPRODUCER_PATH=%t.repro dsymutil -gen-reproducer -f -o %t.generate -oso-prepend-path=%t %t/Inputs/basic.macho.x86_64 | FileCheck %s --check-prefixes=REPRODUCER +RUN: env DSYMUTIL_REPRODUCER_PATH=%t.repro dsymutil -gen-reproducer -f -o %t.generate -oso-prepend-path=%t %t/Inputs/basic.macho.x86_64 2>&1 | FileCheck %s --check-prefixes=REPRODUCER RUN: llvm-dwarfdump -a %t.generate | FileCheck %s # Remove the input files and verify that was successful. @@ -75,6 +75,6 @@ CHECK-NEXT: DW_AT_byte_size (0x01) CHECK: NULL -REPRODUCER: reproducer written +REPRODUCER: Reproducer written ERROR: error: cannot parse the debug map CONFLICT: cannot combine --gen-reproducer and --use-reproducer Index: llvm/tools/dsymutil/Reproducer.h =================================================================== --- llvm/tools/dsymutil/Reproducer.h +++ llvm/tools/dsymutil/Reproducer.h @@ -17,7 +17,8 @@ /// The reproducer mode. enum class ReproducerMode { - Generate, + GenerateOnExit, + GenerateOnCrash, Use, Off, }; @@ -33,9 +34,11 @@ IntrusiveRefCntPtr getVFS() const { return VFS; } + virtual void generate(){}; + /// Create a Reproducer instance based on the given mode. static llvm::Expected> - createReproducer(ReproducerMode Mode, StringRef Root); + createReproducer(ReproducerMode Mode, StringRef Root, int Argc, char **Argv); protected: IntrusiveRefCntPtr VFS; @@ -46,15 +49,27 @@ /// dsymutil. class ReproducerGenerate : public Reproducer { public: - ReproducerGenerate(std::error_code &EC); + ReproducerGenerate(std::error_code &EC, int Argc, char **Argv, + bool GenerateOnExit); ~ReproducerGenerate() override; + virtual void generate() override; + private: /// The path to the reproducer. std::string Root; /// The FileCollector used by the FileCollectorFileSystem. std::shared_ptr FC; + + /// The input arguments to build the reproducer invocation. + llvm::SmallVector Args; + + /// Whether to generate the reproducer on destruction. + bool GenerateOnExit; + + /// Whether we already generated the reproducer. + bool Generated; }; /// Reproducer instance used to use an existing reproducer. The VFS returned by Index: llvm/tools/dsymutil/Reproducer.cpp =================================================================== --- llvm/tools/dsymutil/Reproducer.cpp +++ llvm/tools/dsymutil/Reproducer.cpp @@ -26,21 +26,38 @@ Reproducer::Reproducer() : VFS(vfs::getRealFileSystem()) {} Reproducer::~Reproducer() = default; -ReproducerGenerate::ReproducerGenerate(std::error_code &EC) - : Root(createReproducerDir(EC)) { +ReproducerGenerate::ReproducerGenerate(std::error_code &EC, int Argc, + char **Argv, bool GenerateOnExit) + : Root(createReproducerDir(EC)), GenerateOnExit(GenerateOnExit), + Generated(false) { + for (int I = 0; I < Argc; ++I) + Args.push_back(Argv[I]); if (!Root.empty()) FC = std::make_shared(Root, Root); VFS = FileCollector::createCollectorVFS(vfs::getRealFileSystem(), FC); } ReproducerGenerate::~ReproducerGenerate() { + if (GenerateOnExit && !Generated) + generate(); +} + +void ReproducerGenerate::generate() { if (!FC) return; + Generated = true; FC->copyFiles(false); SmallString<128> Mapping(Root); sys::path::append(Mapping, "mapping.yaml"); FC->writeMapping(Mapping.str()); - outs() << "reproducer written to " << Root << '\n'; + errs() << "********************\n"; + errs() << "Reproducer written to " << Root << '\n'; + errs() << "Please include the reproducer and the following invocation in " + "your bug report:\n"; + for (llvm::StringRef Arg : Args) + errs() << Arg << ' '; + errs() << "--use-reproducer " << Root << '\n'; + errs() << "********************\n"; } ReproducerUse::~ReproducerUse() = default; @@ -60,26 +77,26 @@ } llvm::Expected> -Reproducer::createReproducer(ReproducerMode Mode, StringRef Root) { +Reproducer::createReproducer(ReproducerMode Mode, StringRef Root, int Argc, + char **Argv) { + + std::error_code EC; + std::unique_ptr Repro; switch (Mode) { - case ReproducerMode::Generate: { - std::error_code EC; - std::unique_ptr Repro = - std::make_unique(EC); - if (EC) - return errorCodeToError(EC); - return std::move(Repro); - } - case ReproducerMode::Use: { - std::error_code EC; - std::unique_ptr Repro = - std::make_unique(Root, EC); - if (EC) - return errorCodeToError(EC); - return std::move(Repro); - } + case ReproducerMode::GenerateOnExit: + Repro = std::make_unique(EC, Argc, Argv, true); + break; + case ReproducerMode::GenerateOnCrash: + Repro = std::make_unique(EC, Argc, Argv, false); + break; + case ReproducerMode::Use: + Repro = std::make_unique(Root, EC); + break; case ReproducerMode::Off: - return std::make_unique(); + Repro = std::make_unique(); + break; } - llvm_unreachable("All cases handled above."); + if (EC) + return errorCodeToError(EC); + return Repro; } Index: llvm/tools/dsymutil/dsymutil.cpp =================================================================== --- llvm/tools/dsymutil/dsymutil.cpp +++ llvm/tools/dsymutil/dsymutil.cpp @@ -33,6 +33,7 @@ #include "llvm/Option/ArgList.h" #include "llvm/Option/Option.h" #include "llvm/Support/CommandLine.h" +#include "llvm/Support/CrashRecoveryContext.h" #include "llvm/Support/FileCollector.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/InitLLVM.h" @@ -111,7 +112,7 @@ std::vector InputFiles; unsigned NumThreads; DWARFVerify Verify = DWARFVerify::None; - ReproducerMode ReproMode = ReproducerMode::Off; + ReproducerMode ReproMode = ReproducerMode::GenerateOnCrash; dsymutil::LinkOptions LinkOpts; }; @@ -283,7 +284,7 @@ } if (Args.hasArg(OPT_gen_reproducer)) - Options.ReproMode = ReproducerMode::Generate; + Options.ReproMode = ReproducerMode::GenerateOnExit; if (Expected AccelKind = getAccelTableKind(Args)) { Options.LinkOpts.TheAccelTableKind = *AccelKind; @@ -569,8 +570,8 @@ InitializeAllTargets(); InitializeAllAsmPrinters(); - auto Repro = - Reproducer::createReproducer(Options.ReproMode, Options.ReproducerPath); + auto Repro = Reproducer::createReproducer(Options.ReproMode, + Options.ReproducerPath, argc, argv); if (!Repro) { WithColor::error() << toString(Repro.takeError()); return EXIT_FAILURE; @@ -662,72 +663,88 @@ VerifyOutput = false; } - SmallVector TempFiles; - std::atomic_char AllOK(1); - for (auto &Map : *DebugMapPtrsOrErr) { - if (Options.LinkOpts.Verbose || Options.DumpDebugMap) - Map->print(outs()); - - if (Options.DumpDebugMap) - continue; - - if (!Options.SymbolMap.empty()) - Options.LinkOpts.Translator = SymMapLoader.Load(InputFile, *Map); + // Set up a crash recovery context. + CrashRecoveryContext::Enable(); + CrashRecoveryContext CRC; + CRC.DumpStackAndCleanupOnFailure = true; - if (Map->begin() == Map->end()) - WithColor::warning() - << "no debug symbols in executable (-arch " - << MachOUtils::getArchName(Map->getTriple().getArchName()) << ")\n"; - - // Using a std::shared_ptr rather than std::unique_ptr because move-only - // types don't work with std::bind in the ThreadPool implementation. - std::shared_ptr OS; - - std::string OutputFile = OutputLocationOrErr->DWARFFile; - if (NeedsTempFiles) { - TempFiles.emplace_back(Map->getTriple().getArchName().str()); + std::atomic_char AllOK(1); + SmallVector TempFiles; - auto E = TempFiles.back().createTempFile(); - if (E) { - WithColor::error() << toString(std::move(E)); - return EXIT_FAILURE; + const bool Crashed = !CRC.RunSafely([&]() { + for (auto &Map : *DebugMapPtrsOrErr) { + if (Options.LinkOpts.Verbose || Options.DumpDebugMap) + Map->print(outs()); + + if (Options.DumpDebugMap) + continue; + + if (!Options.SymbolMap.empty()) + Options.LinkOpts.Translator = SymMapLoader.Load(InputFile, *Map); + + if (Map->begin() == Map->end()) + WithColor::warning() + << "no debug symbols in executable (-arch " + << MachOUtils::getArchName(Map->getTriple().getArchName()) + << ")\n"; + + // Using a std::shared_ptr rather than std::unique_ptr because move-only + // types don't work with std::bind in the ThreadPool implementation. + std::shared_ptr OS; + + std::string OutputFile = OutputLocationOrErr->DWARFFile; + if (NeedsTempFiles) { + TempFiles.emplace_back(Map->getTriple().getArchName().str()); + + auto E = TempFiles.back().createTempFile(); + if (E) { + WithColor::error() << toString(std::move(E)); + AllOK.fetch_and(false); + return; + } + + auto &TempFile = *(TempFiles.back().File); + OS = std::make_shared(TempFile.FD, + /*shouldClose*/ false); + OutputFile = TempFile.TmpName; + } else { + std::error_code EC; + OS = std::make_shared( + Options.LinkOpts.NoOutput ? "-" : OutputFile, EC, + sys::fs::OF_None); + if (EC) { + WithColor::error() << OutputFile << ": " << EC.message(); + AllOK.fetch_and(false); + return; + } } - auto &TempFile = *(TempFiles.back().File); - OS = std::make_shared(TempFile.FD, - /*shouldClose*/ false); - OutputFile = TempFile.TmpName; - } else { - std::error_code EC; - OS = std::make_shared( - Options.LinkOpts.NoOutput ? "-" : OutputFile, EC, sys::fs::OF_None); - if (EC) { - WithColor::error() << OutputFile << ": " << EC.message(); - return EXIT_FAILURE; - } + auto LinkLambda = [&, + OutputFile](std::shared_ptr Stream, + LinkOptions Options) { + AllOK.fetch_and( + linkDwarf(*Stream, BinHolder, *Map, std::move(Options))); + Stream->flush(); + if (VerifyOutput) { + AllOK.fetch_and(verifyOutput( + OutputFile, Map->getTriple().getArchName(), Options.Verbose)); + } + }; + + // FIXME: The DwarfLinker can have some very deep recursion that can max + // out the (significantly smaller) stack when using threads. We don't + // want this limitation when we only have a single thread. + if (S.ThreadsRequested == 1) + LinkLambda(OS, Options.LinkOpts); + else + Threads.async(LinkLambda, OS, Options.LinkOpts); } - auto LinkLambda = [&, OutputFile](std::shared_ptr Stream, - LinkOptions Options) { - AllOK.fetch_and( - linkDwarf(*Stream, BinHolder, *Map, std::move(Options))); - Stream->flush(); - if (VerifyOutput) { - AllOK.fetch_and(verifyOutput( - OutputFile, Map->getTriple().getArchName(), Options.Verbose)); - } - }; - - // FIXME: The DwarfLinker can have some very deep recursion that can max - // out the (significantly smaller) stack when using threads. We don't - // want this limitation when we only have a single thread. - if (S.ThreadsRequested == 1) - LinkLambda(OS, Options.LinkOpts); - else - Threads.async(LinkLambda, OS, Options.LinkOpts); - } + Threads.wait(); + }); - Threads.wait(); + if (Crashed) + (*Repro)->generate(); if (!AllOK) return EXIT_FAILURE;