diff --git a/lldb/include/lldb/API/SBReproducer.h b/lldb/include/lldb/API/SBReproducer.h --- a/lldb/include/lldb/API/SBReproducer.h +++ b/lldb/include/lldb/API/SBReproducer.h @@ -48,6 +48,7 @@ static const char *Replay(const char *path, bool skip_version_check); static const char *Replay(const char *path, const SBReplayOptions &options); static const char *PassiveReplay(const char *path); + static const char *Finalize(const char *path); static const char *GetPath(); static bool SetAutoGenerate(bool b); static bool Generate(); diff --git a/lldb/include/lldb/Host/FileSystem.h b/lldb/include/lldb/Host/FileSystem.h --- a/lldb/include/lldb/Host/FileSystem.h +++ b/lldb/include/lldb/Host/FileSystem.h @@ -34,7 +34,7 @@ FileSystem() : m_fs(llvm::vfs::getRealFileSystem()), m_collector(nullptr), m_home_directory(), m_mapped(false) {} - FileSystem(std::shared_ptr collector) + FileSystem(std::shared_ptr collector) : m_fs(llvm::vfs::getRealFileSystem()), m_collector(std::move(collector)), m_home_directory(), m_mapped(false) {} FileSystem(llvm::IntrusiveRefCntPtr fs, @@ -48,7 +48,7 @@ static FileSystem &Instance(); static void Initialize(); - static void Initialize(std::shared_ptr collector); + static void Initialize(std::shared_ptr collector); static llvm::Error Initialize(const FileSpec &mapping); static void Initialize(llvm::IntrusiveRefCntPtr fs); static void Terminate(); @@ -199,7 +199,7 @@ private: static llvm::Optional &InstanceImpl(); llvm::IntrusiveRefCntPtr m_fs; - std::shared_ptr m_collector; + std::shared_ptr m_collector; std::string m_home_directory; bool m_mapped; }; diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -243,6 +243,9 @@ bool check_version = true; }; +llvm::Error Finalize(Loader *loader); +llvm::Error Finalize(const FileSpec &root); + } // namespace repro } // namespace lldb_private diff --git a/lldb/include/lldb/Utility/ReproducerProvider.h b/lldb/include/lldb/Utility/ReproducerProvider.h --- a/lldb/include/lldb/Utility/ReproducerProvider.h +++ b/lldb/include/lldb/Utility/ReproducerProvider.h @@ -90,6 +90,23 @@ } }; +class FlushingFileCollector : public llvm::FileCollectorBase { +public: + FlushingFileCollector(llvm::StringRef files_path, llvm::StringRef dirs_path, + std::error_code &ec); + +protected: + void addFileImpl(llvm::StringRef file) override; + + llvm::vfs::directory_iterator + addDirectoryImpl(const llvm::Twine &dir, + llvm::IntrusiveRefCntPtr vfs, + std::error_code &dir_ec) override; + + llvm::Optional m_files_os; + llvm::Optional m_dirs_os; +}; + class FileProvider : public Provider { public: struct Info { @@ -97,31 +114,26 @@ static const char *file; }; - FileProvider(const FileSpec &directory) - : Provider(directory), - m_collector(std::make_shared( - directory.CopyByAppendingPathComponent("root").GetPath(), - directory.GetPath())) {} + FileProvider(const FileSpec &directory) : Provider(directory) { + std::error_code ec; + m_collector = std::make_shared( + directory.CopyByAppendingPathComponent("files.txt").GetPath(), + directory.CopyByAppendingPathComponent("dirs.txt").GetPath(), ec); + if (ec) + m_collector.reset(); + } - std::shared_ptr GetFileCollector() { + std::shared_ptr GetFileCollector() { return m_collector; } void RecordInterestingDirectory(const llvm::Twine &dir); void RecordInterestingDirectoryRecursive(const llvm::Twine &dir); - void Keep() override { - auto mapping = GetRoot().CopyByAppendingPathComponent(Info::file); - // Temporary files that are removed during execution can cause copy errors. - if (auto ec = m_collector->copyFiles(/*stop_on_error=*/false)) - return; - m_collector->writeMapping(mapping.GetPath()); - } - static char ID; private: - std::shared_ptr m_collector; + std::shared_ptr m_collector; }; /// Provider for the LLDB version number. diff --git a/lldb/source/API/SBReproducer.cpp b/lldb/source/API/SBReproducer.cpp --- a/lldb/source/API/SBReproducer.cpp +++ b/lldb/source/API/SBReproducer.cpp @@ -8,7 +8,6 @@ #include "SBReproducerPrivate.h" -#include "SBReproducerPrivate.h" #include "lldb/API/LLDB.h" #include "lldb/API/SBAddress.h" #include "lldb/API/SBAttachInfo.h" @@ -266,6 +265,27 @@ return nullptr; } +const char *SBReproducer::Finalize(const char *path) { + static std::string error; + if (auto e = Reproducer::Initialize(ReproducerMode::Replay, FileSpec(path))) { + error = llvm::toString(std::move(e)); + return error.c_str(); + } + + repro::Loader *loader = repro::Reproducer::Instance().GetLoader(); + if (!loader) { + error = "unable to get replay loader."; + return error.c_str(); + } + + if (auto e = repro::Finalize(loader)) { + error = llvm::toString(std::move(e)); + return error.c_str(); + } + + return nullptr; +} + bool SBReproducer::Generate() { auto &r = Reproducer::Instance(); if (auto generator = r.GetGenerator()) { @@ -285,10 +305,11 @@ } const char *SBReproducer::GetPath() { - static std::string path; + ConstString path; auto &r = Reproducer::Instance(); - path = r.GetReproducerPath().GetCString(); - return path.c_str(); + if (FileSpec reproducer_path = Reproducer::Instance().GetReproducerPath()) + path = ConstString(r.GetReproducerPath().GetCString()); + return path.GetCString(); } void SBReproducer::SetWorkingDirectory(const char *path) { diff --git a/lldb/source/Commands/CommandObjectReproducer.cpp b/lldb/source/Commands/CommandObjectReproducer.cpp --- a/lldb/source/Commands/CommandObjectReproducer.cpp +++ b/lldb/source/Commands/CommandObjectReproducer.cpp @@ -192,6 +192,10 @@ auto &r = Reproducer::Instance(); if (auto generator = r.GetGenerator()) { generator->Keep(); + if (llvm::Error e = repro::Finalize(r.GetReproducerPath())) { + SetError(result, std::move(e)); + return result.Succeeded(); + } } else if (r.IsReplaying()) { // Make this operation a NO-OP in replay mode. result.SetStatus(eReturnStatusSuccessFinishNoResult); diff --git a/lldb/source/Host/common/FileSystem.cpp b/lldb/source/Host/common/FileSystem.cpp --- a/lldb/source/Host/common/FileSystem.cpp +++ b/lldb/source/Host/common/FileSystem.cpp @@ -49,7 +49,7 @@ InstanceImpl().emplace(); } -void FileSystem::Initialize(std::shared_ptr collector) { +void FileSystem::Initialize(std::shared_ptr collector) { lldbassert(!InstanceImpl() && "Already initialized."); InstanceImpl().emplace(collector); } diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h b/lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h --- a/lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h @@ -18,7 +18,7 @@ : public clang::ModuleDependencyCollector { public: ModuleDependencyCollectorAdaptor( - std::shared_ptr file_collector) + std::shared_ptr file_collector) : clang::ModuleDependencyCollector(""), m_file_collector(file_collector) { } @@ -33,7 +33,7 @@ void writeFileMap() override {} private: - std::shared_ptr m_file_collector; + std::shared_ptr m_file_collector; }; } // namespace lldb_private diff --git a/lldb/source/Utility/Reproducer.cpp b/lldb/source/Utility/Reproducer.cpp --- a/lldb/source/Utility/Reproducer.cpp +++ b/lldb/source/Utility/Reproducer.cpp @@ -176,10 +176,12 @@ Generator::~Generator() { if (!m_done) { - if (m_auto_generate) + if (m_auto_generate) { Keep(); - else + llvm::cantFail(Finalize(GetRoot())); + } else { Discard(); + } } } @@ -359,3 +361,58 @@ } } } + +static llvm::Error addPaths(StringRef path, + function_ref callback) { + auto buffer = llvm::MemoryBuffer::getFile(path); + if (!buffer) + return errorCodeToError(buffer.getError()); + + SmallVector paths; + (*buffer)->getBuffer().split(paths, '\0'); + for (StringRef p : paths) { + if (!p.empty()) + callback(p); + } + + return errorCodeToError(llvm::sys::fs::remove(path)); +} + +llvm::Error repro::Finalize(Loader *loader) { + if (!loader) + return make_error("invalid loader", + llvm::inconvertibleErrorCode()); + + FileSpec reproducer_root = loader->GetRoot(); + std::string files_path = + reproducer_root.CopyByAppendingPathComponent("files.txt").GetPath(); + std::string dirs_path = + reproducer_root.CopyByAppendingPathComponent("dirs.txt").GetPath(); + + FileCollector collector( + reproducer_root.CopyByAppendingPathComponent("root").GetPath(), + reproducer_root.GetPath()); + + if (Error e = + addPaths(files_path, [&](StringRef p) { collector.addFile(p); })) + return e; + + if (Error e = + addPaths(dirs_path, [&](StringRef p) { collector.addDirectory(p); })) + return e; + + FileSpec mapping = + reproducer_root.CopyByAppendingPathComponent(FileProvider::Info::file); + if (auto ec = collector.copyFiles(/*stop_on_error=*/false)) + return errorCodeToError(ec); + collector.writeMapping(mapping.GetPath()); + + return llvm::Error::success(); +} + +llvm::Error repro::Finalize(const FileSpec &root) { + Loader loader(root); + if (Error e = loader.LoadIndex()) + return e; + return Finalize(&loader); +} diff --git a/lldb/source/Utility/ReproducerProvider.cpp b/lldb/source/Utility/ReproducerProvider.cpp --- a/lldb/source/Utility/ReproducerProvider.cpp +++ b/lldb/source/Utility/ReproducerProvider.cpp @@ -8,6 +8,7 @@ #include "lldb/Utility/ReproducerProvider.h" #include "lldb/Utility/ProcessInfo.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" @@ -44,6 +45,40 @@ os << m_version << "\n"; } +FlushingFileCollector::FlushingFileCollector(llvm::StringRef files_path, + llvm::StringRef dirs_path, + std::error_code &ec) { + auto clear = llvm::make_scope_exit([this]() { + m_files_os.reset(); + m_dirs_os.reset(); + }); + m_files_os.emplace(files_path, ec, llvm::sys::fs::OF_Append); + if (ec) + return; + m_dirs_os.emplace(dirs_path, ec, llvm::sys::fs::OF_Append); + if (ec) + return; + clear.release(); +} + +void FlushingFileCollector::addFileImpl(StringRef file) { + if (m_files_os) { + *m_files_os << file << '\0'; + m_files_os->flush(); + } +} + +llvm::vfs::directory_iterator +FlushingFileCollector::addDirectoryImpl(const Twine &dir, + IntrusiveRefCntPtr vfs, + std::error_code &dir_ec) { + if (m_dirs_os) { + *m_dirs_os << dir << '\0'; + m_dirs_os->flush(); + } + return vfs->dir_begin(dir, dir_ec); +} + void FileProvider::RecordInterestingDirectory(const llvm::Twine &dir) { if (m_collector) m_collector->addFile(dir); diff --git a/lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test b/lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test --- a/lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test +++ b/lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test @@ -9,8 +9,6 @@ # RUN: LLDB_CAPTURE_REPRODUCER=0 %lldb -b -o 'reproducer status' --capture --capture-path %t.repro | FileCheck %s --check-prefix OFF # RUN: LLDB_CAPTURE_REPRODUCER=0 %lldb -b -o 'reproducer status' --capture | FileCheck %s --check-prefix OFF -# RUN: LLDB_CAPTURE_REPRODUCER=OFF %lldb -b -o 'reproducer status' --capture --capture-path %t.repro | FileCheck %s --check-prefix OFF -# RUN: LLDB_CAPTURE_REPRODUCER=off %lldb -b -o 'reproducer status' --capture | FileCheck %s --check-prefix OFF # RUN: LLDB_CAPTURE_REPRODUCER=bogus %lldb -b -o 'reproducer status' --capture | FileCheck %s --check-prefix CAPTURE # RUN: LLDB_CAPTURE_REPRODUCER=bogus %lldb -b -o 'reproducer status' | FileCheck %s --check-prefix OFF diff --git a/lldb/test/Shell/Reproducer/TestFinalize.test b/lldb/test/Shell/Reproducer/TestFinalize.test new file mode 100644 --- /dev/null +++ b/lldb/test/Shell/Reproducer/TestFinalize.test @@ -0,0 +1,14 @@ +# RUN: mkdir -p %t.repro +# RUN: touch %t.known.file +# RUN: mkdir -p %t.known.dir +# RUN: touch %t.repro/index.yaml +# RUN: echo -n "%t.known.file" > %t.repro/files.txt +# RUN: echo -n "%t.known.dir" > %t.repro/dirs.txt + +# RUN: %lldb --reproducer-finalize %t.repro 2>&1 | FileCheck %s +# CHECK-NOT: error +# CHECK: Reproducer written to + +# RUN: echo "CHECK-DAG: %t.known.file" > %t.filecheck +# RUN: echo "CHECK-DAG %t.known.dir" >> %t.filecheck +# RUN: cat %t.repro/files.yaml | FileCheck %t.filecheck diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp --- a/lldb/tools/driver/Driver.cpp +++ b/lldb/tools/driver/Driver.cpp @@ -729,25 +729,10 @@ signal(signo, sigcont_handler); } -void reproducer_handler(void *argv0) { +void reproducer_handler(void *finalize_cmd) { if (SBReproducer::Generate()) { - auto exe = static_cast(argv0); - llvm::outs() << "********************\n"; - llvm::outs() << "Crash reproducer for "; - llvm::outs() << lldb::SBDebugger::GetVersionString() << '\n'; - llvm::outs() << '\n'; - llvm::outs() << "Reproducer written to '" << SBReproducer::GetPath() - << "'\n"; - llvm::outs() << '\n'; - llvm::outs() << "Before attaching the reproducer to a bug report:\n"; - llvm::outs() << " - Look at the directory to ensure you're willing to " - "share its content.\n"; - llvm::outs() - << " - Make sure the reproducer works by replaying the reproducer.\n"; - llvm::outs() << '\n'; - llvm::outs() << "Replay the reproducer with the following command:\n"; - llvm::outs() << exe << " -replay " << SBReproducer::GetPath() << "\n"; - llvm::outs() << "********************\n"; + std::system(static_cast(finalize_cmd)); + fflush(stdout); } } @@ -799,6 +784,31 @@ llvm::Optional InitializeReproducer(llvm::StringRef argv0, opt::InputArgList &input_args) { + if (auto *finalize_path = input_args.getLastArg(OPT_reproducer_finalize)) { + if (const char *error = SBReproducer::Finalize(finalize_path->getValue())) { + WithColor::error() << "reproducer finalization failed: " << error << '\n'; + return 1; + } + + llvm::outs() << "********************\n"; + llvm::outs() << "Crash reproducer for "; + llvm::outs() << lldb::SBDebugger::GetVersionString() << '\n'; + llvm::outs() << '\n'; + llvm::outs() << "Reproducer written to '" << SBReproducer::GetPath() + << "'\n"; + llvm::outs() << '\n'; + llvm::outs() << "Before attaching the reproducer to a bug report:\n"; + llvm::outs() << " - Look at the directory to ensure you're willing to " + "share its content.\n"; + llvm::outs() + << " - Make sure the reproducer works by replaying the reproducer.\n"; + llvm::outs() << '\n'; + llvm::outs() << "Replay the reproducer with the following command:\n"; + llvm::outs() << argv0 << " -replay " << finalize_path->getValue() << "\n"; + llvm::outs() << "********************\n"; + return 0; + } + if (auto *replay_path = input_args.getLastArg(OPT_replay)) { SBReplayOptions replay_options; replay_options.SetCheckVersion(!input_args.hasArg(OPT_no_version_check)); @@ -821,12 +831,6 @@ } if (capture || capture_path) { - // Register the reproducer signal handler. - if (!input_args.hasArg(OPT_no_generate_on_signal)) { - llvm::sys::AddSignalHandler(reproducer_handler, - const_cast(argv0.data())); - } - if (capture_path) { if (!capture) WithColor::warning() << "-capture-path specified without -capture\n"; @@ -843,6 +847,19 @@ } if (generate_on_exit) SBReproducer::SetAutoGenerate(true); + + // Register the reproducer signal handler. + if (!input_args.hasArg(OPT_no_generate_on_signal)) { + if (const char *reproducer_path = SBReproducer::GetPath()) { + // Leaking the string on purpose. + std::string *finalize_cmd = new std::string(argv0); + finalize_cmd->append(" --reproducer-finalize '"); + finalize_cmd->append(reproducer_path); + finalize_cmd->append("'"); + llvm::sys::AddSignalHandler(reproducer_handler, + const_cast(finalize_cmd->c_str())); + } + } } return llvm::None; diff --git a/lldb/tools/driver/Options.td b/lldb/tools/driver/Options.td --- a/lldb/tools/driver/Options.td +++ b/lldb/tools/driver/Options.td @@ -232,6 +232,8 @@ def replay: Separate<["--", "-"], "replay">, MetaVarName<"">, HelpText<"Tells the debugger to replay a reproducer from .">; +def reproducer_finalize: Separate<["--", "-"], "reproducer-finalize">, + MetaVarName<"">; def no_version_check: F<"reproducer-no-version-check">, HelpText<"Disable the reproducer version check.">; def no_verification: F<"reproducer-no-verify">,