[lldb] Move copying of files into reproducer out of process

For performance reasons the reproducers don't copy the files captured by
the file collector eagerly, but wait until the reproducer needs to be
generated.

This is a problematic when LLDB crashes and we have to do all this
signal-unsafe work in the signal handler. This patch uses a similar
trick to clang, which has the driver invoke a new cc1 instance to do all
this work out-of-process.

This patch moves the writing of the mapping file as well as copying over
the reproducers into a separate process spawned when lldb crashes.

Differential revision: https://reviews.llvm.org/D89600

(cherry picked from commit 73811d3)

Author: JDevlieghere

lldb/include/lldb/API/SBReproducer.h

@@ -48,6 +48,7 @@ class LLDB_API SBReproducer {
   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/include/lldb/Host/FileSystem.h

@@ -34,7 +34,7 @@ class FileSystem {
   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 @@ class FileSystem {
   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 @@ class FileSystem {
 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;
 };

lldb/include/lldb/Utility/Reproducer.h

@@ -243,6 +243,9 @@ struct ReplayOptions {
   bool check_version = true;
 };
 
+llvm::Error Finalize(Loader *loader);
+llvm::Error Finalize(const FileSpec &root);
+
 } // namespace repro
 } // namespace lldb_private
 

lldb/include/lldb/Utility/ReproducerProvider.h

@@ -90,38 +90,50 @@ class YamlRecorder : public AbstractRecorder {
   }
 };
 
+class FlushingFileCollector : public llvm::FileCollectorBase {
+public:
+  FlushingFileCollector(llvm::StringRef files_path, llvm::StringRef dirs_path,
+                        std::error_code &ec);
+
+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;
+
+  llvm::Optional<llvm::raw_fd_ostream> m_files_os;
+  llvm::Optional<llvm::raw_fd_ostream> m_dirs_os;
+};
+
 class FileProvider : public Provider<FileProvider> {
 public:
   struct Info {
     static const char *name;
     static const char *file;
   };
 
-  FileProvider(const FileSpec &directory)
-      : Provider(directory),
-        m_collector(std::make_shared<llvm::FileCollector>(
-            directory.CopyByAppendingPathComponent("root").GetPath(),
-            directory.GetPath())) {}
+  FileProvider(const FileSpec &directory) : Provider(directory) {
+    std::error_code ec;
+    m_collector = std::make_shared<FlushingFileCollector>(
+        directory.CopyByAppendingPathComponent("files.txt").GetPath(),
+        directory.CopyByAppendingPathComponent("dirs.txt").GetPath(), ec);
+    if (ec)
+      m_collector.reset();
+  }
 
-  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.

lldb/source/API/SBReproducer.cpp

@@ -8,7 +8,6 @@
 
 #include "SBReproducerPrivate.h"
 
-#include "SBReproducerPrivate.h"
 #include "lldb/API/LLDB.h"
 #include "lldb/API/SBAddress.h"
 #include "lldb/API/SBAttachInfo.h"
@@ -266,6 +265,27 @@ const char *SBReproducer::Replay(const char *path,
   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();
+  }
+
+  if (auto e = repro::Finalize(loader)) {
+    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 +305,11 @@ bool SBReproducer::SetAutoGenerate(bool b) {
 }
 
 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) {

lldb/source/Commands/CommandObjectReproducer.cpp

@@ -192,6 +192,10 @@ class CommandObjectReproducerGenerate : public CommandObjectParsed {
     auto &r = Reproducer::Instance();
     if (auto generator = r.GetGenerator()) {
       generator->Keep();
+      if (llvm::Error e = repro::Finalize(r.GetReproducerPath())) {
+        SetError(result, std::move(e));
+        return result.Succeeded();
+      }
     } else if (r.IsReplaying()) {
       // Make this operation a NO-OP in replay mode.
       result.SetStatus(eReturnStatusSuccessFinishNoResult);

lldb/source/Host/common/FileSystem.cpp

@@ -49,7 +49,7 @@ void FileSystem::Initialize() {
   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);
 }

lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h

@@ -18,7 +18,7 @@ class ModuleDependencyCollectorAdaptor
     : 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 @@ class ModuleDependencyCollectorAdaptor
   void writeFileMap() override {}
 
 private:
-  std::shared_ptr<llvm::FileCollector> m_file_collector;
+  std::shared_ptr<llvm::FileCollectorBase> m_file_collector;
 };
 } // namespace lldb_private
 

lldb/source/Utility/Reproducer.cpp

@@ -176,10 +176,12 @@ Generator::Generator(FileSpec root) : m_root(MakeAbsolute(std::move(root))) {
 
 Generator::~Generator() {
   if (!m_done) {
-    if (m_auto_generate)
+    if (m_auto_generate) {
       Keep();
-    else
+      llvm::cantFail(Finalize(GetRoot()));
+    } else {
       Discard();
+    }
   }
 }
 
@@ -359,3 +361,58 @@ void Verifier::Verify(
     }
   }
 }
