[Reproducers] Add file provider

[JDevlieghere]

Summary

This patch adds the file provider which is responsible for capturing files used by LLDB.

When capturing a reproducer, we use a file collector that is very similar to the one used in clang. For every file that we touch, we add an entry with a mapping from its virtual to its real path. When we decide to generate a reproducer we copy over the files and their permission into to reproducer folder.

When replaying a reproducer, we load the VFS mapping and instantiate a RedirectingFileSystem. The latter will transparently use the files available in the reproducer.

I've tested this on two macOS machines with an artificial example. Still, it is very likely that I missed some places where we (still) use native file system calls. I'm hoping to flesh those out while testing with more advanced examples. However, I will fix those things in separate patches.

[JDevlieghere]

• Initialization of the FS needs to happen in the driver.
• All file access has to go through the new FileSystem.
• Needs tests.

[labath]

I am confused by differing scopes of various objects that are interacting here. It seems you are implementing capture/replay as something that can be flicked on/off at any point during a debug session (Debugger lifetime). However, you are modifying global state (the filesystem singleton, at least), which is shared by all debug sessions. If multiple debug sessions try to enable capture/replay (or even access the filesystem concurrently, as none of this is synchronized in any way), things will break horribly. This seems like a bad starting point in implementing a feature, as I don't see any way to fix incrementally in the future.

Maybe you meant that by Initialization of the FS needs to happen in the driver.? In any case, for me the initialization part is the interesting/hard part of this problem. The details of how to collect the files are trivial compared to that and possibly depend on the result of the decisions made there.

[JDevlieghere]

In D54617#1302366, @labath wrote:

I am confused by differing scopes of various objects that are interacting here. It seems you are implementing capture/replay as something that can be flicked on/off at any point during a debug session (Debugger lifetime). However, you are modifying global state (the filesystem singleton, at least), which is shared by all debug sessions. If multiple debug sessions try to enable capture/replay (or even access the filesystem concurrently, as none of this is synchronized in any way), things will break horribly. This seems like a bad starting point in implementing a feature, as I don't see any way to fix incrementally in the future.

Maybe I should've added a comment (I didn't want to litter the code with design goals) but the goal of the commands is not really to enable/disable the reproducer feature. I'm currently abusing the commands to do that, but the long term goal is that you can enable/disable part of the reproducer. For example, you could say only capture GDB packets or capture only files. It makes testing a lot easier if you can isolate a single source of information. Since there's currently only GDB packets I didn't split it up (yet).

Also I piggy-backed off this to enable/disable the feature as a whole because I needed to go through the debugger. See my reply below on why and how that'll be fixed by the next patch :-)

Maybe you meant that by Initialization of the FS needs to happen in the driver.? In any case, for me the initialization part is the interesting/hard part of this problem. The details of how to collect the files are trivial compared to that and possibly depend on the result of the decisions made there.

Yup, as per your suggestion in another review. I have an (unfinished) patch that splits off parsing of the command line arguments in the driver so we know the reproducer path before we initialize anything. That's pretty much the whole reason that we currently go through the debugger right now.

[labath]

In D54617#1302376, @JDevlieghere wrote:

In D54617#1302366, @labath wrote:

I am confused by differing scopes of various objects that are interacting here. It seems you are implementing capture/replay as something that can be flicked on/off at any point during a debug session (Debugger lifetime). However, you are modifying global state (the filesystem singleton, at least), which is shared by all debug sessions. If multiple debug sessions try to enable capture/replay (or even access the filesystem concurrently, as none of this is synchronized in any way), things will break horribly. This seems like a bad starting point in implementing a feature, as I don't see any way to fix incrementally in the future.

Maybe I should've added a comment (I didn't want to litter the code with design goals) but the goal of the commands is not really to enable/disable the reproducer feature. I'm currently abusing the commands to do that, but the long term goal is that you can enable/disable part of the reproducer. For example, you could say only capture GDB packets or capture only files. It makes testing a lot easier if you can isolate a single source of information. Since there's currently only GDB packets I didn't split it up (yet).

Also I piggy-backed off this to enable/disable the feature as a whole because I needed to go through the debugger. See my reply below on why and how that'll be fixed by the next patch :-)

Ok, that makes sense, thanks for the explanation. I guess that means that filesystem capturing (due to it being global) will then be one of the things that cannot be enabled/disabled at runtime ?

source/Host/common/FileSystem.cpp

71–74 ↗

(On Diff #174333)

Do you want to propagate this error up the stack?

source/Utility/FileCollector.cpp

31–32 ↗

(On Diff #174333)

upper_dest = path.upper(); ?

82–87 ↗

(On Diff #174333)

Is there anything which remove_leading_dotslash does, which wouldn't be done by the latter remove_dots call?

[JDevlieghere]

• Get the external path with modifying the VFS in LLVM.
• Integrate with the new integration logic.
• Add a test. (that doesn't work yet, still WIP)

[JDevlieghere]

• Rebase
• Copy over permissions
• Fix path when launching.

[JDevlieghere]

With the LLVM changes now checked-in this is ready for review.

[labath]

Overall, I think the patch is pretty good. I made a bunch of inline suggestions/questions/comments, but they're all fairly minor. From a high-level view the two comments I have are:

• I am slightly concerned about the non-temporality of this approach. Lldb does a fair amount of filesystem write operations, and this may be hard to capture in a static filesystem image. It seems you already had to work around some of these issues when you skip copying deleted files. I think that's a bridge we can cross when we reach it, but I'm just saying I see some potential trouble ahead...
• I think it would be good nice to have a more granular test for this functionality. The existing test is sort of a sledgehammer (and apparently only runs on darwin). It is not obvious from it for instance, whether it actually tests any of the special symlink-handling code you have. (I guess it does, because you needed to write that code, but it's not clear whether that will still be true one year from now, or on someone else's machine). It sounds like it shouldn't be too hard to test the finer details of this implementation via unit tests, either by going through the FileSystem, or by going straight to the FileCollector class. What do you think?

include/lldb/Host/FileSystem.h

47 ↗

(On Diff #177598)

If this pointer should always be non-null, maybe this could be a reference?

lit/Reproducer/TestFileRepro.test

1 ↗

(On Diff #177598)

Would this test be expected to work on other targets using gdb-remote process plugin too?

source/Host/common/FileSystem.cpp

100–101 ↗

(On Diff #177598)

Maybe do this in the Initialize function, so you can return errors from there?

443 ↗

(On Diff #177598)

What are the chances of making this slightly more official by making getVFSFromYAML return a IntrusiveRefCntPtr<RedirectingFileSystem> instead of a generic pointer?

source/Initialization/SystemInitializerCommon.cpp

82 ↗

(On Diff #177598)

Would it be possible to avoid magic strings here, and use something like GetProviderInfo<FileProvider>() instead?

87 ↗

(On Diff #177598)

When can this happen? Should this be an error or maybe even an assert?

[JDevlieghere]

Address Pavel's feedback.

[JDevlieghere]

I'll add a more specific test for the collector.

source/Host/common/FileSystem.cpp

443 ↗

(On Diff #177598)

I'm not sure I understand what the benefit would be, we'd still store it as a generic FileSystem in the member variable?

source/Initialization/SystemInitializerCommon.cpp

82 ↗

(On Diff #177598)

My idea was to decouple the provider (used only for capture) and the info needed for replay, but I think we can improve things by merging them into one. I don't think I've run into a situation yet where we don't have access to the provider during replay.

87 ↗

(On Diff #177598)

Yes, reproducers should still work with "some" functionality disabled. The infrastructure is not there yet, but it would be nice to test pieces individually.

[labath]

source/Host/common/FileSystem.cpp

48 ↗

(On Diff #182090)

Would it be possible to have the overloads which always succeed keep returning void? Here, you've had to add failure checks in SystemInitializerCommon, which are essentially dead code, because the error path will never be taken.

443 ↗

(On Diff #177598)

It's not a big thing, but the advantage would be that it would be possible to locally verify that the up/down-casting is correct just by looking at the FileSystem class. This way you have to peek into the implementation of getVFSFromYAML to see that it indeed always returns the right type. Also, if anyone later gets the idea of changing the type returned by this function, he'll be more likely to thing about possible breakages down the line.

source/Initialization/SystemInitializerCommon.cpp

82 ↗

(On Diff #177598)

Ok, I see what you mean. That should still be possible to achieve even with stricter type checking. You'd just need to have a separate type for the "info", which can be shared by the replay and capture classes.

87 ↗

(On Diff #177598)

Ok, makes sense.

[JDevlieghere]

Feedback + Rebase on D56814

[labath]

include/lldb/Utility/Reproducer.h

91–92 ↗

(On Diff #182158)

Are you sure this can be in a header? I would expect this to give you multiply-defined symbols errors.

[JDevlieghere]

include/lldb/Utility/Reproducer.h

91–92 ↗

(On Diff #182158)

You are correct, should be in the implementation.

[JDevlieghere]

• Fix issue in lit test.
• Add unit test for the file collector.

[davide]

Some comments. Thanks!

include/lldb/Utility/FileCollector.h

5–7 ↗

(On Diff #183631)

You might want to update the license.

50–54 ↗

(On Diff #183631)

Can we add comments to these member variables? Some are not entirely obvious to me, e.g. what's m_root?
What's m_mutex is supposed to protect . I think we should document these invariants.

source/Utility/FileCollector.cpp

27–33 ↗

(On Diff #183631)

should this be a function in FS to check the case-sensitiveness?

84–87 ↗

(On Diff #183631)

Ditto (I thought there was one to canonicalize the source path?)

[JDevlieghere]

Code review feedback Davide.

source/Utility/FileCollector.cpp

27–33 ↗

(On Diff #183631)

As Host depends on Utility, we cannot depend on the filesystem class here.

84–87 ↗

(On Diff #183631)

See previous answer.

[davide]

I don't have other comments, this is good to go for me. Pavel?

[labath]

I don't have any major issues with this patch. The thing I'd like to see though is one test which uses a path with .. components and symlinks. If nothing else, it would serve as a good documentation for how these are supposed to be handled.

include/lldb/Utility/Reproducer.h

91–92 ↗

(On Diff #182158)

It looks like these are still in the header. :)

source/Utility/FileCollector.cpp

27–33 ↗

(On Diff #183631)

FWIW, I think it makes sense for FileCollector to *not* use the FileSystem class, as FileCollector is pretty much an implementation detail of the FileSystem.

unittests/Utility/FileCollectorTest.cpp

31–67 ↗

(On Diff #183641)

I have some doubts about whether this will correctly run on windows (mixing of posix and native paths sounds worrying). It might be better to just have this test always work with host paths (as these class should always deal with files on the host system anyway).

71–72 ↗

(On Diff #183641)

For testing, you can theoretically pull in other modules if they are needed (but I don't think that's the case here).

78–80 ↗

(On Diff #183641)

If you made these into ScopedFile classes like the VFS tests do then besides making this shorter, you'd also make this test self-cleaning in most of the cases (I fear this can start choking the buildbots after a couple of thousand runs...)

[JDevlieghere]

• Feedback pavel

[labath]

Looks good, thanks for bearing with me.

I don't want to hold this up further, but ideally, I'd change that last test to use positive assertions instead of negative ones. E.g., right now this doesn't really fulfill my goal of documenting how symlinks are handled. Something like EXPECT_THAT(mapping, testing::Contains(YAMLVFSEntry(file_root + "/bar", reproducer_root + "/foo"))); would (as well as giving better error messages).

PS: I believe the symlink test will have to be #ifndef WINDOWS.

[stella.stamenova]

This change broke one of the tests on Windows:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/1099

lldb/trunk/include/lldb/Host/FileSystem.h

 //===-- FileSystem.h --------------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_Host_FileSystem_h
 #define liblldb_Host_FileSystem_h
 
 #include "lldb/Host/File.h"
 #include "lldb/Utility/DataBufferLLVM.h"
+#include "lldb/Utility/FileCollector.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
 #include "lldb/lldb-types.h"
 
 #include <stdint.h>
 #include <stdio.h>
 #include <sys/stat.h>
 
 namespace lldb_private {
 class FileSystem {
 public:
   static const char *DEV_NULL;
   static const char *PATH_CONVERSION_ERROR;
 
-  FileSystem() : m_fs(llvm::vfs::getRealFileSystem()) {}
-  FileSystem(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs) : m_fs(fs) {}
+  FileSystem()
+      : m_fs(llvm::vfs::getRealFileSystem()), m_collector(nullptr),
+        m_mapped(false) {}
+  FileSystem(FileCollector &collector)
+      : m_fs(llvm::vfs::getRealFileSystem()), m_collector(&collector),
+        m_mapped(false) {}
+  FileSystem(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
+             bool mapped = false)
+      : m_fs(fs), m_mapped(mapped) {}
 
   FileSystem(const FileSystem &fs) = delete;
   FileSystem &operator=(const FileSystem &fs) = delete;
 
   static FileSystem &Instance();
 
   static void Initialize();
+  static void Initialize(FileCollector &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 EnumerateDirectory(llvm::Twine path, bool find_directories,
                           bool find_files, bool find_other,
                           EnumerateDirectoryCallbackType callback,
                           void *callback_baton);
 
   std::error_code GetRealPath(const llvm::Twine &path,
                               llvm::SmallVectorImpl<char> &output) const;
 
+  llvm::ErrorOr<std::string> GetExternalPath(const llvm::Twine &path);
+  llvm::ErrorOr<std::string> GetExternalPath(const FileSpec &file_spec);
+
 private:
   static llvm::Optional<FileSystem> &InstanceImpl();
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
+  FileCollector *m_collector;
+  bool m_mapped;
 };
 } // namespace lldb_private
 
 #endif

lldb/trunk/include/lldb/Utility/FileCollector.h

+//===-- FileCollector.h -----------------------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UTILITY_FILE_COLLECTOR_H
+#define LLDB_UTILITY_FILE_COLLECTOR_H
+
+#include "lldb/Utility/FileSpec.h"
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include <mutex>
+
+namespace lldb_private {
+
+/// Collects files into a directory and generates a mapping that can be used by
+/// the VFS.
+class FileCollector {
+public:
+  FileCollector(const FileSpec &root);
+
+  void AddFile(const llvm::Twine &file);
+  void AddFile(const FileSpec &file) { return AddFile(file.GetPath()); }
+
+  /// Write the yaml mapping (for the VFS) to the given file.
+  std::error_code WriteMapping(const FileSpec &mapping_file);
+
+  /// Copy the files into the root directory.
+  ///
+  /// When stop_on_error is true (the default) we abort as soon as one file
+  /// cannot be copied. This is relatively common, for example when a file was
+  /// removed after it was added to the mapping.
+  std::error_code CopyFiles(bool stop_on_error = true);
+
+protected:
+  void AddFileImpl(llvm::StringRef src_path);
+
+  bool MarkAsSeen(llvm::StringRef path) { return m_seen.insert(path).second; }
+
+  bool GetRealPath(llvm::StringRef src_path,
+                   llvm::SmallVectorImpl<char> &result);
+
+  void AddFileToMapping(llvm::StringRef virtual_path,
+                        llvm::StringRef real_path) {
+    m_vfs_writer.addFileMapping(virtual_path, real_path);
+  }
+
+  /// Synchronizes adding files.
+  std::mutex m_mutex;
+
+  /// The root directory where files are copied.
+  FileSpec m_root;
+
+  /// Tracks already seen files so they can be skipped.
+  llvm::StringSet<> m_seen;
+
+  /// The yaml mapping writer.
+  llvm::vfs::YAMLVFSWriter m_vfs_writer;
+
+  /// Caches real_path calls when resolving symlinks.
+  llvm::StringMap<std::string> m_symlink_map;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_UTILITY_FILE_COLLECTOR_H

lldb/trunk/include/lldb/Utility/Reproducer.h

 //===-- Reproducer.h --------------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_UTILITY_REPRODUCER_H
 #define LLDB_UTILITY_REPRODUCER_H
 
+#include "lldb/Utility/FileCollector.h"
 #include "lldb/Utility/FileSpec.h"
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/YAMLTraits.h"
 
 #include <mutex>
 #include <string>
public:
 
   llvm::StringRef GetName() const override { return ThisProviderT::info::name; }
   llvm::StringRef GetFile() const override { return ThisProviderT::info::file; }
 
 protected:
   using ProviderBase::ProviderBase; // Inherit constructor.
 };
 
+struct FileInfo {
+  static const char *name;
+  static const char *file;
+};
+
+class FileProvider : public Provider<FileProvider> {
+public:
+  typedef FileInfo info;
+
+  FileProvider(const FileSpec &directory)
+      : Provider(directory),
+        m_collector(directory.CopyByAppendingPathComponent("root")) {}
+
+  FileCollector &GetFileCollector() { return m_collector; }
+
+  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);
+  }
+
+  static char ID;
+
+private:
+  FileCollector m_collector;
+};
+
 /// 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:
   Generator(const FileSpec &root);
   ~Generator();
 

lldb/trunk/lit/Reproducer/Inputs/FileCapture.in

+run
+reproducer status
+reproducer generate

lldb/trunk/lit/Reproducer/Inputs/FileReplay.in

+reproducer status
+run

lldb/trunk/lit/Reproducer/TestFileRepro.test

+# REQUIRES: system-darwin
+
+# This tests the replaying of GDB remote packets.
+#
+# We issue the same commands and ensure the output is identical to the original
+# process. To ensure we're not actually running the original binary we check
+# that the string "testing" is not printed.
+
+# RUN: %clang %S/Inputs/simple.c -g -o %t.out
+# RUN: %lldb -x -b -s %S/Inputs/FileCapture.in --capture %t.repro -- %t.out | FileCheck %s --check-prefix CHECK --check-prefix CAPTURE
+# RUN: rm %t.out
+# RUN: %lldb -x -b -s %S/Inputs/FileReplay.in --replay %t.repro -- %t.out | FileCheck %s --check-prefix CHECK --check-prefix REPLAY
+
+# CAPTURE: testing
+# REPLAY-NOT: testing
+
+# CHECK: Process {{.*}} exited
+
+# CAPTURE: Reproducer is in capture mode.
+# CAPTURE: Reproducer written

lldb/trunk/lit/Reproducer/TestGDBRemoteRepro.test

 # UNSUPPORTED: system-windows, system-freebsd
 
 # This tests the replaying of GDB remote packets.
 #
 # We issue the same commands and ensure the output is identical to the original
 # process. To ensure we're not actually running the original binary we check
 # that the string "testing" is not printed.
 
 # RUN: %clang %S/Inputs/simple.c -g -o %t.out
-# RUN: %lldb -x -b -s %S/Inputs/GDBRemoteCapture.in --capture %T/reproducer -- %t.out | FileCheck %s --check-prefix CHECK --check-prefix CAPTURE
-# RUN: %lldb -x -b -s %S/Inputs/GDBRemoteReplay.in --replay %T/reproducer -- %t.out | FileCheck %s --check-prefix CHECK --check-prefix REPLAY
+# RUN: %lldb -x -b -s %S/Inputs/GDBRemoteCapture.in --capture %t.repro -- %t.out | FileCheck %s --check-prefix CHECK --check-prefix CAPTURE
+# RUN: %lldb -x -b -s %S/Inputs/GDBRemoteReplay.in --replay %t.repro -- %t.out | FileCheck %s --check-prefix CHECK --check-prefix REPLAY
 
 # CHECK: Breakpoint 1
 # CHECK: Process {{.*}} stopped
 # CHECK: Process {{.*}} launched
 # CHECK: thread {{.*}} stop reason = breakpoint
 # CHECK: frame {{.*}} simple.c
 
 # CAPTURE: testing
 # REPLAY-NOT: testing
 
 # CHECK: Process {{.*}} resuming
 # CHECK: Process {{.*}} exited
 
 # CAPTURE: Reproducer is in capture mode.
 # CAPTURE: Reproducer written

lldb/trunk/source/Host/common/FileSystem.cpp

 //===-- FileSystem.cpp ------------------------------------------*- C++ -*-===//
 //
 // 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/Host/FileSystem.h"
 
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/TildeExpressionResolver.h"
 
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/Errno.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Threading.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>

 
 FileSystem &FileSystem::Instance() { return *InstanceImpl(); }
 
 void FileSystem::Initialize() {
   lldbassert(!InstanceImpl() && "Already initialized.");
   InstanceImpl().emplace();
 }
 
+void FileSystem::Initialize(FileCollector &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 =
+      llvm::vfs::getRealFileSystem()->getBufferForFile(mapping.GetPath());
+
+  if (!buffer)
+    return llvm::errorCodeToError(buffer.getError());
+
+  InstanceImpl().emplace(
+      llvm::vfs::getVFSFromYAML(std::move(buffer.get()), nullptr, ""), true);
+
+  return llvm::Error::success();
+}
+
 void FileSystem::Initialize(IntrusiveRefCntPtr<vfs::FileSystem> fs) {
   lldbassert(!InstanceImpl() && "Already initialized.");
   InstanceImpl().emplace(fs);
 }
 
 void FileSystem::Terminate() {
   lldbassert(InstanceImpl() && "Already terminated.");
   InstanceImpl().reset();
void FileSystem::Resolve(FileSpec &file_spec) {
   // Update the FileSpec with the resolved path.
   file_spec.SetPath(path);
   file_spec.SetIsResolved(true);
 }
 
 std::shared_ptr<DataBufferLLVM>
 FileSystem::CreateDataBuffer(const llvm::Twine &path, uint64_t size,
                              uint64_t offset) {
+  if (m_collector)
+    m_collector->AddFile(path);
+
   const bool is_volatile = !IsLocal(path);
+  const ErrorOr<std::string> external_path = GetExternalPath(path);
+
+  if (!external_path)
+    return nullptr;
 
   std::unique_ptr<llvm::WritableMemoryBuffer> buffer;
   if (size == 0) {
     auto buffer_or_error =
-        llvm::WritableMemoryBuffer::getFile(path, -1, is_volatile);
+        llvm::WritableMemoryBuffer::getFile(*external_path, -1, is_volatile);
     if (!buffer_or_error)
       return nullptr;
     buffer = std::move(*buffer_or_error);
   } else {
     auto buffer_or_error = llvm::WritableMemoryBuffer::getFileSlice(
-        path, size, offset, is_volatile);
+        *external_path, size, offset, is_volatile);
     if (!buffer_or_error)
       return nullptr;
     buffer = std::move(*buffer_or_error);
   }
   return std::shared_ptr<DataBufferLLVM>(new DataBufferLLVM(std::move(buffer)));
 }
 
 std::shared_ptr<DataBufferLLVM>
if (permissions & lldb::eFilePermissionsWorldWrite)
     mode |= S_IWOTH;
   if (permissions & lldb::eFilePermissionsWorldExecute)
     mode |= S_IXOTH;
   return mode;
 }
 
 Status FileSystem::Open(File &File, const FileSpec &file_spec, uint32_t options,
                         uint32_t permissions) {
+  if (m_collector)
+    m_collector->AddFile(file_spec);
+
   if (File.IsValid())
     File.Close();
 
   const int open_flags = GetOpenFlags(options);
   const mode_t open_mode =
       (open_flags & O_CREAT) ? GetOpenMode(permissions) : 0;
-  const std::string path = file_spec.GetPath();
+
+  auto path = GetExternalPath(file_spec);
+  if (!path)
+    return Status(path.getError());
 
   int descriptor = llvm::sys::RetryAfterSignal(
-      -1, OpenWithFS, *this, path.c_str(), open_flags, open_mode);
+      -1, OpenWithFS, *this, path->c_str(), open_flags, open_mode);
 
   Status error;
   if (!File::DescriptorIsValid(descriptor)) {
     File.SetDescriptor(descriptor, false);
     error.SetErrorToErrno();
   } else {
     File.SetDescriptor(descriptor, true);
     File.SetOptions(options);
   }
   return error;
 }
+
+ErrorOr<std::string> FileSystem::GetExternalPath(const llvm::Twine &path) {
+  if (!m_mapped)
+    return path.str();
+
+  // If VFS mapped we know the underlying FS is a RedirectingFileSystem.
+  ErrorOr<vfs::RedirectingFileSystem::Entry *> E =
+      static_cast<vfs::RedirectingFileSystem &>(*m_fs).lookupPath(path);
+  if (!E) {
+    if (E.getError() == llvm::errc::no_such_file_or_directory) {
+      return path.str();
+    }
+    return E.getError();
+  }
+
+  auto *F = dyn_cast<vfs::RedirectingFileSystem::RedirectingFileEntry>(*E);
+  if (!F)
+    return make_error_code(llvm::errc::not_supported);
+
+  return F->getExternalContentsPath().str();
+}
+
+ErrorOr<std::string> FileSystem::GetExternalPath(const FileSpec &file_spec) {
+  return GetExternalPath(file_spec.GetPath());
+}

lldb/trunk/source/Host/macosx/objcxx/Host.mm

#else
     error.SetErrorString("launching a process in a new terminal is not "
                          "supported on iOS devices");
     return error;
 #endif
   }
 
   lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
 
-  if (ShouldLaunchUsingXPC(launch_info)) {
-    error = LaunchProcessXPC(exe_spec.GetPath().c_str(), launch_info, pid);
-  } else {
-    error =
-        LaunchProcessPosixSpawn(exe_spec.GetPath().c_str(), launch_info, pid);
-  }
+  // From now on we'll deal with the external (devirtualized) path.
+  auto exe_path = fs.GetExternalPath(exe_spec);
+  if (!exe_path)
+    return Status(exe_path.getError());
+
+  if (ShouldLaunchUsingXPC(launch_info))
+    error = LaunchProcessXPC(exe_path->c_str(), launch_info, pid);
+  else
+    error = LaunchProcessPosixSpawn(exe_path->c_str(), launch_info, pid);
 
   if (pid != LLDB_INVALID_PROCESS_ID) {
     // If all went well, then set the process ID into the launch info
     launch_info.SetProcessID(pid);
 
     // Make sure we reap any processes we spawn or we will have zombies.
     bool monitoring = launch_info.MonitorProcess();
     UNUSED_IF_ASSERT_DISABLED(monitoring);

lldb/trunk/source/Initialization/SystemInitializerCommon.cpp

if (disable_crash_dialog_var &&
     _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG);
     _CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG);
     _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR);
     _CrtSetReportFile(_CRT_WARN, _CRTDBG_FILE_STDERR);
     _CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDERR);
   }
 #endif
 
+  // Initialize the reproducer.
   ReproducerMode mode = ReproducerMode::Off;
   if (options.reproducer_capture)
     mode = ReproducerMode::Capture;
   if (options.reproducer_replay)
     mode = ReproducerMode::Replay;
 
   if (auto e = Reproducer::Initialize(mode, FileSpec(options.reproducer_path)))
     return e;
 
+  // Initialize the file system.
+  auto &r = repro::Reproducer::Instance();
+  if (repro::Loader *loader = r.GetLoader()) {
+    FileSpec vfs_mapping = loader->GetFile<FileInfo>();
+    if (vfs_mapping) {
+      if (llvm::Error e = FileSystem::Initialize(vfs_mapping))
+        return e;
+    } else {
+      FileSystem::Initialize();
+    }
+  } else if (repro::Generator *g = r.GetGenerator()) {
+    repro::FileProvider &fp = g->GetOrCreate<repro::FileProvider>();
+    FileSystem::Initialize(fp.GetFileCollector());
+  } else {
     FileSystem::Initialize();
+  }
+
   Log::Initialize();
   HostInfo::Initialize();
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
 
   process_gdb_remote::ProcessGDBRemoteLog::Initialize();
 
   // Initialize plug-ins

lldb/trunk/source/Utility/CMakeLists.txt

add_lldb_library(lldbUtility
   Connection.cpp
   ConstString.cpp
   CompletionRequest.cpp
   DataBufferHeap.cpp
   DataBufferLLVM.cpp
   DataEncoder.cpp
   DataExtractor.cpp
   Environment.cpp
+  FileCollector.cpp
   Event.cpp
   FileSpec.cpp
   IOObject.cpp
   JSON.cpp
   LLDBAssert.cpp
   Listener.cpp
   Log.cpp
   Logging.cpp

lldb/trunk/source/Utility/FileCollector.cpp

+//===-- FileCollector.cpp ---------------------------------------*- C++ -*-===//
+//
+// 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/FileCollector.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+
+using namespace lldb_private;
+using namespace llvm;
+
+static bool IsCaseSensitivePath(StringRef path) {
+  SmallString<256> tmp_dest = path, upper_dest, real_dest;
+
+  // Remove component traversals, links, etc.
+  if (!sys::fs::real_path(path, tmp_dest))
+    return true; // Current default value in vfs.yaml
+  path = tmp_dest;
+
+  // Change path to all upper case and ask for its real path, if the latter
+  // exists and is equal to path, it's not case sensitive. Default to case
+  // sensitive in the absence of real_path, since this is the YAMLVFSWriter
+  // default.
+  upper_dest = path.upper();
+  if (sys::fs::real_path(upper_dest, real_dest) && path.equals(real_dest))
+    return false;
+  return true;
+}
+
+FileCollector::FileCollector(const FileSpec &root) : m_root(root) {
+  sys::fs::create_directories(m_root.GetPath(), true);
+}
+
+bool FileCollector::GetRealPath(StringRef src_path,
+                                SmallVectorImpl<char> &result) {
+  SmallString<256> real_path;
+  StringRef FileName = sys::path::filename(src_path);
+  std::string directory = sys::path::parent_path(src_path).str();
+  auto dir_with_symlink = m_symlink_map.find(directory);
+
+  // Use real_path to fix any symbolic link component present in a path.
+  // Computing the real path is expensive, cache the search through the
+  // parent path directory.
+  if (dir_with_symlink == m_symlink_map.end()) {
+    auto ec = sys::fs::real_path(directory, real_path);
+    if (ec)
+      return false;
+    m_symlink_map[directory] = real_path.str();
+  } else {
+    real_path = dir_with_symlink->second;
+  }
+
+  sys::path::append(real_path, FileName);
+  result.swap(real_path);
+  return true;
+}
+
+void FileCollector::AddFile(const Twine &file) {
+  std::lock_guard<std::mutex> lock(m_mutex);
+  std::string file_str = file.str();
+  if (MarkAsSeen(file_str))
+    AddFileImpl(file_str);
+}
+
+void FileCollector::AddFileImpl(StringRef src_path) {
+  std::string root = m_root.GetPath();
+
+  // We need an absolute src path to append to the root.
+  SmallString<256> absolute_src = src_path;
+  sys::fs::make_absolute(absolute_src);
+
+  // Canonicalize src to a native path to avoid mixed separator styles.
+  sys::path::native(absolute_src);
+
+  // Remove redundant leading "./" pieces and consecutive separators.
+  absolute_src = sys::path::remove_leading_dotslash(absolute_src);
+
+  // Canonicalize the source path by removing "..", "." components.
+  SmallString<256> virtual_path = absolute_src;
+  sys::path::remove_dots(virtual_path, /*remove_dot_dot=*/true);
+
+  // If a ".." component is present after a symlink component, remove_dots may
+  // lead to the wrong real destination path. Let the source be canonicalized
+  // like that but make sure we always use the real path for the destination.
+  SmallString<256> copy_from;
+  if (!GetRealPath(absolute_src, copy_from))
+    copy_from = virtual_path;
+
+  SmallString<256> dst_path = StringRef(root);
+  sys::path::append(dst_path, sys::path::relative_path(copy_from));
+
+  // Always map a canonical src path to its real path into the YAML, by doing
+  // this we map different virtual src paths to the same entry in the VFS
+  // overlay, which is a way to emulate symlink inside the VFS; this is also
+  // needed for correctness, not doing that can lead to module redefinition
+  // errors.
+  AddFileToMapping(virtual_path, dst_path);
+}
+
+std::error_code FileCollector::CopyFiles(bool stop_on_error) {
+  for (auto &entry : m_vfs_writer.getMappings()) {
+    // Create directory tree.
+    if (std::error_code ec =
+            sys::fs::create_directories(sys::path::parent_path(entry.RPath),
+                                        /*IgnoreExisting=*/true)) {
+      if (stop_on_error)
+        return ec;
+    }
+
+    // Copy file over.
+    if (std::error_code ec = sys::fs::copy_file(entry.VPath, entry.RPath)) {
+      if (stop_on_error)
+        return ec;
+    }
+
+    // Copy over permissions.
+    if (auto perms = sys::fs::getPermissions(entry.VPath)) {
+      if (std::error_code ec = sys::fs::setPermissions(entry.RPath, *perms)) {
+        if (stop_on_error)
+          return ec;
+      }
+    }
+  }
+  return {};
+}
+
+std::error_code FileCollector::WriteMapping(const FileSpec &mapping_file) {
+  std::lock_guard<std::mutex> lock(m_mutex);
+
+  const std::string root = m_root.GetPath();
+  m_vfs_writer.setCaseSensitivity(IsCaseSensitivePath(root));
+  m_vfs_writer.setUseExternalNames(false);
+
+  std::error_code ec;
+  raw_fd_ostream os(mapping_file.GetPath(), ec, sys::fs::F_Text);
+  if (ec)
+    return ec;
+
+  m_vfs_writer.write(os);
+
+  return {};
+}

lldb/trunk/source/Utility/Reproducer.cpp

 bool Loader::HasFile(StringRef file) {
   assert(m_loaded);
   auto it = std::lower_bound(m_files.begin(), m_files.end(), file.str());
   return (it != m_files.end()) && (*it == file);
 }
 
 void ProviderBase::anchor() {}
 char ProviderBase::ID = 0;
+char FileProvider::ID = 0;
+const char *FileInfo::name = "files";
+const char *FileInfo::file = "files.yaml";

lldb/trunk/unittests/Utility/CMakeLists.txt

 add_lldb_unittest(UtilityTests
   AnsiTerminalTest.cpp
   ArgsTest.cpp
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp
   BroadcasterTest.cpp
   CleanUpTest.cpp
   ConstStringTest.cpp
   CompletionRequestTest.cpp
   DataExtractorTest.cpp
   EnvironmentTest.cpp
   EventTest.cpp
+  FileCollectorTest.cpp
   FileSpecTest.cpp
   FlagsTest.cpp
   JSONTest.cpp
   ListenerTest.cpp
   LogTest.cpp
   NameMatchesTest.cpp
   PredicateTest.cpp
   RegisterValueTest.cpp

lldb/trunk/unittests/Utility/FileCollectorTest.cpp

+//===-- FileCollectorTest.cpp -----------------------------------*- C++ -*-===//
+//
+// 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 "lldb/Utility/FileCollector.h"
+#include "lldb/Utility/FileSpec.h"
+
+#include "llvm/Support/FileSystem.h"
+
+using namespace llvm;
+using namespace lldb_private;
+
+namespace llvm {
+namespace vfs {
+inline bool operator==(const llvm::vfs::YAMLVFSEntry &LHS,
+                       const llvm::vfs::YAMLVFSEntry &RHS) {
+  return LHS.VPath == RHS.VPath && LHS.RPath == RHS.RPath;
+}
+} // namespace vfs
+} // namespace llvm
+
+namespace {
+class TestingFileCollector : public FileCollector {
+public:
+  using FileCollector::FileCollector;
+  using FileCollector::m_root;
+  using FileCollector::m_seen;
+  using FileCollector::m_symlink_map;
+  using FileCollector::m_vfs_writer;
+
+  bool HasSeen(FileSpec fs) {
+    return m_seen.find(fs.GetPath()) != m_seen.end();
+  }
+};
+
+struct ScopedDir {
+  SmallString<128> Path;
+  ScopedDir(const Twine &Name, bool Unique = false) {
+    std::error_code EC;
+    if (Unique) {
+      EC = llvm::sys::fs::createUniqueDirectory(Name, Path);
+    } else {
+      Path = Name.str();
+      EC = llvm::sys::fs::create_directory(Twine(Path));
+    }
+    if (EC)
+      Path = "";
+    EXPECT_FALSE(EC);
+    // Ensure the path is the real path so tests can use it to compare against
+    // realpath output.
+    SmallString<128> RealPath;
+    if (!llvm::sys::fs::real_path(Path, RealPath))
+      Path.swap(RealPath);
+  }
+  ~ScopedDir() {
+    if (Path != "") {
+      EXPECT_FALSE(llvm::sys::fs::remove_directories(Path.str()));
+    }
+  }
+  operator StringRef() { return Path.str(); }
+};
+
+struct ScopedLink {
+  SmallString<128> Path;
+  ScopedLink(const Twine &To, const Twine &From) {
+    Path = From.str();
+    std::error_code EC = sys::fs::create_link(To, From);
+    if (EC)
+      Path = "";
+    EXPECT_FALSE(EC);
+  }
+  ~ScopedLink() {
+    if (Path != "") {
+      EXPECT_FALSE(llvm::sys::fs::remove(Path.str()));
+    }
+  }
+  operator StringRef() { return Path.str(); }
+};
+
+struct ScopedFile {
+  SmallString<128> Path;
+  ScopedFile(const Twine &Name) {
+    std::error_code EC;
+    EC = llvm::sys::fs::createUniqueFile(Name, Path);
+    if (EC)
+      Path = "";
+    EXPECT_FALSE(EC);
+  }
+  ~ScopedFile() {
+    if (Path != "") {
+      EXPECT_FALSE(llvm::sys::fs::remove(Path.str()));
+    }
+  }
+  operator StringRef() { return Path.str(); }
+};
+} // end anonymous namespace
+
+TEST(FileCollectorTest, AddFile) {
+  ScopedDir root("add_file_root", true);
+  FileSpec root_fs(root.Path);
+  TestingFileCollector file_collector(root_fs);
+
+  file_collector.AddFile(FileSpec("/path/to/a"));
+  file_collector.AddFile(FileSpec("/path/to/b"));
+  file_collector.AddFile(FileSpec("/path/to/c"));
+
+  // Make sure the root is correct.
+  EXPECT_EQ(file_collector.m_root, root_fs);
+
+  // Make sure we've seen all the added files.
+  EXPECT_TRUE(file_collector.HasSeen(FileSpec("/path/to/a")));
+  EXPECT_TRUE(file_collector.HasSeen(FileSpec("/path/to/b")));
+  EXPECT_TRUE(file_collector.HasSeen(FileSpec("/path/to/c")));
+
+  // Make sure we've only seen the added files.
+  EXPECT_FALSE(file_collector.HasSeen(FileSpec("/path/to/d")));
+}
+
+TEST(FileCollectorTest, CopyFiles) {
+  ScopedDir file_root("file_root", true);
+  ScopedFile a(file_root + "/aaa");
+  ScopedFile b(file_root + "/bbb");
+  ScopedFile c(file_root + "/ccc");
+
+  // Create file collector and add files.
+  ScopedDir root("copy_files_root", true);
+  FileSpec root_fs(root.Path);
+  TestingFileCollector file_collector(root_fs);
+  file_collector.AddFile(a.Path);
+  file_collector.AddFile(b.Path);
+  file_collector.AddFile(c.Path);
+
+  // Make sure we can copy the files.
+  std::error_code ec = file_collector.CopyFiles(true);
+  EXPECT_FALSE(ec);
+
+  // Now add a bogus file and make sure we error out.
+  file_collector.AddFile("/some/bogus/file");
+  ec = file_collector.CopyFiles(true);
+  EXPECT_TRUE(ec);
+
+  // However, if stop_on_error is true the copy should still succeed.
+  ec = file_collector.CopyFiles(false);
+  EXPECT_FALSE(ec);
+}
+
+#ifndef WINDOWS
+TEST(FileCollectorTest, Symlinks) {
+  // Root where the original files live.
+  ScopedDir file_root("file_root", true);
+
+  // Create some files in the file root.
+  ScopedFile a(file_root + "/aaa");
+  ScopedFile b(file_root + "/bbb");
+  ScopedFile c(file_root + "/ccc");
+
+  // Create a directory foo with file ddd.
+  ScopedDir foo(file_root + "/foo");
+  ScopedFile d(foo + "/ddd");
+
+  // Create a file eee in the foo's parent directory.
+  ScopedFile e(foo + "/../eee");
+
+  // Create a symlink bar pointing to foo.
+  ScopedLink symlink(file_root + "/foo", file_root + "/bar");
+
+  // Root where files are copied to.
+  ScopedDir reproducer_root("reproducer_root", true);
+  FileSpec root_fs(reproducer_root.Path);
+  TestingFileCollector file_collector(root_fs);
+
+  // Add all the files to the collector.
+  file_collector.AddFile(a.Path);
+  file_collector.AddFile(b.Path);
+  file_collector.AddFile(c.Path);
+  file_collector.AddFile(d.Path);
+  file_collector.AddFile(e.Path);
+  file_collector.AddFile(file_root + "/bar/ddd");
+
+  auto mapping = file_collector.m_vfs_writer.getMappings();
+
+  {
+    // Make sure the common case works.
+    std::string vpath = (file_root + "/aaa").str();
+    std::string rpath = (reproducer_root.Path + file_root.Path + "/aaa").str();
+    printf("%s -> %s\n", vpath.c_str(), rpath.c_str());
+    EXPECT_THAT(mapping, testing::Contains(vfs::YAMLVFSEntry(vpath, rpath)));
+  }
+
+  {
+    // Make sure the virtual path points to the real source path.
+    std::string vpath = (file_root + "/bar/ddd").str();
+    std::string rpath =
+        (reproducer_root.Path + file_root.Path + "/foo/ddd").str();
+    printf("%s -> %s\n", vpath.c_str(), rpath.c_str());
+    EXPECT_THAT(mapping, testing::Contains(vfs::YAMLVFSEntry(vpath, rpath)));
+  }
+
+  {
+    // Make sure that .. is removed from the source path.
+    std::string vpath = (file_root + "/eee").str();
+    std::string rpath = (reproducer_root.Path + file_root.Path + "/eee").str();
+    printf("%s -> %s\n", vpath.c_str(), rpath.c_str());
+    EXPECT_THAT(mapping, testing::Contains(vfs::YAMLVFSEntry(vpath, rpath)));
+  }
+}
+#endif