[ldb/Reproducers] Add YamlRecorder and MultiProvider

[JDevlieghere]

Summary

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.

This is the basis of a future patch that I'm working on which will implement a Provider through the YamlRecorder.

[JDevlieghere]

Use ScopeExit to cleanup reproducer after test completes.

[labath]

This seems fine. A bit more doxygen wouldn't hurt...

lldb/source/Utility/Reproducer.cpp

10

It doesn't look like this is actually needed.

lldb/unittests/Utility/ReproducerTest.cpp

64–65

superfluous semicolons

73–74

Are both of these needed? I would expect the first one to be enough...

238–240

EXPECT_THAT(data, testing::ElementsAre(data0, data1));

lldb/include/lldb/Utility/Reproducer.h

struct Info {
     static const char *name;
     static const char *file;
   };
   void Keep() override;
   std::string m_cwd;
   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)
       : m_filename(filename.GetFilename().GetStringRef()),
         m_os(filename.GetPath(), ec, llvm::sys::fs::OF_Text), m_record(true) {}
 
 public:
   const FileSpec &GetFilename() { return m_filename; }
 
   void Stop() {
     assert(m_record);
     m_record = false;
   }
 
 private:
   FileSpec m_filename;
 
 protected:
   llvm::raw_fd_ostream m_os;
   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)
       : AbstractRecorder(filename, ec) {}
 
   static llvm::Expected<std::unique_ptr<DataRecorder>>
   Create(const FileSpec &filename);
 
   template <typename T> void Record(const T &t, bool newline = false) {
     if (!m_record)
       return;
     m_os << t;
     if (newline)
       m_os << '\n';
     m_os.flush();
   }
 };
 
-class CommandProvider : public Provider<CommandProvider> {
+/// Recorder that records its data as YAML to a file.
+class YamlRecorder : public AbstractRecorder {
 public:
-  struct Info {
-    static const char *name;
-    static const char *file;
-  };
+  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();
+  }
 
-  CommandProvider(const FileSpec &directory) : Provider(directory) {}
+/// The MultiProvider is a provider that hands out recorder which can be used
+/// to capture data for different 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;
+    }
 
-  DataRecorder *GetNewDataRecorder();
+    m_recorders.push_back(std::move(*recorder_or_error));
+    return m_recorders.back().get();
+  }
 
-  void Keep() override;
-  void Discard() override;
+  void Keep() override {
+    std::vector<std::string> files;
+    for (auto &recorder : m_recorders) {
+      recorder->Stop();
+      files.push_back(recorder->GetFilename().GetPath());
+    }
 
-  static char ID;
+    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<DataRecorder>> m_data_recorders;
+  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)
+      : MultiProvider<DataRecorder, CommandProvider>(directory) {}
+
+  static char ID;
 };
 
 /// The generator is responsible for the logic needed to generate a
 /// reproducer. For doing so it relies on providers, who serialize data that
 /// is necessary for reproducing  a failure.
 class Generator final {
 
 public:
private:
   static llvm::Optional<Reproducer> &InstanceImpl();
 
   llvm::Optional<Generator> m_generator;
   llvm::Optional<Loader> m_loader;
 
   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) {}
 
   static std::unique_ptr<MultiLoader> Create(Loader *loader) {
     if (!loader)
       return {};
 

lldb/source/API/SBDebugger.cpp

SBError SBDebugger::SetInputFile(SBFile file) {
   SBError error;
   if (!m_opaque_sp) {
     error.ref().SetErrorString("invalid debugger");
     return LLDB_RECORD_RESULT(error);
   }
 
   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;
 
   static std::unique_ptr<repro::MultiLoader<repro::CommandProvider>> loader =
       repro::MultiLoader<repro::CommandProvider>::Create(
           repro::Reproducer::Instance().GetLoader());
   if (loader) {
     llvm::Optional<std::string> nextfile = loader->GetNextFile();

lldb/source/Utility/Reproducer.cpp

 //===-- Reproducer.cpp ----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/LLDBAssert.h"
 
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace lldb_private;
 using namespace lldb_private::repro;
 using namespace llvm;

 DataRecorder::Create(const FileSpec &filename) {
   std::error_code ec;
   auto recorder = std::make_unique<DataRecorder>(std::move(filename), ec);
   if (ec)
     return llvm::errorCodeToError(ec);
   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;
   llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text);
   if (ec)
     return;
   os << m_version << "\n";
 }

lldb/unittests/Utility/ReproducerTest.cpp

 //===-- ReproducerTest.cpp ------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Error.h"
 
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Reproducer.h"
 
 using namespace llvm;
 using namespace lldb_private;
 using namespace lldb_private::repro;
 
 class DummyProvider : public repro::Provider<DummyProvider> {
 public:
   struct Info {
     static const char *name;
     static const char *file;
   };
 
   DummyProvider(const FileSpec &directory) : Provider(directory) {}
 
   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:
   DummyReproducer() : Reproducer(){};
 
   using Reproducer::SetCapture;
   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;
 
   // Initially both generator and loader are unset.
   EXPECT_EQ(nullptr, reproducer.GetGenerator());
   EXPECT_EQ(nullptr, reproducer.GetLoader());
 
TEST(GeneratorTest, GetOrCreate) {
 
   auto &provider = generator.GetOrCreate<DummyProvider>();
   EXPECT_EQ(FileSpec("//bogus/path", FileSpec::Style::posix),
             provider.GetRoot());
 
   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));
+  }
+}