+
+static llvm::Error addPaths(StringRef path,
+                            function_ref<void(StringRef)> callback) {
+  auto buffer = llvm::MemoryBuffer::getFile(path);
+  if (!buffer)
+    return errorCodeToError(buffer.getError());
+
+  SmallVector<StringRef, 0> paths;
+  (*buffer)->getBuffer().split(paths, '\0');
+  for (StringRef p : paths) {
+    if (!p.empty())
+      callback(p);
+  }
+
+  return errorCodeToError(llvm::sys::fs::remove(path));
+}
+
+llvm::Error repro::Finalize(Loader *loader) {
+  if (!loader)
+    return make_error<StringError>("invalid loader",
+                                   llvm::inconvertibleErrorCode());
+
+  FileSpec reproducer_root = 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 (Error e =
+          addPaths(files_path, [&](StringRef p) { collector.addFile(p); }))
+    return e;
+
+  if (Error e =
+          addPaths(dirs_path, [&](StringRef p) { collector.addDirectory(p); }))
+    return e;
+
+  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();
+}
+
+llvm::Error repro::Finalize(const FileSpec &root) {
+  Loader loader(root);
+  if (Error e = loader.LoadIndex())
+    return e;
+  return Finalize(&loader);
+}

lldb/source/Utility/ReproducerProvider.cpp

@@ -8,6 +8,7 @@
 
 #include "lldb/Utility/ReproducerProvider.h"
 #include "lldb/Utility/ProcessInfo.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
@@ -44,6 +45,40 @@ void VersionProvider::Keep() {
   os << m_version << "\n";
 }
 
+FlushingFileCollector::FlushingFileCollector(llvm::StringRef files_path,
+                                             llvm::StringRef dirs_path,
+                                             std::error_code &ec) {
+  auto clear = llvm::make_scope_exit([this]() {
+    m_files_os.reset();
+    m_dirs_os.reset();
+  });
+  m_files_os.emplace(files_path, ec, llvm::sys::fs::OF_Append);
+  if (ec)
+    return;
+  m_dirs_os.emplace(dirs_path, ec, llvm::sys::fs::OF_Append);
+  if (ec)
+    return;
+  clear.release();
+}
+
+void FlushingFileCollector::addFileImpl(StringRef file) {
+  if (m_files_os) {
+    *m_files_os << file << '\0';
+    m_files_os->flush();
+  }
+}
+
+llvm::vfs::directory_iterator
+FlushingFileCollector::addDirectoryImpl(const Twine &dir,
+                                        IntrusiveRefCntPtr<vfs::FileSystem> vfs,
+                                        std::error_code &dir_ec) {
+  if (m_dirs_os) {
+    *m_dirs_os << dir << '\0';
+    m_dirs_os->flush();
+  }
+  return vfs->dir_begin(dir, dir_ec);
+}
+
 void FileProvider::RecordInterestingDirectory(const llvm::Twine &dir) {
   if (m_collector)
     m_collector->addFile(dir);

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

lldb/test/Shell/Reproducer/TestFinalize.test

@@ -0,0 +1,14 @@
+# RUN: mkdir -p %t.repro
+# RUN: touch %t.known.file
+# RUN: mkdir -p %t.known.dir
+# RUN: touch %t.repro/index.yaml
+# RUN: echo -n "%t.known.file" > %t.repro/files.txt
+# RUN: echo -n "%t.known.dir" > %t.repro/dirs.txt
+
+# RUN: %lldb --reproducer-finalize %t.repro 2>&1 | FileCheck %s
+# CHECK-NOT: error
+# CHECK: Reproducer written to
+
+# RUN: echo "CHECK-DAG: %t.known.file" > %t.filecheck
+# RUN: echo "CHECK-DAG %t.known.dir" >> %t.filecheck
+# RUN: cat %t.repro/files.yaml | FileCheck %t.filecheck