Index: lldb/include/lldb/API/SBReproducer.h =================================================================== --- lldb/include/lldb/API/SBReproducer.h +++ 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(); Index: lldb/include/lldb/Host/FileSystem.h =================================================================== --- lldb/include/lldb/Host/FileSystem.h +++ 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; }; Index: lldb/include/lldb/Utility/Reproducer.h =================================================================== --- lldb/include/lldb/Utility/Reproducer.h +++ lldb/include/lldb/Utility/Reproducer.h @@ -238,6 +238,23 @@ Loader *m_loader; }; +class Finalizer { +public: + Finalizer(Loader *loader) : m_loader(loader) {} + llvm::Error Finalize() const; + +protected: + Loader *m_loader; +}; + +class InProcessFinalizer : public Finalizer { +public: + InProcessFinalizer(const FileSpec &root); + +private: + Loader m_loader_storage; +}; + struct ReplayOptions { bool verify = true; bool check_version = true; Index: lldb/include/lldb/Utility/ReproducerProvider.h =================================================================== --- lldb/include/lldb/Utility/ReproducerProvider.h +++ lldb/include/lldb/Utility/ReproducerProvider.h @@ -90,6 +90,22 @@ } }; +class FlushingFileCollector : public llvm::FileCollectorBase { +public: + FlushingFileCollector(std::string files_path, std::string dirs_path); + +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; + + std::string m_files_path; + std::string m_dirs_path; +}; + class FileProvider : public Provider { public: struct Info { @@ -99,29 +115,21 @@ FileProvider(const FileSpec &directory) : Provider(directory), - m_collector(std::make_shared( - directory.CopyByAppendingPathComponent("root").GetPath(), - directory.GetPath())) {} + m_collector(std::make_shared( + directory.CopyByAppendingPathComponent("files.txt").GetPath(), + directory.CopyByAppendingPathComponent("dirs.txt").GetPath())) {} - 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. Index: lldb/source/API/SBReproducer.cpp =================================================================== --- lldb/source/API/SBReproducer.cpp +++ lldb/source/API/SBReproducer.cpp @@ -266,6 +266,28 @@ 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(); + } + + Finalizer finalizer(loader); + if (auto e = finalizer.Finalize()) { + 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 +307,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) { Index: lldb/source/Commands/CommandObjectReproducer.cpp =================================================================== --- lldb/source/Commands/CommandObjectReproducer.cpp +++ lldb/source/Commands/CommandObjectReproducer.cpp @@ -192,6 +192,11 @@ auto &r = Reproducer::Instance(); if (auto generator = r.GetGenerator()) { generator->Keep(); + InProcessFinalizer finalizer(r.GetReproducerPath()); + if (llvm::Error e = finalizer.Finalize()) { + SetError(result, std::move(e)); + return result.Succeeded(); + } } else if (r.IsReplaying()) { // Make this operation a NO-OP in replay mode. result.SetStatus(eReturnStatusSuccessFinishNoResult); Index: lldb/source/Host/common/FileSystem.cpp =================================================================== --- lldb/source/Host/common/FileSystem.cpp +++ 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); } Index: lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h =================================================================== --- lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h +++ 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 Index: lldb/source/Utility/Reproducer.cpp =================================================================== --- lldb/source/Utility/Reproducer.cpp +++ lldb/source/Utility/Reproducer.cpp @@ -176,10 +176,13 @@ Generator::~Generator() { if (!m_done) { - if (m_auto_generate) + if (m_auto_generate) { Keep(); - else + InProcessFinalizer finalizer(GetRoot()); + llvm::cantFail(finalizer.Finalize()); + } else { Discard(); + } } } @@ -359,3 +362,55 @@ } } } + +InProcessFinalizer::InProcessFinalizer(const FileSpec &root) + : Finalizer(nullptr), m_loader_storage(root) { + if (Error e = m_loader_storage.LoadIndex()) + llvm::consumeError(std::move(e)); + else + m_loader = &m_loader_storage; +} + +llvm::Error Finalizer::Finalize() const { + if (!m_loader) + return make_error("invalid loader", + llvm::inconvertibleErrorCode()); + + FileSpec reproducer_root = m_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 (auto files = llvm::MemoryBuffer::getFile(files_path)) { + SmallVector paths; + (*files)->getBuffer().split(paths, '\n'); + for (StringRef path : paths) { + if (!path.empty()) + collector.addFile(path); + } + llvm::sys::fs::remove(files_path); + } + + if (auto dirs = llvm::MemoryBuffer::getFile(dirs_path)) { + SmallVector paths; + (*dirs)->getBuffer().split(paths, '\n'); + for (StringRef path : paths) { + if (!path.empty()) + collector.addDirectory(path); + } + llvm::sys::fs::remove(dirs_path); + } + + 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(); +} Index: lldb/source/Utility/ReproducerProvider.cpp =================================================================== --- lldb/source/Utility/ReproducerProvider.cpp +++ lldb/source/Utility/ReproducerProvider.cpp @@ -44,6 +44,30 @@ os << m_version << "\n"; } +FlushingFileCollector::FlushingFileCollector(std::string files_path, + std::string dirs_path) + : m_files_path(std::move(files_path)), m_dirs_path(std::move(dirs_path)) {} + +void FlushingFileCollector::addFileImpl(StringRef file) { + std::error_code ec; + llvm::raw_fd_ostream os(m_files_path, ec, llvm::sys::fs::OF_Append); + if (ec) + return; + os << file << "\n"; +} + +llvm::vfs::directory_iterator +FlushingFileCollector::addDirectoryImpl(const Twine &dir, + IntrusiveRefCntPtr vfs, + std::error_code &dir_ec) { + std::error_code ec; + llvm::raw_fd_ostream os(m_dirs_path, ec, llvm::sys::fs::OF_Append); + if (ec) + return vfs->dir_begin(dir, dir_ec); + os << dir << "\n"; + return vfs->dir_begin(dir, dir_ec); +} + void FileProvider::RecordInterestingDirectory(const llvm::Twine &dir) { if (m_collector) m_collector->addFile(dir); Index: lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test =================================================================== --- lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test +++ 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 Index: lldb/tools/driver/Driver.cpp =================================================================== --- lldb/tools/driver/Driver.cpp +++ lldb/tools/driver/Driver.cpp @@ -729,25 +729,10 @@ signal(signo, sigcont_handler); } -void reproducer_handler(void *argv0) { +void reproducer_handler(void *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(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 *str = new std::string(argv0); + str->append(" --reproducer-finalize '"); + str->append(reproducer_path); + str->append("'"); + llvm::sys::AddSignalHandler(reproducer_handler, + const_cast(str->c_str())); + } + } } return llvm::None; Index: lldb/tools/driver/Options.td =================================================================== --- lldb/tools/driver/Options.td +++ 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">,