[llvm] 33b6891 - [dsymutil] Automatically generate a reproducer when dsymutil crashes
Jonas Devlieghere via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 14 16:00:14 PDT 2022
Author: Jonas Devlieghere
Date: 2022-06-14T16:00:08-07:00
New Revision: 33b6891db24dd1e6702b4f04d2b08c1bf417dbee
URL: https://github.com/llvm/llvm-project/commit/33b6891db24dd1e6702b4f04d2b08c1bf417dbee
DIFF: https://github.com/llvm/llvm-project/commit/33b6891db24dd1e6702b4f04d2b08c1bf417dbee.diff
LOG: [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
Added:
Modified:
llvm/docs/CommandGuide/dsymutil.rst
llvm/test/tools/dsymutil/X86/reproducer.test
llvm/test/tools/dsymutil/cmdline.test
llvm/tools/dsymutil/Options.td
llvm/tools/dsymutil/Reproducer.cpp
llvm/tools/dsymutil/Reproducer.h
llvm/tools/dsymutil/dsymutil.cpp
Removed:
################################################################################
diff --git a/llvm/docs/CommandGuide/dsymutil.rst b/llvm/docs/CommandGuide/dsymutil.rst
index a640c1456a711..7ee6566cad421 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 <mode>
+
+ 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 <path>
- 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 b2a9b65b6bd67..80db5bd2500e2 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 14707110825b4..03a57f62eedbc 100644
--- a/llvm/test/tools/dsymutil/cmdline.test
+++ b/llvm/test/tools/dsymutil/cmdline.test
@@ -23,6 +23,7 @@ CHECK: {{-o <filename>}}
CHECK: -papertrail
CHECK: -remarks-output-format <format>
CHECK: -remarks-prepend-path <path>
+CHECK: -reproducer <mode>
CHECK: -statistics
CHECK: -symbol-map
CHECK: -symtab
diff --git a/llvm/tools/dsymutil/Options.td b/llvm/tools/dsymutil/Options.td
index 54f02ff1540b9..d500762fc830b 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<grp_general>;
+def reproducer: Separate<["--", "-"], "reproducer">,
+ MetaVarName<"<mode>">,
+ HelpText<"Specify the reproducer generation mode. Valid options are 'GenerateOnExit', 'GenerateOnCrash', 'Use', 'Off'.">,
+ Group<grp_general>;
+def: Joined<["--", "-"], "reproducer=">, Alias<reproducer>;
+
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<grp_general>;
def use_reproducer: Separate<["--", "-"], "use-reproducer">,
MetaVarName<"<path>">,
- 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<grp_general>;
def: Joined<["--", "-"], "use-reproducer=">, Alias<use_reproducer>;
diff --git a/llvm/tools/dsymutil/Reproducer.cpp b/llvm/tools/dsymutil/Reproducer.cpp
index 4f2e0db297e5c..7b06078a104b4 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<FileCollector>(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<std::unique_ptr<Reproducer>>
-Reproducer::createReproducer(ReproducerMode Mode, StringRef Root) {
+Reproducer::createReproducer(ReproducerMode Mode, StringRef Root, int Argc,
+ char **Argv) {
+
+ std::error_code EC;
+ std::unique_ptr<Reproducer> Repro;
switch (Mode) {
- case ReproducerMode::Generate: {
- std::error_code EC;
- std::unique_ptr<Reproducer> Repro =
- std::make_unique<ReproducerGenerate>(EC);
- if (EC)
- return errorCodeToError(EC);
- return std::move(Repro);
- }
- case ReproducerMode::Use: {
- std::error_code EC;
- std::unique_ptr<Reproducer> Repro =
- std::make_unique<ReproducerUse>(Root, EC);
- if (EC)
- return errorCodeToError(EC);
- return std::move(Repro);
- }
+ case ReproducerMode::GenerateOnExit:
+ Repro = std::make_unique<ReproducerGenerate>(EC, Argc, Argv, true);
+ break;
+ case ReproducerMode::GenerateOnCrash:
+ Repro = std::make_unique<ReproducerGenerate>(EC, Argc, Argv, false);
+ break;
+ case ReproducerMode::Use:
+ Repro = std::make_unique<ReproducerUse>(Root, EC);
+ break;
case ReproducerMode::Off:
- return std::make_unique<Reproducer>();
+ Repro = std::make_unique<Reproducer>();
+ 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 a53b97f7ca9f4..c88e1dd111912 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 @@ class Reproducer {
IntrusiveRefCntPtr<vfs::FileSystem> getVFS() const { return VFS; }
+ virtual void generate(){};
+
/// Create a Reproducer instance based on the given mode.
static llvm::Expected<std::unique_ptr<Reproducer>>
- createReproducer(ReproducerMode Mode, StringRef Root);
+ createReproducer(ReproducerMode Mode, StringRef Root, int Argc, char **Argv);
protected:
IntrusiveRefCntPtr<vfs::FileSystem> VFS;
@@ -46,15 +49,27 @@ class Reproducer {
/// 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<FileCollector> FC;
+
+ /// The input arguments to build the reproducer invocation.
+ llvm::SmallVector<llvm::StringRef, 0> 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 abdb4ca4d48ce..d26c0c04ef6f5 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<std::string> 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<ReproducerMode> 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<StringError>(
+ "invalid reproducer mode: '" + S +
+ "'. Supported values are 'GenerateOnExit', 'GenerateOnCrash', "
+ "'Use', 'Off'.",
+ inconvertibleErrorCode());
+ }
+ return ReproducerMode::GenerateOnCrash;
+}
+
static Expected<DWARFVerify> getVerifyKind(opt::InputArgList &Args) {
if (Args.hasArg(OPT_verify))
return DWARFVerify::Output;
@@ -280,11 +303,14 @@ static Expected<DsymutilOptions> getOptions(opt::InputArgList &Args) {
if (opt::Arg *ReproducerPath = Args.getLastArg(OPT_use_reproducer)) {
Options.ReproMode = ReproducerMode::Use;
Options.ReproducerPath = ReproducerPath->getValue();
+ } else {
+ if (Expected<ReproducerMode> ReproMode = getReproducerMode(Args)) {
+ Options.ReproMode = *ReproMode;
+ } else {
+ return ReproMode.takeError();
+ }
}
- if (Args.hasArg(OPT_gen_reproducer))
- Options.ReproMode = ReproducerMode::Generate;
-
if (Expected<DwarfLinkerAccelTableKind> 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<MachOUtils::ArchAndFile, 4> 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<raw_fd_ostream> OS;
-
- std::string OutputFile = OutputLocationOrErr->DWARFFile;
- if (NeedsTempFiles) {
- TempFiles.emplace_back(Map->getTriple().getArchName().str());
+ std::atomic_char AllOK(1);
+ SmallVector<MachOUtils::ArchAndFile, 4> 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<raw_fd_ostream> 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<raw_fd_ostream>(TempFile.FD,
+ /*shouldClose*/ false);
+ OutputFile = TempFile.TmpName;
+ } else {
+ std::error_code EC;
+ OS = std::make_shared<raw_fd_ostream>(
+ 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<raw_fd_ostream>(TempFile.FD,
- /*shouldClose*/ false);
- OutputFile = TempFile.TmpName;
- } else {
- std::error_code EC;
- OS = std::make_shared<raw_fd_ostream>(
- 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<raw_fd_ostream> 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<raw_fd_ostream> 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;
More information about the llvm-commits
mailing list