Author: Jonas Devlieghere Date: 2020-07-10T12:48:22-07:00 New Revision: 169c83208f37f8e5539b1386030d9f3e4b15f16a
URL: https://github.com/llvm/llvm-project/commit/169c83208f37f8e5539b1386030d9f3e4b15f16a DIFF: https://github.com/llvm/llvm-project/commit/169c83208f37f8e5539b1386030d9f3e4b15f16a.diff LOG: [ldb/Reproducers] Add YamlRecorder and MultiProvider This patch does several things that are all closely related: - It introduces a new YamlRecorder as a counterpart to the existing DataRecorder. As the name suggests the former serializes data as yaml while the latter uses raw texts or bytes. - It introduces a new MultiProvider base class which can be backed by either a DataRecorder or a YamlRecorder. - It reimplements the CommandProvider in terms of the new MultiProvider. Finally, it adds unit testing coverage for the MultiProvider, a naive YamlProvider built on top of the new YamlRecorder and the existing MutliLoader. Differential revision: https://reviews.llvm.org/D83441 Added: Modified: lldb/include/lldb/Utility/Reproducer.h lldb/source/API/SBDebugger.cpp lldb/source/Utility/Reproducer.cpp lldb/unittests/Utility/ReproducerTest.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h index 5d810460a0c5..ab673e5e0019 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -159,6 +159,9 @@ class WorkingDirectoryProvider : public Provider<WorkingDirectoryProvider> { static char ID; }; +/// The recorder is a small object handed out by a provider to record data. It +/// is commonly used in combination with a MultiProvider which is meant to +/// record information for multiple instances of the same source of data. class AbstractRecorder { protected: AbstractRecorder(const FileSpec &filename, std::error_code &ec) @@ -181,6 +184,7 @@ class AbstractRecorder { bool m_record; }; +/// Recorder that records its data as text to a file. class DataRecorder : public AbstractRecorder { public: DataRecorder(const FileSpec &filename, std::error_code &ec) @@ -199,24 +203,88 @@ class DataRecorder : public AbstractRecorder { } }; -class CommandProvider : public Provider<CommandProvider> { +/// Recorder that records its data as YAML to a file. +class YamlRecorder : public AbstractRecorder { +public: + YamlRecorder(const FileSpec &filename, std::error_code &ec) + : AbstractRecorder(filename, ec) {} + + static llvm::Expected<std::unique_ptr<YamlRecorder>> + Create(const FileSpec &filename); + + template <typename T> void Record(const T &t) { + if (!m_record) + return; + llvm::yaml::Output yout(m_os); + // The YAML traits are defined as non-const because they are used for + // serialization and deserialization. The cast is safe because + // serialization doesn't modify the object. + yout << const_cast<T &>(t); + m_os.flush(); + } +}; + +/// The MultiProvider is a provider that hands out recorder which can be used +/// to capture data for diff erent instances of the same object. The recorders +/// can be passed around or stored as an instance member. +/// +/// The Info::file for the MultiProvider contains an index of files for every +/// recorder. Use the MultiLoader to read the index and get the individual +/// files. +template <typename T, typename V> +class MultiProvider : public repro::Provider<V> { +public: + MultiProvider(const FileSpec &directory) : Provider<V>(directory) {} + + T *GetNewRecorder() { + std::size_t i = m_recorders.size() + 1; + std::string filename = (llvm::Twine(V::Info::name) + llvm::Twine("-") + + llvm::Twine(i) + llvm::Twine(".yaml")) + .str(); + auto recorder_or_error = + T::Create(this->GetRoot().CopyByAppendingPathComponent(filename)); + if (!recorder_or_error) { + llvm::consumeError(recorder_or_error.takeError()); + return nullptr; + } + + m_recorders.push_back(std::move(*recorder_or_error)); + return m_recorders.back().get(); + } + + void Keep() override { + std::vector<std::string> files; + for (auto &recorder : m_recorders) { + recorder->Stop(); + files.push_back(recorder->GetFilename().GetPath()); + } + + FileSpec file = this->GetRoot().CopyByAppendingPathComponent(V::Info::file); + std::error_code ec; + llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text); + if (ec) + return; + llvm::yaml::Output yout(os); + yout << files; + } + + void Discard() override { m_recorders.clear(); } + +private: + std::vector<std::unique_ptr<T>> m_recorders; +}; + +class CommandProvider : public MultiProvider<DataRecorder, CommandProvider> { public: struct Info { static const char *name; static const char *file; }; - CommandProvider(const FileSpec &directory) : Provider(directory) {} - - DataRecorder *GetNewDataRecorder(); - - void Keep() override; - void Discard() override; + CommandProvider(const FileSpec &directory) + : MultiProvider<DataRecorder, CommandProvider>(directory) {} static char ID; - -private: - std::vector<std::unique_ptr<DataRecorder>> m_data_recorders; }; /// The generator is responsible for the logic needed to generate a @@ -360,6 +428,8 @@ class Reproducer { mutable std::mutex m_mutex; }; +/// Loader for data captured with the MultiProvider. It will read the index and +/// return the path to the files in the index. template <typename T> class MultiLoader { public: MultiLoader(std::vector<std::string> files) : m_files(files) {} diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index 978d0befa5ff..5f62987f37da 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -313,7 +313,7 @@ SBError SBDebugger::SetInputFile(SBFile file) { repro::DataRecorder *recorder = nullptr; if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) - recorder = g->GetOrCreate<repro::CommandProvider>().GetNewDataRecorder(); + recorder = g->GetOrCreate<repro::CommandProvider>().GetNewRecorder(); FileSP file_sp = file.m_opaque_sp; diff --git a/lldb/source/Utility/Reproducer.cpp b/lldb/source/Utility/Reproducer.cpp index eb7d96baab57..7620ab2c389d 100644 --- a/lldb/source/Utility/Reproducer.cpp +++ b/lldb/source/Utility/Reproducer.cpp @@ -272,40 +272,15 @@ DataRecorder::Create(const FileSpec &filename) { return std::move(recorder); } -DataRecorder *CommandProvider::GetNewDataRecorder() { - std::size_t i = m_data_recorders.size() + 1; - std::string filename = (llvm::Twine(Info::name) + llvm::Twine("-") + - llvm::Twine(i) + llvm::Twine(".yaml")) - .str(); - auto recorder_or_error = - DataRecorder::Create(GetRoot().CopyByAppendingPathComponent(filename)); - if (!recorder_or_error) { - llvm::consumeError(recorder_or_error.takeError()); - return nullptr; - } - - m_data_recorders.push_back(std::move(*recorder_or_error)); - return m_data_recorders.back().get(); -} - -void CommandProvider::Keep() { - std::vector<std::string> files; - for (auto &recorder : m_data_recorders) { - recorder->Stop(); - files.push_back(recorder->GetFilename().GetPath()); - } - - FileSpec file = GetRoot().CopyByAppendingPathComponent(Info::file); +llvm::Expected<std::unique_ptr<YamlRecorder>> +YamlRecorder::Create(const FileSpec &filename) { std::error_code ec; - llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text); + auto recorder = std::make_unique<YamlRecorder>(std::move(filename), ec); if (ec) - return; - yaml::Output yout(os); - yout << files; + return llvm::errorCodeToError(ec); + return std::move(recorder); } -void CommandProvider::Discard() { m_data_recorders.clear(); } - void VersionProvider::Keep() { FileSpec file = GetRoot().CopyByAppendingPathComponent(Info::file); std::error_code ec; diff --git a/lldb/unittests/Utility/ReproducerTest.cpp b/lldb/unittests/Utility/ReproducerTest.cpp index 1ada0b32827c..5a9dea3450f0 100644 --- a/lldb/unittests/Utility/ReproducerTest.cpp +++ b/lldb/unittests/Utility/ReproducerTest.cpp @@ -9,6 +9,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/Support/Error.h" #include "llvm/Testing/Support/Error.h" @@ -31,8 +32,25 @@ class DummyProvider : public repro::Provider<DummyProvider> { static char ID; }; +class YamlMultiProvider + : public MultiProvider<YamlRecorder, YamlMultiProvider> { +public: + struct Info { + static const char *name; + static const char *file; + }; + + YamlMultiProvider(const FileSpec &directory) : MultiProvider(directory) {} + + static char ID; +}; + const char *DummyProvider::Info::name = "dummy"; const char *DummyProvider::Info::file = "dummy.yaml"; +const char *YamlMultiProvider::Info::name = "mutli"; +const char *YamlMultiProvider::Info::file = "mutli.yaml"; +char DummyProvider::ID = 0; +char YamlMultiProvider::ID = 0; class DummyReproducer : public Reproducer { public: @@ -42,7 +60,25 @@ class DummyReproducer : public Reproducer { using Reproducer::SetReplay; }; -char DummyProvider::ID = 0; +struct YamlData { + YamlData() : i(-1) {} + YamlData(int i) : i(i) {} + int i; +}; + +inline bool operator==(const YamlData &LHS, const YamlData &RHS) { + return LHS.i == RHS.i; +} + +LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(YamlData) + +namespace llvm { +namespace yaml { +template <> struct MappingTraits<YamlData> { + static void mapping(IO &io, YamlData &Y) { io.mapRequired("i", Y.i); }; +}; +} // namespace yaml +} // namespace llvm TEST(ReproducerTest, SetCapture) { DummyReproducer reproducer; @@ -144,3 +180,83 @@ TEST(GeneratorTest, GetOrCreate) { auto &provider_alt = generator.GetOrCreate<DummyProvider>(); EXPECT_EQ(&provider, &provider_alt); } + +TEST(GeneratorTest, YamlMultiProvider) { + SmallString<128> root; + std::error_code ec = llvm::sys::fs::createUniqueDirectory("reproducer", root); + ASSERT_FALSE(static_cast<bool>(ec)); + + auto cleanup = llvm::make_scope_exit( + [&] { EXPECT_FALSE(llvm::sys::fs::remove_directories(root.str())); }); + + YamlData data0(0); + YamlData data1(1); + YamlData data2(2); + YamlData data3(3); + + { + DummyReproducer reproducer; + EXPECT_THAT_ERROR(reproducer.SetCapture(FileSpec(root.str())), Succeeded()); + + auto &generator = *reproducer.GetGenerator(); + auto *provider = generator.Create<YamlMultiProvider>(); + ASSERT_NE(nullptr, provider); + + auto *recorder = provider->GetNewRecorder(); + ASSERT_NE(nullptr, recorder); + recorder->Record(data0); + recorder->Record(data1); + + recorder = provider->GetNewRecorder(); + ASSERT_NE(nullptr, recorder); + recorder->Record(data2); + recorder->Record(data3); + + generator.Keep(); + } + + { + DummyReproducer reproducer; + EXPECT_THAT_ERROR(reproducer.SetReplay(FileSpec(root.str())), Succeeded()); + + auto &loader = *reproducer.GetLoader(); + std::unique_ptr<repro::MultiLoader<YamlMultiProvider>> multi_loader = + repro::MultiLoader<YamlMultiProvider>::Create(&loader); + + // Read the first file. + { + llvm::Optional<std::string> file = multi_loader->GetNextFile(); + EXPECT_TRUE(static_cast<bool>(file)); + + auto buffer = llvm::MemoryBuffer::getFile(*file); + EXPECT_TRUE(static_cast<bool>(buffer)); + + yaml::Input yin((*buffer)->getBuffer()); + std::vector<YamlData> data; + yin >> data; + + ASSERT_EQ(data.size(), 2U); + EXPECT_THAT(data, testing::ElementsAre(data0, data1)); + } + + // Read the second file. + { + llvm::Optional<std::string> file = multi_loader->GetNextFile(); + EXPECT_TRUE(static_cast<bool>(file)); + + auto buffer = llvm::MemoryBuffer::getFile(*file); + EXPECT_TRUE(static_cast<bool>(buffer)); + + yaml::Input yin((*buffer)->getBuffer()); + std::vector<YamlData> data; + yin >> data; + + ASSERT_EQ(data.size(), 2U); + EXPECT_THAT(data, testing::ElementsAre(data2, data3)); + } + + // There is no third file. + llvm::Optional<std::string> file = multi_loader->GetNextFile(); + EXPECT_FALSE(static_cast<bool>(file)); + } +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits