[lldb] Move copying of reproducer out of process

[JDevlieghere]

Summary

For performance reasons the reproducers don't copy the files captured by the file collector until we decide that the reproducer needs to be kept (i.e. generated). This is a problematic when LLDB crashes and we need to do this signal-unsafe work in the signal handler. This patch uses the same trick as clang and reinvokes itself to copy the files out of process.

[labath]

Hm... Are you sure that this will help with anything? IIUC, the serialization of the VFS file map still happens in the context of the crashing process. This requires walking a bunch of data structures with liberally-asserting code, which will abort on any inconsistency it runs into. If *that* succeeds, I'd expect that the copying of the files will succeed too.

I think it would be better if we moved the vfs map construction into the other process too. Maybe via something like this:

• The first process opens a "log" file, which will contain the list of files to be copied. For added safety the file should be opened append-only.
• As it works, it writes file names to this file.
• Upon a crash, the re-execed process reads this file, constructs the vfs map, and copies all necessary files. It should be prepared to handle/ignore any garbled entries, and the format of the file should make this easy. (Since the log file must by definition contain enough information to recreate the vfs map, it might be simpler to not write out the map, but just always recreate it in memory when replaying.)

lldb/tools/driver/Driver.cpp

857

I guess this won't work if there are spaces anywhere in the path..

lldb/tools/driver/Options.td

235

Should this be a hidden argument?

[JDevlieghere]

In D89600#2337868, @labath wrote:

Hm... Are you sure that this will help with anything? IIUC, the serialization of the VFS file map still happens in the context of the crashing process. This requires walking a bunch of data structures with liberally-asserting code, which will abort on any inconsistency it runs into. If *that* succeeds, I'd expect that the copying of the files will succeed too.

I only have a handful of crash logs to base myself off. Currently the walking and the copying are intertwined, so I can't say for sure. My idea was to give this a try and see where that got us and make more invasive changes (like what you proposed) if needed. But since you sound pretty convinced that we'll need it anyway I will extend the patch.

I think it would be better if we moved the vfs map construction into the other process too. Maybe via something like this:

• The first process opens a "log" file, which will contain the list of files to be copied. For added safety the file should be opened append-only.
• As it works, it writes file names to this file.
• Upon a crash, the re-execed process reads this file, constructs the vfs map, and copies all necessary files. It should be prepared to handle/ignore any garbled entries, and the format of the file should make this easy. (Since the log file must by definition contain enough information to recreate the vfs map, it might be simpler to not write out the map, but just always recreate it in memory when replaying.)

Yes, that would be similar to how we immediately flush GDB remote packets to disk. It would require adding some "immediate" mode to the FileCollector and an API to convert between the formats. I think the new format could be just host paths separated by newlines.

lldb/tools/driver/Options.td

235

Yep

[JDevlieghere]

• 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

[labath]

In D89600#2338996, @JDevlieghere wrote:

In D89600#2337868, @labath wrote:

Hm... Are you sure that this will help with anything? IIUC, the serialization of the VFS file map still happens in the context of the crashing process. This requires walking a bunch of data structures with liberally-asserting code, which will abort on any inconsistency it runs into. If *that* succeeds, I'd expect that the copying of the files will succeed too.

I only have a handful of crash logs to base myself off. Currently the walking and the copying are intertwined, so I can't say for sure. My idea was to give this a try and see where that got us and make more invasive changes (like what you proposed) if needed. But since you sound pretty convinced that we'll need it anyway I will extend the patch.

Well.. I'd expect that the reason why saving of the reproducer fails is because we corrupt the heap/stack before we finally crash. Copying files is basically a bunch of syscalls (though some of our abstractions get in the way), so it wouldn't normally be affected by this. Our ability to _find_ the files that we're supposed to copy is a different matter...

That said, this doesn't seem significantly more invasive than the previous approach.

I think it would be better if we moved the vfs map construction into the other process too. Maybe via something like this:

• The first process opens a "log" file, which will contain the list of files to be copied. For added safety the file should be opened append-only.
• As it works, it writes file names to this file.
• Upon a crash, the re-execed process reads this file, constructs the vfs map, and copies all necessary files. It should be prepared to handle/ignore any garbled entries, and the format of the file should make this easy. (Since the log file must by definition contain enough information to recreate the vfs map, it might be simpler to not write out the map, but just always recreate it in memory when replaying.)

Yes, that would be similar to how we immediately flush GDB remote packets to disk. It would require adding some "immediate" mode to the FileCollector and an API to convert between the formats. I think the new format could be just host paths separated by newlines.

A nul character is better than a newline, as paths can contain newline characters.

lldb/include/lldb/Utility/Reproducer.h

241–256

This appears to be over-engineered. Is there a reason plain functions won't do?

llvm::Error FinalizeReproducer(Loader&);
llvm::Error FinalizeReproducerInProcess(FileSpec); // calls the first one

That said, I'm not sure "InProcess" captures the distinction very well, as both are "in process" at the point where they are used. Maybe name them according to what they do (as in, whether they generate a full reproducer, or one that needs to be fixed up afterwards)

lldb/source/Utility/Reproducer.cpp

388

Is this deliberately ignoring a possible error?

lldb/source/Utility/ReproducerProvider.cpp

54

So, this reopens the file every time a file is added? That is suboptimal... Why not just open it once and keep it that way? That would also enable handling of error during opening of files.

66–68

if(!ec) os<<dir; ?

lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test

1

Any chance of testing the finalization code by letting it loose on an hand-generated unfinalized reproducer?

lldb/tools/driver/Driver.cpp

734–735

this is really cryptic. Maybe renaming cmd to finalize_cmd would help?

[JDevlieghere]

Address Pavel's code review feedback

[labath]

Looks good.

lldb/source/Utility/Reproducer.cpp

371

I'd expect this to contain more than 20 paths in a typical session, so maybe just use zero?

lldb/source/Utility/ReproducerProvider.cpp

67

Flushing also makes things slower, so there's a tradeoff between speed an accuracy to consider. Theoretically we could play with the size of the stream buffer to fine tune that...

lldb/test/Shell/Reproducer/TestFinalize.test

6–7

s/printf/echo. printf is not needed here, and I'm not sure it would work on windows.

[JDevlieghere]

lldb/source/Utility/Reproducer.cpp

388

It was because we were only opening/creating the file when adding to it but that's no longer the case.

lldb/test/Shell/Reproducer/TestFinalize.test

6–7

It was to avoid the newline, but I can pass -n.

lldb/include/lldb/API/SBReproducer.h

 class LLDB_API SBReproducer {
 public:
   static const char *Capture();
   static const char *Capture(const char *path);
   static const char *Replay(const char *path);
   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();
 
   /// The working directory is set to the current working directory when the
   /// reproducers are initialized. This method allows setting a different
   /// working directory. This is used by the API test suite  which temporarily
   /// changes the directory to where the test lives. This is a NO-OP in every
   /// mode but capture.
   static void SetWorkingDirectory(const char *path);
 };
 
 } // namespace lldb
 
 #endif

lldb/include/lldb/Host/FileSystem.h

 class FileSystem {
 public:
   static const char *DEV_NULL;
   static const char *PATH_CONVERSION_ERROR;
 
   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,
              bool mapped = false)
       : m_fs(std::move(fs)), m_collector(nullptr), m_home_directory(),
         m_mapped(mapped) {}
 
   FileSystem(const FileSystem &fs) = delete;
   FileSystem &operator=(const FileSystem &fs) = delete;
 
   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();
 
   Status Symlink(const FileSpec &src, const FileSpec &dst);
   Status Readlink(const FileSpec &src, FileSpec &dst);
 
   Status ResolveSymbolicLink(const FileSpec &src, FileSpec &dst);
public:
   void Collect(const FileSpec &file_spec);
   void Collect(const llvm::Twine &file);
 
   void SetHomeDirectory(std::string home_directory);
 
 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;
 };
 } // namespace lldb_private
 
 #endif

lldb/include/lldb/Utility/Reproducer.h

public:
   void Verify(llvm::function_ref<void(llvm::StringRef)> error_callback,
               llvm::function_ref<void(llvm::StringRef)> warning_callback,
               llvm::function_ref<void(llvm::StringRef)> note_callback) const;
 
 private:
   Loader *m_loader;
 };
 
 struct ReplayOptions {
   bool verify = true;
   bool check_version = true;
 };
 
+llvm::Error Finalize(Loader *loader);
+llvm::Error Finalize(const FileSpec &root);
+
 } // namespace repro
 } // namespace lldb_private
 
 #endif // LLDB_UTILITY_REPRODUCER_H

lldb/include/lldb/Utility/ReproducerProvider.h

template <typename T> void Record(const T &t) {
     // 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();
   }
 };
 
+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.
 ///
 /// When the reproducer is kept, it writes the lldb version to a file named
 /// version.txt in the reproducer root.
 class VersionProvider : public Provider<VersionProvider> {
 public:

lldb/source/API/SBReproducer.cpp

 //===-- SBReproducer.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 "SBReproducerPrivate.h"
 
-#include "SBReproducerPrivate.h"
 #include "lldb/API/LLDB.h"
 #include "lldb/API/SBAddress.h"
 #include "lldb/API/SBAttachInfo.h"
 #include "lldb/API/SBBlock.h"
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBCommandInterpreterRunOptions.h"
 #include "lldb/API/SBData.h"
const char *SBReproducer::Replay(const char *path,
   }
 
   SBRegistry registry;
   registry.Replay(file);
 
   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()) {
     generator->Keep();
     return true;
   }
   return false;
 }
 
 bool SBReproducer::SetAutoGenerate(bool b) {
   auto &r = Reproducer::Instance();
   if (auto generator = r.GetGenerator()) {
     generator->SetAutoGenerate(b);
     return true;
   }
   return false;
 }
 
 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) {
   if (auto *g = lldb_private::repro::Reproducer::Instance().GetGenerator()) {
     auto &wp = g->GetOrCreate<repro::WorkingDirectoryProvider>();
     wp.SetDirectory(path);
     auto &fp = g->GetOrCreate<repro::FileProvider>();
     fp.RecordInterestingDirectory(wp.GetDirectory());
   }
 }
 
 char lldb_private::repro::SBProvider::ID = 0;
 const char *SBProvider::Info::name = "sbapi";
 const char *SBProvider::Info::file = "sbapi.bin";

lldb/source/Commands/CommandObjectReproducer.cpp

if (!command.empty()) {
       result.AppendErrorWithFormat("'%s' takes no arguments",
                                    m_cmd_name.c_str());
       return false;
     }
 
     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);
       return result.Succeeded();
     } else {
       result.AppendErrorWithFormat("Unable to get the reproducer generator");
       result.SetStatus(eReturnStatusFailed);
       return false;

lldb/source/Host/common/FileSystem.cpp

 
 FileSystem &FileSystem::Instance() { return *InstanceImpl(); }
 
 void FileSystem::Initialize() {
   lldbassert(!InstanceImpl() && "Already initialized.");
   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);
 }
 
 llvm::Error FileSystem::Initialize(const FileSpec &mapping) {
   lldbassert(!InstanceImpl() && "Already initialized.");
 
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> buffer =

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

 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FileCollector.h"
 
 namespace lldb_private {
 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) {
   }
 
   void addFile(llvm::StringRef Filename,
                llvm::StringRef FileDst = {}) override {
     if (m_file_collector)
       m_file_collector->addFile(Filename);
   }
 
   bool insertSeen(llvm::StringRef Filename) override { return false; }
   void addFileMapping(llvm::StringRef VPath, llvm::StringRef RPath) override {}
   void writeFileMap() override {}
 
 private:
-  std::shared_ptr<llvm::FileCollector> m_file_collector;
+  std::shared_ptr<llvm::FileCollectorBase> m_file_collector;
 };
 } // namespace lldb_private
 
 #endif

lldb/source/Utility/Reproducer.cpp

 
 Generator::Generator(FileSpec root) : m_root(MakeAbsolute(std::move(root))) {
   GetOrCreate<repro::WorkingDirectoryProvider>();
   GetOrCreate<repro::HomeDirectoryProvider>();
 }
 
 Generator::~Generator() {
   if (!m_done) {
-    if (m_auto_generate)
+    if (m_auto_generate) {
       Keep();
-    else
+      llvm::cantFail(Finalize(GetRoot()));
+    } else {
       Discard();
     }
   }
+}
 
 ProviderBase *Generator::Register(std::unique_ptr<ProviderBase> provider) {
   std::lock_guard<std::mutex> lock(m_providers_mutex);
   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();
 }
for (llvm::StringRef root : roots) {
     for (; iter != end && !ec; iter.increment(ec)) {
       ErrorOr<vfs::Status> status = vfs->status(iter->path());
       if (!status)
         note_callack("'" + iter->path().str() +
                      "': " + status.getError().message());
     }
   }
 }
+
+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

 //===-- 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/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"
 
 using namespace lldb_private;
 using namespace lldb_private::repro;
 using namespace llvm;
 using namespace llvm::yaml;
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";
 }
 
+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);
 }
 
 void FileProvider::RecordInterestingDirectoryRecursive(const llvm::Twine &dir) {
   if (m_collector)
     m_collector->addDirectory(dir);

lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test

 # This tests the LLDB_CAPTURE_REPRODUCER override.
 
 # RUN: %lldb -b -o 'reproducer status' --capture --capture-path %t.repro /bin/ls | FileCheck %s --check-prefix CAPTURE
 # RUN: %lldb -b -o 'reproducer status' --capture | FileCheck %s --check-prefix CAPTURE
 
 # RUN: LLDB_CAPTURE_REPRODUCER=1 %lldb -b -o 'reproducer status' | FileCheck %s --check-prefix CAPTURE
 # RUN: LLDB_CAPTURE_REPRODUCER=ON %lldb -b -o 'reproducer status' | FileCheck %s --check-prefix CAPTURE
 # RUN: LLDB_CAPTURE_REPRODUCER=on %lldb -b -o 'reproducer status' | FileCheck %s --check-prefix CAPTURE
 
 # 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
 
 # CAPTURE: Reproducer is in capture mode.
 # OFF: Reproducer is off.

lldb/test/Shell/Reproducer/TestFinalize.test

+# 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

lldb/tools/driver/Driver.cpp

void sigcont_handler(int signo) {
   if (g_driver != nullptr)
     g_driver->GetDebugger().RestoreInputTerminalState();
 
   signal(signo, SIG_DFL);
   kill(getpid(), signo);
   signal(signo, sigcont_handler);
 }
 
-void reproducer_handler(void *argv0) {
+void reproducer_handler(void *finalize_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 *>(finalize_cmd));
+    fflush(stdout);
   }
 }
 
 static void printHelp(LLDBOptTable &table, llvm::StringRef tool_name) {
   std::string usage_str = tool_name.str() + " [options]";
   table.PrintHelp(llvm::outs(), usage_str.c_str(), "LLDB", false);
 
   std::string examples = R"___(
EXAMPLES:
 
   Note: In REPL mode no file is loaded, so commands specified to run after
   loading the file (via -o or -s) will be ignored.)___";
   llvm::outs() << examples << '\n';
 }
 
 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));
     replay_options.SetVerify(!input_args.hasArg(OPT_no_verification));
     if (const char *error =
             SBReproducer::Replay(replay_path->getValue(), replay_options)) {
       WithColor::error() << "reproducer replay failed: " << error << '\n';
       return 1;
     }
     return 0;
   }
 
   bool capture = input_args.hasArg(OPT_capture);
   bool generate_on_exit = input_args.hasArg(OPT_generate_on_exit);
   auto *capture_path = input_args.getLastArg(OPT_capture_path);
 
   if (generate_on_exit && !capture) {
     WithColor::warning()
         << "-reproducer-generate-on-exit specified without -capture\n";
   }
 
   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";
       if (const char *error = SBReproducer::Capture(capture_path->getValue())) {
         WithColor::error() << "reproducer capture failed: " << error << '\n';
         return 1;
       }
     } else {
       const char *error = SBReproducer::Capture();
       if (error) {
         WithColor::error() << "reproducer capture failed: " << error << '\n';
         return 1;
       }
     }
     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 *finalize_cmd = new std::string(argv0);
+        finalize_cmd->append(" --reproducer-finalize '");
+        finalize_cmd->append(reproducer_path);
+        finalize_cmd->append("'");
+        llvm::sys::AddSignalHandler(reproducer_handler,
+                                    const_cast<char *>(finalize_cmd->c_str()));
+      }
+    }
   }
 
   return llvm::None;
 }
 
 int main(int argc, char const *argv[]) {
   // Setup LLVM signal handlers and make sure we call llvm_shutdown() on
   // destruction.

lldb/tools/driver/Options.td

 def capture: F<"capture">,
   HelpText<"Tells the debugger to capture a reproducer.">;
 def capture_path: Separate<["--", "-"], "capture-path">,
   MetaVarName<"<filename>">,
   HelpText<"Tells the debugger to use the given filename for the reproducer.">;
 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">,
   HelpText<"Disable the reproducer verification.">;
 def no_generate_on_signal: F<"reproducer-no-generate-on-signal">,
   HelpText<"Don't generate reproducer when a signal is received.">;
 def generate_on_exit: F<"reproducer-generate-on-exit">,
   HelpText<"Generate reproducer on exit.">;
 
 def REM : R<["--"], "">;