JDevlieghere updated this revision to Diff 299214.
JDevlieghere added a comment.
- Implement a FlushingFileCollector that appends paths to a file
- Create the mapping out-of-process
- Add an InProcessFinalizer that uses the current reproducer instance to copy
over the files in-process
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89600/new/
https://reviews.llvm.org/D89600
Files:
lldb/include/lldb/API/SBReproducer.h
lldb/include/lldb/Host/FileSystem.h
lldb/include/lldb/Utility/Reproducer.h
lldb/include/lldb/Utility/ReproducerProvider.h
lldb/source/API/SBReproducer.cpp
lldb/source/Commands/CommandObjectReproducer.cpp
lldb/source/Host/common/FileSystem.cpp
lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h
lldb/source/Utility/Reproducer.cpp
lldb/source/Utility/ReproducerProvider.cpp
lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test
lldb/tools/driver/Driver.cpp
lldb/tools/driver/Options.td
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<"<filename>">,
HelpText<"Tells the debugger to replay a reproducer from <filename>.">;
+def reproducer_finalize: Separate<["--", "-"], "reproducer-finalize">,
+ MetaVarName<"<filename>">;
def no_version_check: F<"reproducer-no-version-check">,
HelpText<"Disable the reproducer version check.">;
def no_verification: F<"reproducer-no-verify">,
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<const char *>(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<const char *>(cmd));
+ fflush(stdout);
}
}
@@ -799,6 +784,31 @@
llvm::Optional<int> 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<char *>(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<char *>(str->c_str()));
+ }
+ }
}
return llvm::None;
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/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::FileSystem> 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/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<StringError>("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<StringRef, 20> 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<StringRef, 20> 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/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<llvm::FileCollector> file_collector)
+ std::shared_ptr<llvm::FileCollectorBase> file_collector)
: clang::ModuleDependencyCollector(""), m_file_collector(file_collector) {
}
@@ -33,7 +33,7 @@
void writeFileMap() override {}
private:
- std::shared_ptr<llvm::FileCollector> m_file_collector;
+ std::shared_ptr<llvm::FileCollectorBase> m_file_collector;
};
} // namespace lldb_private
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<FileCollector> collector) {
+void FileSystem::Initialize(std::shared_ptr<FileCollectorBase> collector) {
lldbassert(!InstanceImpl() && "Already initialized.");
InstanceImpl().emplace(collector);
}
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/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/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<llvm::vfs::FileSystem> vfs,
+ std::error_code &dir_ec) override;
+
+ std::string m_files_path;
+ std::string m_dirs_path;
+};
+
class FileProvider : public Provider<FileProvider> {
public:
struct Info {
@@ -99,29 +115,21 @@
FileProvider(const FileSpec &directory)
: Provider(directory),
- m_collector(std::make_shared<llvm::FileCollector>(
- directory.CopyByAppendingPathComponent("root").GetPath(),
- directory.GetPath())) {}
+ m_collector(std::make_shared<FlushingFileCollector>(
+ directory.CopyByAppendingPathComponent("files.txt").GetPath(),
+ directory.CopyByAppendingPathComponent("dirs.txt").GetPath())) {}
- std::shared_ptr<llvm::FileCollector> GetFileCollector() {
+ std::shared_ptr<llvm::FileCollectorBase> 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<llvm::FileCollector> m_collector;
+ std::shared_ptr<FlushingFileCollector> m_collector;
};
/// Provider for the LLDB version number.
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/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<llvm::FileCollector> collector)
+ FileSystem(std::shared_ptr<llvm::FileCollectorBase> collector)
: m_fs(llvm::vfs::getRealFileSystem()), m_collector(std::move(collector)),
m_home_directory(), m_mapped(false) {}
FileSystem(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
@@ -48,7 +48,7 @@
static FileSystem &Instance();
static void Initialize();
- static void Initialize(std::shared_ptr<llvm::FileCollector> collector);
+ static void Initialize(std::shared_ptr<llvm::FileCollectorBase> collector);
static llvm::Error Initialize(const FileSpec &mapping);
static void Initialize(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs);
static void Terminate();
@@ -199,7 +199,7 @@
private:
static llvm::Optional<FileSystem> &InstanceImpl();
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
- std::shared_ptr<llvm::FileCollector> m_collector;
+ std::shared_ptr<llvm::FileCollectorBase> m_collector;
std::string m_home_directory;
bool m_mapped;
};
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();
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
