From: Jonas Devlieghere Date: Tue, 14 Jun 2022 16:22:46 +0000 (-0700) Subject: [dsymutil] Automatically generate a reproducer when dsymutil crashes X-Git-Tag: upstream/15.0.7~4739 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=33b6891db24dd1e6702b4f04d2b08c1bf417dbee;p=platform%2Fupstream%2Fllvm.git [dsymutil] Automatically generate a reproducer when dsymutil crashes Automatically generate a reproducer when dsymutil crashes. We already support generating reproducers with the --gen-reproducer flag, which emits a reproducer on exit. This patch adds support for doing the same on a crash and makes it the default behavior. rdar://68357665 Differential revision: https://reviews.llvm.org/D127441 --- diff --git a/llvm/docs/CommandGuide/dsymutil.rst b/llvm/docs/CommandGuide/dsymutil.rst index a640c14..7ee6566 100644 --- a/llvm/docs/CommandGuide/dsymutil.rst +++ b/llvm/docs/CommandGuide/dsymutil.rst @@ -44,7 +44,8 @@ OPTIONS .. option:: --gen-reproducer - Generate a reproducer consisting of the input object files. + Generate a reproducer consisting of the input object files. Alias for + --reproducer=GenerateOnExit. .. option:: --help, -h @@ -110,6 +111,11 @@ OPTIONS Specify a directory to prepend the paths of the external remark files. +.. option:: --reproducer + + Specify the reproducer generation mode. Valid options are 'GenerateOnExit', + 'GenerateOnCrash', 'Use', 'Off'. + .. option:: --statistics Print statistics about the contribution of each object file to the linked @@ -142,7 +148,8 @@ OPTIONS .. option:: --use-reproducer - Use the object files from the given reproducer path. + Use the object files from the given reproducer path. Alias for + --reproducer=Use. .. option:: --verbose diff --git a/llvm/test/tools/dsymutil/X86/reproducer.test b/llvm/test/tools/dsymutil/X86/reproducer.test index b2a9b65..80db5bd 100644 --- a/llvm/test/tools/dsymutil/X86/reproducer.test +++ b/llvm/test/tools/dsymutil/X86/reproducer.test @@ -14,7 +14,7 @@ RUN: cp %p/../Inputs/basic3.macho.x86_64.o %t/Inputs 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. @@ -24,8 +24,8 @@ RUN: not dsymutil -f -o %t.error -oso-prepend-path=%t %t/Inputs/basic.macho.x86_ # Use the reproducer. RUN: dsymutil -use-reproducer %t.repro -f -o - -oso-prepend-path=%t %t/Inputs/basic.macho.x86_64 | llvm-dwarfdump -a - | FileCheck %s -# Conflicting options. -RUN: not dsymutil -gen-reproducer -use-reproducer %t.repro -f -o %t.error -oso-prepend-path=%t %t/Inputs/basic.macho.x86_64 2>&1 | FileCheck %s --check-prefix=CONFLICT +# Using a reproducer takes precedence. +RUN: dsymutil -gen-reproducer -use-reproducer %t.repro -f -o - -oso-prepend-path=%t %t/Inputs/basic.macho.x86_64 | llvm-dwarfdump -a - | FileCheck %s CHECK: .debug_info CHECK: DW_TAG_compile_unit @@ -75,6 +75,5 @@ CHECK-NEXT: DW_AT_encoding (DW_ATE_signed_char) 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 diff --git a/llvm/test/tools/dsymutil/cmdline.test b/llvm/test/tools/dsymutil/cmdline.test index 1470711..03a57f62 100644 --- a/llvm/test/tools/dsymutil/cmdline.test +++ b/llvm/test/tools/dsymutil/cmdline.test @@ -23,6 +23,7 @@ CHECK: {{-o }} CHECK: -papertrail CHECK: -remarks-output-format CHECK: -remarks-prepend-path +CHECK: -reproducer CHECK: -statistics CHECK: -symbol-map CHECK: -symtab diff --git a/llvm/tools/dsymutil/Options.td b/llvm/tools/dsymutil/Options.td index 54f02ff..d500762 100644 --- a/llvm/tools/dsymutil/Options.td +++ b/llvm/tools/dsymutil/Options.td @@ -161,13 +161,19 @@ def: Separate<["-"], "j">, HelpText<"Alias for --num-threads">, Group; +def reproducer: Separate<["--", "-"], "reproducer">, + MetaVarName<"">, + HelpText<"Specify the reproducer generation mode. Valid options are 'GenerateOnExit', 'GenerateOnCrash', 'Use', 'Off'.">, + Group; +def: Joined<["--", "-"], "reproducer=">, Alias; + def gen_reproducer: F<"gen-reproducer">, - HelpText<"Generate a reproducer consisting of the input object files.">, + HelpText<"Generate a reproducer consisting of the input object files. Alias for --reproducer=GenerateOnExit.">, Group; def use_reproducer: Separate<["--", "-"], "use-reproducer">, MetaVarName<"">, - HelpText<"Use the object files from the given reproducer path.">, + HelpText<"Use the object files from the given reproducer path. Alias for --reproducer=Use.">, Group; def: Joined<["--", "-"], "use-reproducer=">, Alias; diff --git a/llvm/tools/dsymutil/Reproducer.cpp b/llvm/tools/dsymutil/Reproducer.cpp index 4f2e0db..7b06078 100644 --- a/llvm/tools/dsymutil/Reproducer.cpp +++ b/llvm/tools/dsymutil/Reproducer.cpp @@ -26,21 +26,37 @@ static std::string createReproducerDir(std::error_code &EC) { 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) { + 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 +76,26 @@ ReproducerUse::ReproducerUse(StringRef Root, std::error_code &EC) { } 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; } diff --git a/llvm/tools/dsymutil/Reproducer.h b/llvm/tools/dsymutil/Reproducer.h index a53b97f..c88e1dd 100644 --- a/llvm/tools/dsymutil/Reproducer.h +++ b/llvm/tools/dsymutil/Reproducer.h @@ -17,7 +17,8 @@ namespace dsymutil { /// The reproducer mode. enum class ReproducerMode { - Generate, + GenerateOnExit, + GenerateOnCrash, Use, Off, }; @@ -33,9 +34,11 @@ public: 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 @@ protected: /// 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 = false; + + /// Whether we already generated the reproducer. + bool Generated = false; }; /// Reproducer instance used to use an existing reproducer. The VFS returned by diff --git a/llvm/tools/dsymutil/dsymutil.cpp b/llvm/tools/dsymutil/dsymutil.cpp index abdb4ca..d26c0c0 100644 --- a/llvm/tools/dsymutil/dsymutil.cpp +++ b/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 @@ struct DsymutilOptions { std::vector InputFiles; unsigned NumThreads; DWARFVerify Verify = DWARFVerify::None; - ReproducerMode ReproMode = ReproducerMode::Off; + ReproducerMode ReproMode = ReproducerMode::GenerateOnCrash; dsymutil::LinkOptions LinkOpts; }; @@ -228,6 +229,28 @@ getAccelTableKind(opt::InputArgList &Args) { return DwarfLinkerAccelTableKind::Default; } +static Expected getReproducerMode(opt::InputArgList &Args) { + if (Args.hasArg(OPT_gen_reproducer)) + return ReproducerMode::GenerateOnExit; + if (opt::Arg *Reproducer = Args.getLastArg(OPT_reproducer)) { + StringRef S = Reproducer->getValue(); + if (S == "GenerateOnExit") + return ReproducerMode::GenerateOnExit; + if (S == "GenerateOnCrash") + return ReproducerMode::GenerateOnCrash; + if (S == "Use") + return ReproducerMode::Use; + if (S == "Off") + return ReproducerMode::Off; + return make_error( + "invalid reproducer mode: '" + S + + "'. Supported values are 'GenerateOnExit', 'GenerateOnCrash', " + "'Use', 'Off'.", + inconvertibleErrorCode()); + } + return ReproducerMode::GenerateOnCrash; +} + static Expected getVerifyKind(opt::InputArgList &Args) { if (Args.hasArg(OPT_verify)) return DWARFVerify::Output; @@ -280,11 +303,14 @@ static Expected getOptions(opt::InputArgList &Args) { if (opt::Arg *ReproducerPath = Args.getLastArg(OPT_use_reproducer)) { Options.ReproMode = ReproducerMode::Use; Options.ReproducerPath = ReproducerPath->getValue(); + } else { + if (Expected ReproMode = getReproducerMode(Args)) { + Options.ReproMode = *ReproMode; + } else { + return ReproMode.takeError(); + } } - if (Args.hasArg(OPT_gen_reproducer)) - Options.ReproMode = ReproducerMode::Generate; - if (Expected AccelKind = getAccelTableKind(Args)) { Options.LinkOpts.TheAccelTableKind = *AccelKind; } else { @@ -569,8 +595,8 @@ int dsymutil_main(int argc, char **argv) { 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 +688,88 @@ int dsymutil_main(int argc, char **argv) { VerifyOutput = false; } - SmallVector TempFiles; - std::atomic_char AllOK(1); - for (auto &Map : *DebugMapPtrsOrErr) { - if (Options.LinkOpts.Verbose || Options.DumpDebugMap) - Map->print(outs()); + // Set up a crash recovery context. + CrashRecoveryContext::Enable(); + CrashRecoveryContext CRC; + CRC.DumpStackAndCleanupOnFailure = true; - 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()); + 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;