[Reproducers] Improve reproducer API and add unit tests.
When I landed the initial reproducer framework I knew there were some
things that needed improvement. Rather than bundling it with a patch
that adds more functionality I split it off into this patch. I also
think the API is stable enough to add unit testing, which is included in
this patch as well.
Other improvements include:
- Refactor how we initialize the loader and generator.
- Improve naming consistency: capture and replay seems the least ambiguous.
- Index providers by name and make sure there's only one of each.
- Add convenience methods for creating and accessing providers.
Differential revision: https://reviews.llvm.org/D54616
llvm-svn: 347716
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 1ea1258..a721796 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1057,9 +1057,15 @@
: nullptr);
}
-void SBDebugger::SetReproducerPath(const char *p) {
- if (m_opaque_sp)
- m_opaque_sp->SetReproducerPath(llvm::StringRef::withNullAsEmpty(p));
+SBError SBDebugger::ReplayReproducer(const char *p) {
+ SBError sb_error;
+ if (m_opaque_sp) {
+ auto error =
+ m_opaque_sp->SetReproducerReplay(llvm::StringRef::withNullAsEmpty(p));
+ std::string error_str = llvm::toString(std::move(error));
+ sb_error.ref().SetErrorString(error_str);
+ }
+ return sb_error;
}
ScriptLanguage SBDebugger::GetScriptLanguage() const {
diff --git a/lldb/source/Commands/CommandObjectReproducer.cpp b/lldb/source/Commands/CommandObjectReproducer.cpp
index a1fdb4a..03a3e23 100644
--- a/lldb/source/Commands/CommandObjectReproducer.cpp
+++ b/lldb/source/Commands/CommandObjectReproducer.cpp
@@ -143,25 +143,13 @@
auto &r = repro::Reproducer::Instance();
- if (auto e = r.SetReplayReproducer(true)) {
+ const char *repro_path = command.GetArgumentAtIndex(0);
+ if (auto e = r.SetReplay(FileSpec(repro_path))) {
std::string error_str = llvm::toString(std::move(e));
result.AppendErrorWithFormat("%s", error_str.c_str());
return false;
}
- if (auto loader = r.GetLoader()) {
- const char *repro_path = command.GetArgumentAtIndex(0);
- if (auto e = loader->LoadIndex(FileSpec(repro_path))) {
- std::string error_str = llvm::toString(std::move(e));
- result.AppendErrorWithFormat("Unable to load reproducer: %s",
- error_str.c_str());
- return false;
- }
- } else {
- result.AppendErrorWithFormat("Unable to get the reproducer loader");
- return false;
- }
-
result.SetStatus(eReturnStatusSuccessFinishNoResult);
return result.Succeeded();
}
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 19b476b..1f12dc4 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -418,17 +418,22 @@
return r.GetReproducerPath().GetCString();
}
-void Debugger::SetReproducerPath(llvm::StringRef p) {
- auto &r = repro::Reproducer::Instance();
- if (auto e = r.SetReproducerPath(FileSpec(p)))
- llvm::consumeError(std::move(e));
+llvm::Error Debugger::SetReproducerReplay(llvm::StringRef p) {
+ llvm::Optional<FileSpec> arg = llvm::None;
+
+ if (!p.empty())
+ arg = FileSpec(p);
+
+ return repro::Reproducer::Instance().SetReplay(arg);
}
llvm::Error Debugger::SetReproducerCapture(bool b) {
- auto &r = repro::Reproducer::Instance();
- if (auto e = r.SetGenerateReproducer(b))
- return e;
- return llvm::Error::success();
+ llvm::Optional<FileSpec> arg = llvm::None;
+
+ if (b)
+ arg = HostInfo::GetReproducerTempDir();
+
+ return repro::Reproducer::Instance().SetCapture(arg);
}
const FormatEntity::Entry *Debugger::GetThreadFormat() const {
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index e1be28e..7225a39 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -157,7 +157,8 @@
return g_settings_sp;
}
-class ProcessGDBRemoteProvider : public repro::Provider {
+class ProcessGDBRemoteProvider
+ : public repro::Provider<ProcessGDBRemoteProvider> {
public:
ProcessGDBRemoteProvider(const FileSpec &directory) : Provider(directory) {
m_info.name = "gdb-remote";
@@ -166,7 +167,7 @@
raw_ostream *GetHistoryStream() {
FileSpec history_file =
- GetDirectory().CopyByAppendingPathComponent("gdb-remote.yaml");
+ GetRoot().CopyByAppendingPathComponent("gdb-remote.yaml");
std::error_code EC;
m_stream_up = llvm::make_unique<raw_fd_ostream>(history_file.GetPath(), EC,
@@ -182,11 +183,15 @@
void Discard() override { m_callback(); }
+ static char ID;
+
private:
std::function<void()> m_callback;
std::unique_ptr<raw_fd_ostream> m_stream_up;
};
+char ProcessGDBRemoteProvider::ID = 0;
+
} // namespace
// TODO Randomly assigning a port is unsafe. We should get an unused
@@ -297,10 +302,9 @@
m_async_broadcaster.SetEventName(eBroadcastBitAsyncThreadDidExit,
"async thread did exit");
- repro::Generator *generator = repro::Reproducer::Instance().GetGenerator();
- if (generator) {
+ if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) {
ProcessGDBRemoteProvider &provider =
- generator->CreateProvider<ProcessGDBRemoteProvider>();
+ g->GetOrCreate<ProcessGDBRemoteProvider>();
// Set the history stream to the stream owned by the provider.
m_gdb_comm.SetHistoryStream(provider.GetHistoryStream());
// Make sure to clear the stream again when we're finished.
@@ -3436,8 +3440,8 @@
return Status("Provider for gdb-remote contains no files.");
// Construct replay history path.
- FileSpec history_file(loader->GetDirectory());
- history_file.AppendPathComponent(provider_info->files.front());
+ FileSpec history_file = loader->GetRoot().CopyByAppendingPathComponent(
+ provider_info->files.front());
// Enable replay mode.
m_replay_mode = true;
diff --git a/lldb/source/Utility/Reproducer.cpp b/lldb/source/Utility/Reproducer.cpp
index 5b8480f..0048a11 100644
--- a/lldb/source/Utility/Reproducer.cpp
+++ b/lldb/source/Utility/Reproducer.cpp
@@ -8,7 +8,6 @@
//===----------------------------------------------------------------------===//
#include "lldb/Utility/Reproducer.h"
-#include "lldb/Host/HostInfo.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Threading.h"
@@ -26,145 +25,131 @@
const Generator *Reproducer::GetGenerator() const {
std::lock_guard<std::mutex> guard(m_mutex);
- if (m_generate_reproducer)
- return &m_generator;
+ if (m_generator)
+ return &(*m_generator);
return nullptr;
}
const Loader *Reproducer::GetLoader() const {
std::lock_guard<std::mutex> guard(m_mutex);
- if (m_replay_reproducer)
- return &m_loader;
+ if (m_loader)
+ return &(*m_loader);
return nullptr;
}
Generator *Reproducer::GetGenerator() {
std::lock_guard<std::mutex> guard(m_mutex);
- if (m_generate_reproducer)
- return &m_generator;
+ if (m_generator)
+ return &(*m_generator);
return nullptr;
}
Loader *Reproducer::GetLoader() {
std::lock_guard<std::mutex> guard(m_mutex);
- if (m_replay_reproducer)
- return &m_loader;
+ if (m_loader)
+ return &(*m_loader);
return nullptr;
}
-llvm::Error Reproducer::SetGenerateReproducer(bool value) {
+llvm::Error Reproducer::SetCapture(llvm::Optional<FileSpec> root) {
std::lock_guard<std::mutex> guard(m_mutex);
- if (value && m_replay_reproducer)
+ if (root && m_loader)
return make_error<StringError>(
"cannot generate a reproducer when replay one",
inconvertibleErrorCode());
- m_generate_reproducer = value;
- m_generator.SetEnabled(value);
+ if (root)
+ m_generator.emplace(*root);
+ else
+ m_generator.reset();
return Error::success();
}
-llvm::Error Reproducer::SetReplayReproducer(bool value) {
+llvm::Error Reproducer::SetReplay(llvm::Optional<FileSpec> root) {
std::lock_guard<std::mutex> guard(m_mutex);
- if (value && m_generate_reproducer)
+ if (root && m_generator)
return make_error<StringError>(
"cannot replay a reproducer when generating one",
inconvertibleErrorCode());
- m_replay_reproducer = value;
+ if (root) {
+ m_loader.emplace(*root);
+ if (auto e = m_loader->LoadIndex())
+ return e;
+ } else {
+ m_loader.reset();
+ }
return Error::success();
}
-llvm::Error Reproducer::SetReproducerPath(const FileSpec &path) {
- // Setting the path implies using the reproducer.
- if (auto e = SetReplayReproducer(true))
- return e;
-
- // Tell the reproducer to load the index form the given path.
- if (auto loader = GetLoader()) {
- if (auto e = loader->LoadIndex(path))
- return e;
- }
-
- return make_error<StringError>("unable to get loader",
- inconvertibleErrorCode());
-}
-
FileSpec Reproducer::GetReproducerPath() const {
if (auto g = GetGenerator())
- return g->GetDirectory();
+ return g->GetRoot();
+ if (auto l = GetLoader())
+ return l->GetRoot();
return {};
}
-Generator::Generator() : m_enabled(false), m_done(false) {
- m_directory = HostInfo::GetReproducerTempDir();
-}
+Generator::Generator(const FileSpec &root) : m_root(root), m_done(false) {}
Generator::~Generator() {}
-Provider &Generator::Register(std::unique_ptr<Provider> provider) {
+ProviderBase *Generator::Register(std::unique_ptr<ProviderBase> provider) {
std::lock_guard<std::mutex> lock(m_providers_mutex);
-
- AddProviderToIndex(provider->GetInfo());
-
- m_providers.push_back(std::move(provider));
- return *m_providers.back();
+ std::pair<const void *, std::unique_ptr<ProviderBase>> key_value(
+ provider->DynamicClassID(), std::move(provider));
+ auto e = m_providers.insert(std::move(key_value));
+ return e.first->getSecond().get();
}
void Generator::Keep() {
assert(!m_done);
m_done = true;
- if (!m_enabled)
- return;
-
for (auto &provider : m_providers)
- provider->Keep();
+ provider.second->Keep();
+
+ AddProvidersToIndex();
}
void Generator::Discard() {
assert(!m_done);
m_done = true;
- if (!m_enabled)
- return;
-
for (auto &provider : m_providers)
- provider->Discard();
+ provider.second->Discard();
- llvm::sys::fs::remove_directories(m_directory.GetPath());
+ llvm::sys::fs::remove_directories(m_root.GetPath());
}
-void Generator::ChangeDirectory(const FileSpec &directory) {
- assert(m_providers.empty() && "Changing the directory after providers have "
- "been registered would invalidate the index.");
- m_directory = directory;
-}
+const FileSpec &Generator::GetRoot() const { return m_root; }
-const FileSpec &Generator::GetDirectory() const { return m_directory; }
-
-void Generator::AddProviderToIndex(const ProviderInfo &provider_info) {
- FileSpec index = m_directory;
+void Generator::AddProvidersToIndex() {
+ FileSpec index = m_root;
index.AppendPathComponent("index.yaml");
std::error_code EC;
auto strm = llvm::make_unique<raw_fd_ostream>(index.GetPath(), EC,
sys::fs::OpenFlags::F_None);
yaml::Output yout(*strm);
- yout << const_cast<ProviderInfo &>(provider_info);
+
+ for (auto &provider : m_providers) {
+ auto &provider_info = provider.second->GetInfo();
+ yout << const_cast<ProviderInfo &>(provider_info);
+ }
}
-Loader::Loader() : m_loaded(false) {}
+Loader::Loader(const FileSpec &root) : m_root(root), m_loaded(false) {}
-llvm::Error Loader::LoadIndex(const FileSpec &directory) {
+llvm::Error Loader::LoadIndex() {
if (m_loaded)
return llvm::Error::success();
- FileSpec index = directory.CopyByAppendingPathComponent("index.yaml");
+ FileSpec index = m_root.CopyByAppendingPathComponent("index.yaml");
auto error_or_file = MemoryBuffer::getFile(index.GetPath());
if (auto err = error_or_file.getError())
@@ -180,7 +165,6 @@
for (auto &info : provider_info)
m_provider_info[info.name] = info;
- m_directory = directory;
m_loaded = true;
return llvm::Error::success();
@@ -195,3 +179,6 @@
return it->second;
}
+
+void ProviderBase::anchor() {}
+char ProviderBase::ID = 0;