[VFS] Add a new getVFSFromYAMLs API

[bnbarham]

Summary

Implements redirecting-with using an OverlayFileSystem rather than
nesting RedirectingFileSystem -

• "fallthrough": adds the RedirectingFileSystem to OverlayFileSystem
• "fallback": adds the RedirectingFileSystem *before* the previously added one. This is really only useful when its the initial overlay to place it before the RealFileSystem
• "redirect-only": removes any previously added filesystems. Mostly intended to remove the RealFileSystem, but works in the general case as well.

Updates getVFSFromYAML to use this API, as well as any existing uses
of getVFSFromYAML since it's simpler anyway (ie. no need to read the
underlying buffers first). This does come with a slight change in
semantics though - it is no longer possible to "chain" VFS mappings.
Given an overlay with a mapping of A -> B and another with B -> C, A can
no longer map to C. An A ->C mapping must be specified as its own
mapping if this is desired.

The intention of this change is to allow for simplifying
RedirectingFileSystem by removing all the redirecting-with handling
from it. This will allow reverting it back to its original
implementation where it simply performed the mapping and failed if that
mapping did not exist in ExternalFS. That will be completed in a later
commit to allow for an easier migration path if any downstream user
requires the ability to chain mappings.

Depends on D121423

[bnbarham]

Fixed up the getVFSFromYAML implementation to actually pass the buffer. Note that I'm happy to remove this, I only left it around to make it easier on downstream users.

[bnbarham]

Add a check for CWD when using redirect-only

[bnbarham]

Update to single review dependency

[bnbarham]

Re-order to be before D121424 and merge with D121426

[bnbarham]

There's two failing tests with this change:

• VFSFromYAMLTest.ReturnsExternalPathVFSHit
• VFSFromYAMLTest.ReturnsInternalPathVFSHit

Apparently we allow relative paths in external-contents *without* specifying overlay-relative: true. In this case the relative paths are resolved based on the CWD at the time of the operation. Do we really want that? I believe this wouldn't have worked prior to https://reviews.llvm.org/D109128 as there was no canonicalising before then as well. @keith was that change intentional? If we want it then it will *require* setting CWD on each FS in OverlayFS, which I was really hoping to avoid.

To give the concrete case...

Prior to all these changes:

RedirectingFileSystem
  /a/foo -> bar
ExternalFS
  RealFileSystem
    /a/bar
    /b/bar

cd /a
cat foo # /a/bar

cd /b # fail since it doesn't exist in RedirectingFS

After:

OverlayFileSystem
  RedirectingFileSystem
    /a/foo -> bar

  RealFileSystem
    /a/bar
    /b/bar

cd /a
cat foo # fails since OverlayFS::setCWD doesn't set CWD on the underlying FS

cd /b # succeed
cat foo # fail since there's no /b/foo

In D121424 it will fail for a slightly different reason, but still fail - there's no longer any canonicalise after mapping the path in RedirectingFS (I assumed this was unintentional and missed the test case).

If we want to allow this then I think we'll need to go with the previous plan for OverlayFS, ie. keep track of filesystems that setCWD failed on and don't call them at all until they succeed again. That then doesn't allow certain cases that the current method allows.

[bnbarham]

In D121425#3384188, @bnbarham wrote:

There's two failing tests with this change:

• VFSFromYAMLTest.ReturnsExternalPathVFSHit
• VFSFromYAMLTest.ReturnsInternalPathVFSHit

Apparently we allow relative paths in external-contents *without* specifying overlay-relative: true. In this case the relative paths are resolved based on the CWD at the time of the operation. Do we really want that? I believe this wouldn't have worked prior to https://reviews.llvm.org/D109128 as there was no canonicalising before then as well. @keith was that change intentional? If we want it then it will *require* setting CWD on each FS in OverlayFS, which I was really hoping to avoid.

I spoke to Keith offline. This has always worked - it previously worked by RedirectingFileSystem setting CWD on ExternalFS when setCWD was called on it. It's also important to keep supporting as it's used in Bazel (https://github.com/bazelbuild/rules_swift/blob/c1d7d1df6969c2675c7826ecf1202d78016b1753/swift/internal/vfsoverlay.bzl#L41-L55).

I'm hoping I can fix this by resolving the paths when the overlay is created. I'll see if that works (it'll depend on when -working-directory is actually used by the frontend).

[dexonsmith]

In D121425#3384188, @bnbarham wrote:

There's two failing tests with this change:

• VFSFromYAMLTest.ReturnsExternalPathVFSHit
• VFSFromYAMLTest.ReturnsInternalPathVFSHit

Apparently we allow relative paths in external-contents *without* specifying overlay-relative: true. In this case the relative paths are resolved based on the CWD at the time of the operation. Do we really want that? I believe this wouldn't have worked prior to https://reviews.llvm.org/D109128 as there was no canonicalising before then as well. @keith was that change intentional? If we want it then it will *require* setting CWD on each FS in OverlayFS, which I was really hoping to avoid.

I'm not sure how external-contents factors into this (shouldn't that just affect the observed file path, not whether the lookup succeeds?).

Regardless, here's the testcase again:

OverlayFileSystem
  RedirectingFileSystem
    /a/foo -> bar

  RealFileSystem
    /a/bar
    /b/bar

I think that should behave like:

InMemoryFileSystem
  /a/bar
  /a/foo -> bar (link, with "magic" to sometimes leak path)
  /b/bar

cd /a # succeeds
cat foo # succeeds (get content of /a/bar; report path of /a/bar or /a/foo)

cd /b # succeeds
cat foo # fails

Is the model correct?

If so, I think these are the things we need to get right (not just for that test case):

1. Return error on operations that should fail.
2. Get the right content what cat succeeds.
3. Reporting the "right" paths for files that are opened (depends on external contents, etc.).
4. Keeping the "view" / state consistent when the underlying filesystems changes state.

Does that breakdown make sense?

If so, can you clarify which part is failing? (Not sure if it's (3) or (4)... or maybe some (5).)

Or another way in: I don't fully understand why this fails:

cd /a
cat foo # fails since OverlayFS::setCWD doesn't set CWD on the underlying FS

Can you be more detailed about the overall state at the time of cat, which causes it to fail?

[dexonsmith]

In D121425#3384479, @bnbarham wrote:

In D121425#3384188, @bnbarham wrote:

There's two failing tests with this change:

• VFSFromYAMLTest.ReturnsExternalPathVFSHit
• VFSFromYAMLTest.ReturnsInternalPathVFSHit

Apparently we allow relative paths in external-contents *without* specifying overlay-relative: true. In this case the relative paths are resolved based on the CWD at the time of the operation. Do we really want that? I believe this wouldn't have worked prior to https://reviews.llvm.org/D109128 as there was no canonicalising before then as well. @keith was that change intentional? If we want it then it will *require* setting CWD on each FS in OverlayFS, which I was really hoping to avoid.

I spoke to Keith offline. This has always worked - it previously worked by RedirectingFileSystem setting CWD on ExternalFS when setCWD was called on it. It's also important to keep supporting as it's used in Bazel (https://github.com/bazelbuild/rules_swift/blob/c1d7d1df6969c2675c7826ecf1202d78016b1753/swift/internal/vfsoverlay.bzl#L41-L55).

I'm hoping I can fix this by resolving the paths when the overlay is created. I'll see if that works (it'll depend on when -working-directory is actually used by the frontend).

(Would this mean resolving everything in the .yaml file eagerly at launch? That sounds a bit scary...)

[bnbarham]

In D121425#3384492, @dexonsmith wrote:

Can you be more detailed about the overall state at the time of cat, which causes it to fail?

Sure. I think there's also some confusion with names here but I'm generally trying to use the names as they are used in the overlay YAML or the name of the members in RedirectingFS.

"external-contents" is just the path that we're mapping to, so in my example above (/a/foo -> bar), "bar" is what I was referring to by "external-contents". So let's use that again -

OverlayFileSystem
  RedirectingFileSystem
    /a/foo -> bar

  RealFileSystem
    /a/bar
    /b/bar

After D121423, cd /a will result in the following -

• OverlayFS: CWD set to /a
• RedirectingFS and RealFS: CWD unchanged, so let's assume they were / at the time of creation and that's what they are now

cat foo -

• OverlayFS canonicalises to /a/foo and passes that to openFileForRead in RedirectingFS
• RedirectingFS has a /a/foo -> bar mapping, so it now runs openFileForRead on its ExternalFS (which is the RealFS)
• RealFS is at / and there is no /bar and thus fails
• RedirectingFS also fails
• Back in OverlayFS now, and because we failed we now try RealFS with /a/foo
• RealFS has no /a/foo, it fails
• Back in OverlayFS, all FS have now failed, return a failure

How this used to work really depends on when we're talking about 😆. Prior to D121423 the FS would have looked like:

RedirectingFileSystem
  /a/foo -> bar
ExternalFS
  RealFileSystem
    /a/bar
    /b/bar

Prior to Jonas' change, setCWD on RedirectingFS would run setCWD on ExternalFS. So cd /a would set CWD to /a for both RedirectingFS and RealFileSystem. Then cat /a/foo would give bar and openFileForRead on ExternalFS (RealFS) (which has /a CWD) would open /a/bar.

After Jonas' *and* Keith's change, setCWD no longer runs on ExternalFS but instead the path was canonicalised before sending it along. So /a/foo maps to bar as before, but then it would be canonicalised using RedirectingFS CWD and thus do an open for /a/bar on ExternalFS.

In D121425#3384499, @dexonsmith wrote:

(Would this mean resolving everything in the .yaml file eagerly at launch? That sounds a bit scary...)

Yes, depending on what you mean by "resolving". All I mean is "prefix relative paths with CWD". The main issue with doing this is that it prohibits canonicalising paths to a virtual CWD, since you wouldn't be able to set CWD to a virtual path before making the overlay. Perhaps that's what you mean by "a bit scary" though.

We currently prefix relative paths with ExternalContentsPrefixDir if overlay-relative: true is set. That path is a combination of CWD + the directory to the overlay. This is to handle overlays that have absolute paths that we want to remap (specifically for the crash dump use case I believe). But if overlay-relative: false is set then we just canonicalise the path on open/etc.

I just ran a few tests with the frontend though:

• By default, paths come in relative and will become absolute from the process-wide CWD
• If -working-directory *is* set then paths come in absolute from FileManager - setCWD is never run

I'm not sure if there's a good reason it's done this way or it's just that CWD was only added to FileSystem relatively recently, but it does mean that that we're actually canonicalising the paths with the process-wide CWD regardless at the moment (at least from the frontend). Possibly unexpected since you could pass in -working-directory/foo but the relative mappings would still get canonicalised using the process CWD.

[bnbarham]

Blocked on the dependent

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
   if (Config.getNumOccurrences() > 0)
     return LoadConfig(Config, "<command-line-config>");
 
   return std::make_unique<FileOptionsProvider>(
       std::move(GlobalOptions), std::move(DefaultOptions),
       std::move(OverrideOptions), std::move(FS));
 }
 
-llvm::IntrusiveRefCntPtr<vfs::FileSystem>
-getVfsFromFile(const std::string &OverlayFile,
-               llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS) {
-  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
-      BaseFS->getBufferForFile(OverlayFile);
-  if (!Buffer) {
-    llvm::errs() << "Can't load virtual filesystem overlay file '"
-                 << OverlayFile << "': " << Buffer.getError().message()
-                 << ".\n";
-    return nullptr;
+static IntrusiveRefCntPtr<vfs::OverlayFileSystem>
+getVFSFromFile(StringRef OverlayFile) {
+  if (OverlayFile.empty())
+    return makeIntrusiveRefCnt<vfs::OverlayFileSystem>(
+        vfs::getRealFileSystem());
+
+  auto OverlayFS = vfs::getVFSFromYAMLs(OverlayFile, vfs::getRealFileSystem());
+  if (auto Err = OverlayFS.takeError()) {
+    handleAllErrors(std::move(Err), [](const FileError &FE) {
+      if (FE.convertToErrorCode() == std::errc::no_such_file_or_directory) {
+        errs() << "Can't load virtual filesystem overlay file '"
+               << FE.getFileName() << "': " << FE.message() << ".\n";
+      } else {
+        errs() << "Error: invalid virtual filesystem overlay file '"
+               << FE.getFileName() << "'.\n";
       }
-
-  IntrusiveRefCntPtr<vfs::FileSystem> FS = vfs::getVFSFromYAML(
-      std::move(Buffer.get()), /*DiagHandler*/ nullptr, OverlayFile);
-  if (!FS) {
-    llvm::errs() << "Error: invalid virtual filesystem overlay file '"
-                 << OverlayFile << "'.\n";
+    });
     return nullptr;
   }
-  return FS;
+  return std::move(*OverlayFS);
 }
 
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
 
   // Enable help for -load option, if plugins are enabled.
   if (cl::Option *LoadOpt = cl::getRegisteredOptions().lookup("load"))
     LoadOpt->addCategory(ClangTidyCategory);
 
   llvm::Expected<CommonOptionsParser> OptionsParser =
       CommonOptionsParser::create(argc, argv, ClangTidyCategory,
                                   cl::ZeroOrMore);
   if (!OptionsParser) {
     llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
     return 1;
   }
 
-  llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> BaseFS(
-      new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
-
-  if (!VfsOverlay.empty()) {
-    IntrusiveRefCntPtr<vfs::FileSystem> VfsFromFile =
-        getVfsFromFile(VfsOverlay, BaseFS);
-    if (!VfsFromFile)
-      return 1;
-    BaseFS->pushOverlay(std::move(VfsFromFile));
-  }
+  llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> BaseFS =
+      getVFSFromFile(VfsOverlay);
+  if (!BaseFS)
 
   auto OwningOptionsProvider = createOptionsProvider(BaseFS);
   auto *OptionsProvider = OwningOptionsProvider.get();
   if (!OptionsProvider)
     return 1;
 
   auto MakeAbsolute = [](const std::string &Input) -> SmallString<256> {
     if (Input.empty())

clang/lib/Frontend/CompilerInvocation.cpp

clang::createVFSFromCompilerInvocation(const CompilerInvocation &CI,
   return createVFSFromCompilerInvocation(CI, Diags,
                                          llvm::vfs::getRealFileSystem());
 }
 
 IntrusiveRefCntPtr<llvm::vfs::FileSystem>
 clang::createVFSFromCompilerInvocation(
     const CompilerInvocation &CI, DiagnosticsEngine &Diags,
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) {
-  if (CI.getHeaderSearchOpts().VFSOverlayFiles.empty())
+  ArrayRef<std::string> OptFiles = CI.getHeaderSearchOpts().VFSOverlayFiles;
+  if (OptFiles.empty())
     return BaseFS;
 
-  IntrusiveRefCntPtr<llvm::vfs::FileSystem> Result = BaseFS;
-  // earlier vfs files are on the bottom
-  for (const auto &File : CI.getHeaderSearchOpts().VFSOverlayFiles) {
-    llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
-        Result->getBufferForFile(File);
-    if (!Buffer) {
-      Diags.Report(diag::err_missing_vfs_overlay_file) << File;
-      continue;
-    }
-
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = llvm::vfs::getVFSFromYAML(
-        std::move(Buffer.get()), /*DiagHandler*/ nullptr, File,
-        /*DiagContext*/ nullptr, Result);
-    if (!FS) {
-      Diags.Report(diag::err_invalid_vfs_overlay) << File;
-      continue;
+  SmallVector<StringRef> Files(OptFiles.begin(), OptFiles.end());
+  Expected<IntrusiveRefCntPtr<llvm::vfs::FileSystem>> OverlayFS =
+      llvm::vfs::getVFSFromYAMLs(Files, BaseFS);
+  if (auto Err = OverlayFS.takeError()) {
+    llvm::handleAllErrors(std::move(Err), [&](const llvm::FileError &FE) {
+      if (FE.convertToErrorCode() == std::errc::no_such_file_or_directory) {
+        Diags.Report(diag::err_missing_vfs_overlay_file) << FE.getFileName();
+      } else {
+        Diags.Report(diag::err_invalid_vfs_overlay) << FE.getFileName();
       }
-
-    Result = FS;
+    });
+    return BaseFS;
   }
-  return Result;
+  return *OverlayFS;
 }

clang/test/VFS/Inputs/vfsroot.yaml

'roots': [
         { 'name': 'not_real.h', 'type': 'file',
           'external-contents': 'TEST_DIR/Inputs/actual_header.h'
         },
         { 'name': 'vfsoverlay.yaml', 'type': 'file',
           'external-contents': 'OUT_DIR/vfsoverlay.yaml'
         }
       ]
     },
-    { 'name': '/indirect-vfs-root-files', 'type': 'directory',
-      'contents': [
-        { 'name': 'actual_header.h', 'type': 'file',
-          'external-contents': 'TEST_DIR/Inputs/actual_header.h'
-        }
-      ]
-    },
     { 'name': 'TEST_DIR/Inputs/Broken.framework', 'type': 'directory',
       'contents': [
         { 'name': 'Headers/A.h', 'type': 'file',
           'external-contents': 'TEST_DIR/Inputs/Broken.framework/VFSHeaders/A.h'
         },
         { 'name': 'Modules/module.modulemap', 'type': 'file',
           'external-contents': 'TEST_DIR/Inputs/Broken.framework/Modules/module.modulemap'
         }

clang/test/VFS/directory.c

 // RUN: rm -rf %t
 // RUN: mkdir -p %t/Underlying
 // RUN: mkdir -p %t/Overlay
-// RUN: mkdir -p %t/Middle
 // RUN: echo '// B.h in Underlying' > %t/Underlying/B.h
 // RUN: echo '#ifdef NESTED' >> %t/Underlying/B.h
 // RUN: echo '#include "C.h"' >> %t/Underlying/B.h
 // RUN: echo '#endif' >> %t/Underlying/B.h
 // RUN: echo '// C.h in Underlying' > %t/Underlying/C.h
-// RUN: echo '// C.h in Middle' > %t/Middle/C.h
 // RUN: echo '// C.h in Overlay' > %t/Overlay/C.h
 
 // 1) Underlying -> Overlay (C.h found, B.h falling back to Underlying)
 // RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}/Overlay@g" -e "s@OUT_DIR@%{/t:regex_replacement}/Underlying@g" %S/Inputs/vfsoverlay-directory.yaml > %t/vfs.yaml
 // RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
 // RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -fsyntax-only -DNESTED -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
 // RUN: sed -e "s@INPUT_DIR@Overlay@g" -e "s@OUT_DIR@%{/t:regex_replacement}/Underlying@g" %S/Inputs/vfsoverlay-directory-relative.yaml > %t/vfs-relative.yaml
 // RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs-relative.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
 
+// DIRECT: # 1 "{{.*(/|\\\\)Underlying(/|\\\\)}}B.h"
 // DIRECT: {{^}}// B.h in Underlying
+// DIRECT: # 1 "{{.*(/|\\\\)Overlay(/|\\\\)}}C.h"
 // DIRECT: {{^}}// C.h in Overlay
 
-// 2) Underlying -> Middle -> Overlay (C.h found, B.h falling back to Underlying)
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}/Overlay@g" -e "s@OUT_DIR@%{/t:regex_replacement}/Middle@g" %S/Inputs/vfsoverlay-directory.yaml > %t/vfs.yaml
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}/Middle@g" -e "s@OUT_DIR@%{/t:regex_replacement}/Underlying@g" %S/Inputs/vfsoverlay-directory.yaml > %t/vfs2.yaml
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -DNESTED -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
-
-// Same as direct above
-
-// 3) Underlying -> Middle -> Overlay (C.h falling back to Middle, B.h falling back to Underlying)
-// RUN: rm -f %t/Overlay/C.h
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=FALLBACK %s
-
-// FALLBACK: {{^}}// B.h in Underlying
-// FALLBACK: {{^}}// C.h in Middle
-
-// 3) Underlying -> Middle -> Overlay (C.h falling back to Underlying, B.h falling back to Underlying)
-// RUN: rm -f %t/Middle/C.h
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=FALLBACK2 %s
-
-// FALLBACK2: {{^}}// B.h in Underlying
-// FALLBACK2: {{^}}// C.h in Underlying
-
 #include "B.h"
 #ifndef NESTED
 #include "C.h"
 #endif

clang/test/VFS/multiple-overlays.c

+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@EXTERNAL_DIR@%{/t:regex_replacement}/B@g" -e "s@NAME_DIR@%{/t:regex_replacement}/A@g" %t/vfs/base.yaml > %t/vfs/a-b.yaml
+// RUN: sed -e "s@EXTERNAL_DIR@%{/t:regex_replacement}/C@g" -e "s@NAME_DIR@%{/t:regex_replacement}/B@g" %t/vfs/base.yaml > %t/vfs/b-c.yaml
+
+// Overlays should not be transitive, ie. given overlays of A -> B and B -> C, A should not remap to
+// C.
+
+// RUN: %clang_cc1 -Werror -I %t/A -ivfsoverlay %t/vfs/b-c.yaml -ivfsoverlay %t/vfs/a-b.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=FROM_B %s
+// FROM_B: # 1 "{{.*(/|\\\\)B(/|\\\\)}}Header.h"
+// FROM_B: // Header.h in B
+
+// RUN: %clang_cc1 -Werror -I %t/B -ivfsoverlay %t/vfs/b-c.yaml -ivfsoverlay %t/vfs/a-b.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=FROM_C %s
+// FROM_C: # 1 "{{.*(/|\\\\)C(/|\\\\)}}Header.h"
+// FROM_C: // Header.h in C
+
+//--- main.c
+#include "Header.h"
+
+//--- A/Header.h
+// Header.h in A
+
+//--- B/Header.h
+// Header.h in B
+
+//--- C/Header.h
+// Header.h in C
+
+//--- vfs/base.yaml
+{
+  'version': 0,
+  'redirecting-with': 'fallthrough',
+  'roots': [
+    { 'name': 'NAME_DIR',
+      'type': 'directory-remap',
+      'external-contents': 'EXTERNAL_DIR'
+    }
+  ]
+}

clang/test/VFS/vfsroot-with-overlay.c

 // RUN: rm -rf %t
 // RUN: mkdir -p %t
 // RUN: sed -e "s@TEST_DIR@%{/S:regex_replacement}@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsroot.yaml > %t.yaml
-// RUN: sed -e "s@INPUT_DIR@/indirect-vfs-root-files@g" -e "s@OUT_DIR@/overlay-dir@g" %S/Inputs/vfsoverlay.yaml > %t/vfsoverlay.yaml
+// RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@/overlay-dir@g" %S/Inputs/vfsoverlay.yaml > %t/vfsoverlay.yaml
 // RUN: %clang_cc1 -Werror -ivfsoverlay %t.yaml -ivfsoverlay /direct-vfs-root-files/vfsoverlay.yaml -I /overlay-dir -fsyntax-only /tests/vfsroot-with-overlay.c
 
 #include "not_real.h"
 
 void foo(void) {
   bar();
 }

lldb/source/Commands/CommandObjectReproducer.cpp

bool DoExecute(Args &command, CommandReturnObject &result) override {
     Loader *loader =
         GetLoaderFromPathOrCurrent(loader_storage, result, m_options.file);
     if (!loader)
       return false;
 
     switch (m_options.provider) {
     case eReproducerProviderFiles: {
       FileSpec vfs_mapping = loader->GetFile<FileProvider::Info>();
+      std::string overlay_path = vfs_mapping.GetPath();
 
-      // Read the VFS mapping.
-      ErrorOr<std::unique_ptr<MemoryBuffer>> buffer =
-          vfs::getRealFileSystem()->getBufferForFile(vfs_mapping.GetPath());
-      if (!buffer) {
-        SetError(result, errorCodeToError(buffer.getError()));
+      Expected<IntrusiveRefCntPtr<vfs::FileSystem>> vfs =
+          vfs::getVFSFromYAMLs(StringRef(overlay_path));
+      if (auto err = vfs.takeError()) {
+        SetError(result, std::move(err));
         return false;
       }
 
-      // Initialize a VFS from the given mapping.
-      IntrusiveRefCntPtr<vfs::FileSystem> vfs = vfs::getVFSFromYAML(
-          std::move(buffer.get()), nullptr, vfs_mapping.GetPath());
-
       // Dump the VFS to a buffer.
       std::string str;
       raw_string_ostream os(str);
-      static_cast<vfs::RedirectingFileSystem &>(*vfs).print(os);
+      (*vfs)->print(os);
       os.flush();
 
       // Return the string.
       result.AppendMessage(str);
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return true;
     }
     case eReproducerProviderSymbolFiles: {

llvm/include/llvm/Support/VirtualFileSystem.h

 protected:
   void printImpl(raw_ostream &OS, PrintType Type,
                  unsigned IndentLevel) const override;
 };
 
 /// Get a globally unique ID for a virtual file or directory.
 llvm::sys::fs::UniqueID getNextVirtualUniqueID();
 
-/// Gets a \p FileSystem for a virtual file system described in YAML
-/// format.
+/// \see getVFSFromYAMLs
+///
+/// Returns nullptr if \p Buffer could not be parsed as a YAML containing
+/// overlay mappings.
 std::unique_ptr<FileSystem>
 getVFSFromYAML(std::unique_ptr<llvm::MemoryBuffer> Buffer,
                llvm::SourceMgr::DiagHandlerTy DiagHandler,
                StringRef YAMLFilePath, void *DiagContext = nullptr,
                IntrusiveRefCntPtr<FileSystem> ExternalFS = getRealFileSystem());
 
+/// Convenience function to read each file from \p ExternalFS and pass their
+/// buffers along to the next overload. Returns a \c FileError if any file
+/// could not be read.
+Expected<std::unique_ptr<OverlayFileSystem>>
+getVFSFromYAMLs(ArrayRef<StringRef> YAMLOverlayPaths,
+                IntrusiveRefCntPtr<FileSystem> ExternalFS = getRealFileSystem(),
+                SourceMgr::DiagHandlerTy DiagHandler = nullptr,
+                void *DiagContext = nullptr);
+
+/// Gets a \c FileSystem that runs operations on the virtual filesystems
+/// described by each buffer in \p Buffers and then the \p ExternalFS if those
+/// all fail (dependent on the options below). The order that each is run is
+/// the reverse of the list, ie. the last buffer creates the first used
+/// \c FileSystem.
+///
+/// Returns a \c FileError if any of the buffers are invalid and an
+/// \c OverlayFileSystem with just the \p ExternalFS if no buffers were
+/// provided.
+///
+/// Two extra root configuration options are handled by this method when
+/// parsing the YAML of each filesystem:
+///   'fallthrough': <boolean, default=true, deprecated - use 'redirecting-with'
+///                   instead>
+///   'redirecting-with': <string, one of 'fallthrough', 'fallback', or
+///                        'redirect-only', default='fallthrough'>
+///
+/// These specify the order each filesystem is added to an \c OverlayFileSystem
+/// and whether they're added at all -
+///   - 'fallthrough': allow "falling through" to the next filesystem
+///   - 'fallback': run opertions on the next filesystem before this one.
+///                 Most useful when specified on the first overlay, which
+///                 causes the \p ExternalFS to run before it (ie. "fallback"
+///                 to using the mapped paths only when original fails)
+///   - 'redirect-only': skip running operations on any further filesystems,
+///                      ie. this is the last used filesystem. As with
+///                      'fallback', this is more useful when specified on the
+///                      first overlay to avoid also checking the \p ExternalFS.
+///
+/// \see OverlayFileSystem
+/// \see RedirectingFileSystem
+Expected<std::unique_ptr<OverlayFileSystem>>
+getVFSFromYAMLs(ArrayRef<MemoryBufferRef> YAMLOverlays,
+                IntrusiveRefCntPtr<FileSystem> ExternalFS = getRealFileSystem(),
+                SourceMgr::DiagHandlerTy DiagHandler = nullptr,
+                void *DiagContext = nullptr);
+
 struct YAMLVFSEntry {
   template <typename T1, typename T2>
   YAMLVFSEntry(T1 &&VPath, T2 &&RPath, bool IsDirectory = false)
       : VPath(std::forward<T1>(VPath)), RPath(std::forward<T2>(RPath)),
         IsDirectory(IsDirectory) {}
   std::string VPath;
   std::string RPath;
   bool IsDirectory = false;

 ///
 /// The roots may be absolute or relative. If relative they will be made
 /// absolute against the current working directory.
 ///
 /// All configuration options are optional.
 ///   'case-sensitive': <boolean, default=(true for Posix, false for Windows)>
 ///   'use-external-names': <boolean, default=true>
 ///   'overlay-relative': <boolean, default=false>
-///   'fallthrough': <boolean, default=true, deprecated - use 'redirecting-with'
-///                   instead>
-///   'redirecting-with': <string, one of 'fallthrough', 'fallback', or
-///                        'redirect-only', default='fallthrough'>
 ///
 /// Virtual directories that list their contents are represented as
 /// \verbatim
 /// {
 ///   'type': 'directory',
 ///   'name': <string>,
 ///   'contents': [ <file or directory entries> ]
 /// }

 /// FIXME: 'use-external-name' causes behaviour that's inconsistent with how
 /// "real" filesystems behave. Maybe there should be a separate channel for
 /// this information.
 class RedirectingFileSystem : public vfs::FileSystem {
 public:
   enum EntryKind { EK_Directory, EK_DirectoryRemap, EK_File };
   enum NameKind { NK_NotSet, NK_External, NK_Virtual };
 
+  // TODO: Simplify RedirectingFileSystem to remove redirection completely.
+  // Remove RedirectKind, Redirection, and all non-redirect-only paths. Require
+  // any old uses to move to using OverlayFileSystem instead (see the new
+  // getVFSFromYAMLs API).
+
   /// The type of redirection to perform.
   enum class RedirectKind {
     /// Lookup the redirected path first (ie. the one specified in
     /// 'external-contents') and if that fails "fallthrough" to a lookup of the
     /// originally provided path.
     Fallthrough,
     /// Lookup the provided path first and if that fails, "fallback" to a
     /// lookup of the redirected path.
DirectoryEntry(StringRef Name, std::vector<std::unique_ptr<Entry>> Contents,
                    Status S)
         : Entry(EK_Directory, Name), Contents(std::move(Contents)),
           S(std::move(S)) {}
 
     /// Constructs an empty directory entry.
     DirectoryEntry(StringRef Name, Status S)
         : Entry(EK_Directory, Name), S(std::move(S)) {}
 
-    Status getStatus() { return S; }
+    Status getStatus() const { return S; }
 
     void addContent(std::unique_ptr<Entry> Content) {
       Contents.push_back(std::move(Content));
     }
 
     Entry *getLastContent() const { return Contents.back().get(); }
 
     using iterator = decltype(Contents)::iterator;
private:
   std::vector<std::unique_ptr<Entry>> Roots;
 
   /// The current working directory of the file system.
   std::string WorkingDirectory;
 
   /// The file system to use for external references.
   IntrusiveRefCntPtr<FileSystem> ExternalFS;
 
-  /// If IsRelativeOverlay is set, this represents the directory
-  /// path that should be prefixed to each 'external-contents' entry
-  /// when reading from YAML files.
-  std::string ExternalContentsPrefixDir;
-
   /// @name Configuration
   /// @{
 
   /// Whether to perform case-sensitive comparisons.
   ///
   /// Currently, case-insensitive matching only works correctly with ASCII.
   bool CaseSensitive = is_style_posix(sys::path::Style::native);
 
-  /// IsRelativeOverlay marks whether a ExternalContentsPrefixDir path must
-  /// be prefixed in every 'external-contents' when reading from YAML files.
-  bool IsRelativeOverlay = false;
-
   /// Whether to use to use the value of 'external-contents' for the
   /// names of files.  This global value is overridable on a per-file basis.
   bool UseExternalNames = true;
 
   /// Determines the lookups to perform, as well as their order. See
   /// \c RedirectKind for details.
   RedirectKind Redirection = RedirectKind::Fallthrough;
   /// @}
create(std::unique_ptr<MemoryBuffer> Buffer,
          void *DiagContext, IntrusiveRefCntPtr<FileSystem> ExternalFS);
 
   /// Redirect each of the remapped files from first to second.
   static std::unique_ptr<RedirectingFileSystem>
   create(ArrayRef<std::pair<std::string, std::string>> RemappedFiles,
          bool UseExternalNames, FileSystem &ExternalFS);
 
   ErrorOr<Status> status(const Twine &Path) override;
+
   ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override;
 
   std::error_code getRealPath(const Twine &Path,
                               SmallVectorImpl<char> &Output) const override;
 
   llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override;
 
   std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
 
   std::error_code isLocal(const Twine &Path, bool &Result) override;
 
   std::error_code makeAbsolute(SmallVectorImpl<char> &Path) const override;
 
   directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
 
-  void setExternalContentsPrefixDir(StringRef PrefixDir);
-
-  StringRef getExternalContentsPrefixDir() const;
-
   /// Sets the redirection kind to \c Fallthrough if true or \c RedirectOnly
   /// otherwise. Will removed in the future, use \c setRedirection instead.
   void setFallthrough(bool Fallthrough);
 
   void setRedirection(RedirectingFileSystem::RedirectKind Kind);
 
   std::vector<llvm::StringRef> getRoots() const;
 

llvm/lib/Support/VirtualFileSystem.cpp

directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
 
   directory_iterator Combined{
       std::make_shared<CombiningDirIterImpl>(Iters, EC)};
   if (EC)
     return {};
   return Combined;
 }
 
-void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) {
-  ExternalContentsPrefixDir = PrefixDir.str();
-}
-
-StringRef RedirectingFileSystem::getExternalContentsPrefixDir() const {
-  return ExternalContentsPrefixDir;
-}
-
 void RedirectingFileSystem::setFallthrough(bool Fallthrough) {
   if (Fallthrough) {
     Redirection = RedirectingFileSystem::RedirectKind::Fallthrough;
   } else {
     Redirection = RedirectingFileSystem::RedirectKind::RedirectOnly;
   }
 }
 
void RedirectingFileSystem::printEntry(raw_ostream &OS,
   }
   }
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 void RedirectingFileSystem::dump() const { print(dbgs()); }
 #endif
 
+namespace {
+
+struct YAMLParseResult {
+  /// The way in which this VFS should be added to an overlay. Note that the
+  /// order is right to left - given [A, B, C], any FS operations run on C
+  /// *first* and if failing, B, followed by A.
+  enum class RedirectKind {
+    /// Allow "falling through" to the next (ie. to the left) VFS.
+    Fallthrough,
+    /// Run any operations on the next VFS *first*, using this VFS as a
+    /// "fallback".
+    Fallback,
+    /// Do not perform any more operations on further VFS.
+    RedirectOnly
+  };
+
+  std::unique_ptr<RedirectingFileSystem> FS;
+  RedirectKind Redirection;
+
+  // TODO: Remove after simplifying RedirectingFileSystem
+  static RedirectingFileSystem::RedirectKind
+  toRFSKind(RedirectKind Redirection) {
+    switch (Redirection) {
+    case RedirectKind::Fallthrough:
+      return RedirectingFileSystem::RedirectKind::Fallthrough;
+    case RedirectKind::Fallback:
+      return RedirectingFileSystem::RedirectKind::Fallback;
+    case RedirectKind::RedirectOnly:
+      return RedirectingFileSystem::RedirectKind::RedirectOnly;
+    }
+  }
+};
+
+} // namespace
+
 /// A helper class to hold the common YAML parsing state.
 class llvm::vfs::RedirectingFileSystemParser {
   yaml::Stream &Stream;
 
   void error(yaml::Node *N, const Twine &Msg) { Stream.printError(N, Msg); }
 
   // false on error
   bool parseScalarString(yaml::Node *N, StringRef &Result,
if (Value.equals_insensitive("true") || Value.equals_insensitive("on") ||
       Result = false;
       return true;
     }
 
     error(N, "expected boolean value");
     return false;
   }
 
-  Optional<RedirectingFileSystem::RedirectKind>
-  parseRedirectKind(yaml::Node *N) {
+  Optional<YAMLParseResult::RedirectKind> parseRedirectKind(yaml::Node *N) {
     SmallString<12> Storage;
     StringRef Value;
     if (!parseScalarString(N, Value, Storage))
       return None;
 
     if (Value.equals_insensitive("fallthrough")) {
-      return RedirectingFileSystem::RedirectKind::Fallthrough;
+      return YAMLParseResult::RedirectKind::Fallthrough;
     } else if (Value.equals_insensitive("fallback")) {
-      return RedirectingFileSystem::RedirectKind::Fallback;
+      return YAMLParseResult::RedirectKind::Fallback;
     } else if (Value.equals_insensitive("redirect-only")) {
-      return RedirectingFileSystem::RedirectKind::RedirectOnly;
+      return YAMLParseResult::RedirectKind::RedirectOnly;
     }
     return None;
   }
 
   struct KeyStatus {
     bool Required;
     bool Seen = false;
 
for (const auto &I : Keys) {
         return false;
       }
     }
     return true;
   }
 
 public:
   static RedirectingFileSystem::Entry *
-  lookupOrCreateEntry(RedirectingFileSystem *FS, StringRef Name,
+  lookupOrCreateEntry(RedirectingFileSystem &FS, StringRef Name,
                       RedirectingFileSystem::Entry *ParentEntry = nullptr) {
     if (!ParentEntry) { // Look for a existent root
-      for (const auto &Root : FS->Roots) {
+      for (const auto &Root : FS.Roots) {
         if (Name.equals(Root->getName())) {
           ParentEntry = Root.get();
           return ParentEntry;
         }
       }
     } else { // Advance to the next component
       auto *DE = dyn_cast<RedirectingFileSystem::DirectoryEntry>(ParentEntry);
       for (std::unique_ptr<RedirectingFileSystem::Entry> &Content :
            llvm::make_range(DE->contents_begin(), DE->contents_end())) {
         auto *DirContent =
             dyn_cast<RedirectingFileSystem::DirectoryEntry>(Content.get());
         if (DirContent && Name.equals(Content->getName()))
           return DirContent;
       }
     }
 
     // ... or create a new one
     std::unique_ptr<RedirectingFileSystem::Entry> E =
         std::make_unique<RedirectingFileSystem::DirectoryEntry>(
             Name, Status("", getNextVirtualUniqueID(),
                          std::chrono::system_clock::now(), 0, 0, 0,
                          file_type::directory_file, sys::fs::all_all));
 
     if (!ParentEntry) { // Add a new root to the overlay
-      FS->Roots.push_back(std::move(E));
-      ParentEntry = FS->Roots.back().get();
+      FS.Roots.push_back(std::move(E));
+      ParentEntry = FS.Roots.back().get();
       return ParentEntry;
     }
 
     auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(ParentEntry);
     DE->addContent(std::move(E));
     return DE->getLastContent();
   }
 
 private:
-  void uniqueOverlayTree(RedirectingFileSystem *FS,
+  void uniqueOverlayTree(RedirectingFileSystem &FS,
                          RedirectingFileSystem::Entry *SrcE,
                          RedirectingFileSystem::Entry *NewParentE = nullptr) {
     StringRef Name = SrcE->getName();
     switch (SrcE->getKind()) {
     case RedirectingFileSystem::EK_Directory: {
       auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(SrcE);
       // Empty directories could be present in the YAML as a way to
       // describe a file for a current directory after some of its subdir
case RedirectingFileSystem::EK_File: {
       DE->addContent(std::make_unique<RedirectingFileSystem::FileEntry>(
           Name, FE->getExternalContentsPath(), FE->getUseName()));
       break;
     }
     }
   }
 
   std::unique_ptr<RedirectingFileSystem::Entry>
-  parseEntry(yaml::Node *N, RedirectingFileSystem *FS, bool IsRootEntry) {
+  parseEntry(yaml::Node *N, bool IsRelativeOverlay,
+             StringRef ExternalContentsPrefixDir, bool IsRootEntry) {
     auto *M = dyn_cast<yaml::MappingNode>(N);
     if (!M) {
       error(N, "expected mapping node for file or directory entry");
       return nullptr;
     }
 
     KeyStatusPair Fields[] = {
         KeyStatusPair("name", true),
for (auto &I : *M) {
         if (!Contents) {
           // FIXME: this is only for directories, what about files?
           error(I.getValue(), "expected array");
           return nullptr;
         }
 
         for (auto &I : *Contents) {
           if (std::unique_ptr<RedirectingFileSystem::Entry> E =
-                  parseEntry(&I, FS, /*IsRootEntry*/ false))
+                  parseEntry(&I, IsRelativeOverlay, ExternalContentsPrefixDir,
+                             /*IsRootEntry=*/false))
             EntryArrayContents.push_back(std::move(E));
           else
             return nullptr;
         }
       } else if (Key == "external-contents") {
         if (ContentsField != CF_NotSet) {
           error(I.getKey(),
                 "entry already has 'contents' or 'external-contents'");
           return nullptr;
         }
         ContentsField = CF_External;
         if (!parseScalarString(I.getValue(), Value, Buffer))
           return nullptr;
 
         SmallString<256> FullPath;
-        if (FS->IsRelativeOverlay) {
-          FullPath = FS->getExternalContentsPrefixDir();
+        if (IsRelativeOverlay) {
+          FullPath.append(ExternalContentsPrefixDir);
           assert(!FullPath.empty() &&
                  "External contents prefix directory must exist");
           llvm::sys::path::append(FullPath, Value);
         } else {
           FullPath = Value;
         }
 
         // Guarantee that old YAML files containing paths with ".." and "."
for (sys::path::reverse_iterator I = sys::path::rbegin(Parent, path_style),
                  0, 0, 0, file_type::directory_file, sys::fs::all_all));
     }
     return Result;
   }
 
 public:
   RedirectingFileSystemParser(yaml::Stream &S) : Stream(S) {}
 
-  // false on error
-  bool parse(yaml::Node *Root, RedirectingFileSystem *FS) {
+  static Optional<YAMLParseResult>
+  parse(MemoryBufferRef Buffer, SourceMgr &SM,
+        IntrusiveRefCntPtr<FileSystem> ExternalFS) {
+    yaml::Stream Stream(Buffer, SM);
+    yaml::document_iterator DI = Stream.begin();
+    yaml::Node *Root = DI->getRoot();
+    if (DI == Stream.end() || !Root) {
+      SM.PrintMessage(SMLoc(), SourceMgr::DK_Error, "expected root node");
+      return None;
+    }
+
+    SmallString<256> ExternalContentsPrefixDir;
+    if (!Buffer.getBufferIdentifier().empty()) {
+      // Use the YAML path from -ivfsoverlay to compute the dir to be prefixed
+      // to each 'external-contents' path.
+      //
+      // Example:
+      //    -ivfsoverlay dummy.cache/vfs/vfs.yaml
+      // yields:
+      //  FS->ExternalContentsPrefixDir => /<absolute_path_to>/dummy.cache/vfs
+      //
+      ExternalContentsPrefixDir =
+          sys::path::parent_path(Buffer.getBufferIdentifier());
+      std::error_code EC = ExternalFS->makeAbsolute(ExternalContentsPrefixDir);
+      assert(!EC && "Overlay dir final path must be absolute");
+      (void)EC;
+    }
+
+    RedirectingFileSystemParser P(Stream);
+    return P.parse(Root, ExternalContentsPrefixDir, std::move(ExternalFS));
+  }
+
+  Optional<YAMLParseResult> parse(yaml::Node *Root,
+                                  StringRef ExternalContentsPrefixDir,
+                                  IntrusiveRefCntPtr<FileSystem> ExternalFS) {
     auto *Top = dyn_cast<yaml::MappingNode>(Root);
     if (!Top) {
       error(Root, "expected mapping node");
-      return false;
+      return None;
     }
 
     KeyStatusPair Fields[] = {
         KeyStatusPair("version", true),
         KeyStatusPair("case-sensitive", false),
         KeyStatusPair("use-external-names", false),
         KeyStatusPair("overlay-relative", false),
         KeyStatusPair("fallthrough", false),
         KeyStatusPair("redirecting-with", false),
         KeyStatusPair("roots", true),
     };
 
     DenseMap<StringRef, KeyStatus> Keys(std::begin(Fields), std::end(Fields));
     std::vector<std::unique_ptr<RedirectingFileSystem::Entry>> RootEntries;
 
+    bool IsRelativeOverlay = false;
+    YAMLParseResult Result{std::unique_ptr<RedirectingFileSystem>(
+                               new RedirectingFileSystem(ExternalFS)),
+                           YAMLParseResult::RedirectKind::Fallthrough};
+    if (auto CWD = ExternalFS->getCurrentWorkingDirectory())
+      Result.FS->setCurrentWorkingDirectory(*CWD);
+
     // Parse configuration and 'roots'
     for (auto &I : *Top) {
       SmallString<10> KeyBuffer;
       StringRef Key;
       if (!parseScalarString(I.getKey(), Key, KeyBuffer))
-        return false;
+        return None;
 
       if (!checkDuplicateOrUnknownKey(I.getKey(), Key, Keys))
-        return false;
+        return None;
 
       if (Key == "roots") {
         auto *Roots = dyn_cast<yaml::SequenceNode>(I.getValue());
         if (!Roots) {
           error(I.getValue(), "expected array");
-          return false;
+          return None;
         }
 
         for (auto &I : *Roots) {
           if (std::unique_ptr<RedirectingFileSystem::Entry> E =
-                  parseEntry(&I, FS, /*IsRootEntry*/ true))
+                  parseEntry(&I, IsRelativeOverlay, ExternalContentsPrefixDir,
+                             /*IsRootEntry=*/true))
             RootEntries.push_back(std::move(E));
           else
-            return false;
+            return None;
         }
       } else if (Key == "version") {
         StringRef VersionString;
         SmallString<4> Storage;
         if (!parseScalarString(I.getValue(), VersionString, Storage))
-          return false;
+          return None;
         int Version;
         if (VersionString.getAsInteger<int>(10, Version)) {
           error(I.getValue(), "expected integer");
-          return false;
+          return None;
         }
         if (Version < 0) {
           error(I.getValue(), "invalid version number");
-          return false;
+          return None;
         }
         if (Version != 0) {
           error(I.getValue(), "version mismatch, expected 0");
-          return false;
+          return None;
         }
       } else if (Key == "case-sensitive") {
-        if (!parseScalarBool(I.getValue(), FS->CaseSensitive))
-          return false;
+        if (!parseScalarBool(I.getValue(), Result.FS->CaseSensitive))
+          return None;
       } else if (Key == "overlay-relative") {
-        if (!parseScalarBool(I.getValue(), FS->IsRelativeOverlay))
-          return false;
+        if (!parseScalarBool(I.getValue(), IsRelativeOverlay))
+          return None;
       } else if (Key == "use-external-names") {
-        if (!parseScalarBool(I.getValue(), FS->UseExternalNames))
-          return false;
+        if (!parseScalarBool(I.getValue(), Result.FS->UseExternalNames))
+          return None;
       } else if (Key == "fallthrough") {
         if (Keys["redirecting-with"].Seen) {
           error(I.getValue(),
                 "'fallthrough' and 'redirecting-with' are mutually exclusive");
-          return false;
+          return None;
         }
 
         bool ShouldFallthrough = false;
         if (!parseScalarBool(I.getValue(), ShouldFallthrough))
-          return false;
+          return None;
 
         if (ShouldFallthrough) {
-          FS->Redirection = RedirectingFileSystem::RedirectKind::Fallthrough;
+          Result.Redirection = YAMLParseResult::RedirectKind::Fallthrough;
+          // TODO: Remove after simplifying RedirectingFileSystem
+          Result.FS->Redirection =
+              RedirectingFileSystem::RedirectKind::Fallthrough;
         } else {
-          FS->Redirection = RedirectingFileSystem::RedirectKind::RedirectOnly;
+          Result.Redirection = YAMLParseResult::RedirectKind::RedirectOnly;
+          // TODO: Remove after simplifying RedirectingFileSystem
+          Result.FS->Redirection =
+              RedirectingFileSystem::RedirectKind::RedirectOnly;
         }
       } else if (Key == "redirecting-with") {
         if (Keys["fallthrough"].Seen) {
           error(I.getValue(),
                 "'fallthrough' and 'redirecting-with' are mutually exclusive");
-          return false;
+          return None;
         }
 
         if (auto Kind = parseRedirectKind(I.getValue())) {
-          FS->Redirection = *Kind;
+          Result.Redirection = *Kind;
+          // TODO: Remove after simplifying RedirectingFileSystem
+          Result.FS->Redirection = YAMLParseResult::toRFSKind(*Kind);
         } else {
           error(I.getValue(), "expected valid redirect kind");
-          return false;
+          return None;
         }
       } else {
         llvm_unreachable("key missing from Keys");
       }
     }
 
     if (Stream.failed())
-      return false;
+      return None;
 
     if (!checkMissingKeys(Top, Keys))
-      return false;
+      return None;
 
     // Now that we sucessefully parsed the YAML file, canonicalize the internal
     // representation to a proper directory tree so that we can search faster
     // inside the VFS.
     for (auto &E : RootEntries)
-      uniqueOverlayTree(FS, E.get());
+      uniqueOverlayTree(*Result.FS, E.get());
 
-    return true;
+    return Result;
   }
 };
 
 std::unique_ptr<RedirectingFileSystem>
 RedirectingFileSystem::create(std::unique_ptr<MemoryBuffer> Buffer,
                               SourceMgr::DiagHandlerTy DiagHandler,
                               StringRef YAMLFilePath, void *DiagContext,
                               IntrusiveRefCntPtr<FileSystem> ExternalFS) {
   SourceMgr SM;
-  yaml::Stream Stream(Buffer->getMemBufferRef(), SM);
-
   SM.setDiagHandler(DiagHandler, DiagContext);
-  yaml::document_iterator DI = Stream.begin();
-  yaml::Node *Root = DI->getRoot();
-  if (DI == Stream.end() || !Root) {
-    SM.PrintMessage(SMLoc(), SourceMgr::DK_Error, "expected root node");
-    return nullptr;
-  }
 
-  RedirectingFileSystemParser P(Stream);
-
-  std::unique_ptr<RedirectingFileSystem> FS(
-      new RedirectingFileSystem(ExternalFS));
-
-  if (!YAMLFilePath.empty()) {
-    // Use the YAML path from -ivfsoverlay to compute the dir to be prefixed
-    // to each 'external-contents' path.
-    //
-    // Example:
-    //    -ivfsoverlay dummy.cache/vfs/vfs.yaml
-    // yields:
-    //  FS->ExternalContentsPrefixDir => /<absolute_path_to>/dummy.cache/vfs
-    //
-    SmallString<256> OverlayAbsDir = sys::path::parent_path(YAMLFilePath);
-    std::error_code EC = llvm::sys::fs::make_absolute(OverlayAbsDir);
-    assert(!EC && "Overlay dir final path must be absolute");
-    (void)EC;
-    FS->setExternalContentsPrefixDir(OverlayAbsDir);
-  }
-
-  if (!P.parse(Root, FS.get()))
-    return nullptr;
-
-  return FS;
+  if (Optional<YAMLParseResult> Result = RedirectingFileSystemParser::parse(
+          Buffer->getMemBufferRef(), SM, std::move(ExternalFS)))
+    return std::move(Result->FS);
 }
 
 std::unique_ptr<RedirectingFileSystem> RedirectingFileSystem::create(
     ArrayRef<std::pair<std::string, std::string>> RemappedFiles,
     bool UseExternalNames, FileSystem &ExternalFS) {
   std::unique_ptr<RedirectingFileSystem> FS(
       new RedirectingFileSystem(&ExternalFS));
   FS->UseExternalNames = UseExternalNames;
if (ToEntry)
       continue;
 
     // Add parent directories.
     RedirectingFileSystem::Entry *Parent = nullptr;
     StringRef FromDirectory = llvm::sys::path::parent_path(From);
     for (auto I = llvm::sys::path::begin(FromDirectory),
               E = llvm::sys::path::end(FromDirectory);
          I != E; ++I) {
-      Parent = RedirectingFileSystemParser::lookupOrCreateEntry(FS.get(), *I,
-                                                                Parent);
+      Parent =
+          RedirectingFileSystemParser::lookupOrCreateEntry(*FS, *I, Parent);
     }
     assert(Parent && "File without a directory?");
     {
       auto EC = ExternalFS.makeAbsolute(To);
       (void)EC;
       assert(!EC && "Could not make absolute path");
     }
 
class FileWithFixedStatus : public File {
   std::unique_ptr<File> InnerFile;
   Status S;
 
 public:
   FileWithFixedStatus(std::unique_ptr<File> InnerFile, Status S)
       : InnerFile(std::move(InnerFile)), S(std::move(S)) {}
 
   ErrorOr<Status> status() override { return S; }
-  ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
 
+  ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
   getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
             bool IsVolatile) override {
     return InnerFile->getBuffer(Name, FileSize, RequiresNullTerminator,
                                 IsVolatile);
   }
 
   std::error_code close() override { return InnerFile->close(); }
 
RedirectingFileSystem::getRealPath(const Twine &OriginalPath,
   return llvm::errc::invalid_argument;
 }
 
 std::unique_ptr<FileSystem>
 vfs::getVFSFromYAML(std::unique_ptr<MemoryBuffer> Buffer,
                     SourceMgr::DiagHandlerTy DiagHandler,
                     StringRef YAMLFilePath, void *DiagContext,
                     IntrusiveRefCntPtr<FileSystem> ExternalFS) {
-  return RedirectingFileSystem::create(std::move(Buffer), DiagHandler,
-                                       YAMLFilePath, DiagContext,
-                                       std::move(ExternalFS));
+  MemoryBufferRef BufferRef{Buffer->getBuffer(), YAMLFilePath};
+  auto FS = getVFSFromYAMLs(BufferRef, ExternalFS, DiagHandler, DiagContext);
+  if (auto Err = FS.takeError()) {
+    consumeError(std::move(Err));
+    return nullptr;
+  }
+  return std::move(*FS);
+}
+
+namespace {
+
+struct VFSResult {
+  IntrusiveRefCntPtr<FileSystem> FS;
+  YAMLParseResult::RedirectKind Redirection;
+
+  VFSResult(IntrusiveRefCntPtr<FileSystem> FS,
+            YAMLParseResult::RedirectKind Redirection)
+      : FS(std::move(FS)), Redirection(Redirection) {}
+
+  VFSResult(YAMLParseResult &Result)
+      : FS(std::move(Result.FS)), Redirection(Result.Redirection) {}
+};
+
+} // namespace
+
+static std::unique_ptr<OverlayFileSystem>
+createOverlay(ArrayRef<VFSResult> VFSResults) {
+  if (VFSResults.empty())
+    return nullptr;
+
+  auto OverlayFS = std::make_unique<OverlayFileSystem>(VFSResults.front().FS);
+  for (const auto &Result : VFSResults.drop_front()) {
+    OverlayFS->pushOverlay(Result.FS);
+  }
+  return OverlayFS;
+}
+
+Expected<std::unique_ptr<OverlayFileSystem>>
+vfs::getVFSFromYAMLs(ArrayRef<StringRef> YAMLOverlayPaths,
+                     IntrusiveRefCntPtr<FileSystem> ExternalFS,
+                     SourceMgr::DiagHandlerTy DiagHandler, void *DiagContext) {
+  SmallVector<MemoryBufferRef, 2> BufferRefs;
+  for (StringRef Path : YAMLOverlayPaths) {
+    BufferRefs.emplace_back(StringRef(), Path);
+  }
+  return getVFSFromYAMLs(BufferRefs, std::move(ExternalFS), DiagHandler,
+                         DiagContext);
+}
+
+Expected<std::unique_ptr<OverlayFileSystem>>
+vfs::getVFSFromYAMLs(ArrayRef<MemoryBufferRef> YAMLOverlays,
+                     IntrusiveRefCntPtr<FileSystem> ExternalFS,
+                     SourceMgr::DiagHandlerTy DiagHandler, void *DiagContext) {
+  if (YAMLOverlays.empty())
+    return std::make_unique<OverlayFileSystem>(std::move(ExternalFS));
+
+  SourceMgr SM;
+  SM.setDiagHandler(DiagHandler, DiagContext);
+
+  SmallVector<VFSResult, 2> Overlays;
+  Overlays.emplace_back(ExternalFS, YAMLParseResult::RedirectKind::Fallthrough);
+  std::unique_ptr<OverlayFileSystem> OverlayFS = createOverlay(Overlays);
+
+  for (MemoryBufferRef Buffer : YAMLOverlays) {
+    // Bit of a hack. If the buffer's data is a nullptr (not just empty),
+    // attempt to read the the file from the current FS. This is to
+    // differentiate between an actual buffer and one that was made from just
+    // a path. The paths cannot be read all up front in order to support
+    // overlay files being defined in a VFS.
+    std::unique_ptr<MemoryBuffer> TempBuffer;
+    if (Buffer.getBuffer().data() == nullptr) {
+      ErrorOr<std::unique_ptr<MemoryBuffer>> NewBufferOrError =
+          OverlayFS->getBufferForFile(Buffer.getBufferIdentifier());
+      if (!NewBufferOrError)
+        return createFileError(Buffer.getBufferIdentifier(),
+                               NewBufferOrError.getError());
+      TempBuffer = std::move(*NewBufferOrError);
+      Buffer = MemoryBufferRef(TempBuffer->getBuffer(),
+                               Buffer.getBufferIdentifier());
+    }
+
+    Optional<YAMLParseResult> Result =
+        RedirectingFileSystemParser::parse(Buffer, SM, ExternalFS);
+    if (!Result)
+      return createFileError(Buffer.getBufferIdentifier(),
+                             llvm::errc::invalid_argument);
+
+    // TODO: Remove after simplifying RedirectingFileSystem
+    Result->FS->setRedirection(
+        RedirectingFileSystem::RedirectKind::RedirectOnly);
+    switch (Result->Redirection) {
+    case YAMLParseResult::RedirectKind::Fallthrough:
+      // Simple case - add on the FS
+      Overlays.emplace_back(*Result);
+      OverlayFS->pushOverlay(Overlays.back().FS);
+      break;
+    case YAMLParseResult::RedirectKind::Fallback:
+      // Fallback implies that the previously added FS should run operations
+      // before hitting this FS. This doesn't make a whole lot of sense for any
+      // position other than the beginning where we want the external FS to run
+      // first. For other positions the overlays could just be specified in the
+      // opposite order.
+      //
+      // This is fallout from the history of \c RedirectingFileSystem. Ideally
+      // we would just depend on order and have some way to specify the real
+      // filesystem.
+
+      if (Overlays.back().Redirection !=
+          YAMLParseResult::RedirectKind::RedirectOnly) {
+        // If the previously inserted overlay is redirect only, don't bother
+        // adding this one - it would never run anyway (since redirect only
+        // is necessarily the last FS).
+
+        // Note: Overlays is never empty, it always has at least the
+        // \c ExternalFS
+        Overlays.insert(Overlays.end() - 1, VFSResult(*Result));
+        OverlayFS = createOverlay(Overlays);
+      }
+      break;
+    case YAMLParseResult::RedirectKind::RedirectOnly:
+      // This is now the last FS, clear the rest if there were any
+      Overlays.clear();
+      Overlays.emplace_back(*Result);
+      OverlayFS = createOverlay(Overlays);
+      break;
+    }
+  }
+
+  return OverlayFS;
 }
 
 static void getVFSEntries(RedirectingFileSystem::Entry *SrcE,
                           SmallVectorImpl<StringRef> &Path,
                           SmallVectorImpl<YAMLVFSEntry> &Entries) {
   auto Kind = SrcE->getKind();
   if (Kind == RedirectingFileSystem::EK_Directory) {
     auto *DE = dyn_cast<RedirectingFileSystem::DirectoryEntry>(SrcE);

llvm/tools/dsymutil/Reproducer.h

private:
   /// The path to the reproducer.
   std::string Root;
 
   /// The FileCollector used by the FileCollectorFileSystem.
   std::shared_ptr<FileCollector> FC;
 };
 
 /// Reproducer instance used to use an existing reproducer. The VFS returned by
-/// this instance is a RedirectingFileSystem that remaps paths to their
-/// counterpart in the reproducer.
+/// this instance remaps paths to their counterpart in the reproducer.
 class ReproducerUse : public Reproducer {
 public:
   ReproducerUse(StringRef Root, std::error_code &EC);
   ~ReproducerUse() override;
 
 private:
   /// The path to the reproducer.
   std::string Root;
 };
 
 } // end namespace dsymutil
 } // end namespace llvm
 
 #endif // LLVM_TOOLS_DSYMUTIL_REPRODUCER_H

llvm/tools/dsymutil/Reproducer.cpp

ReproducerGenerate::~ReproducerGenerate() {
   outs() << "reproducer written to " << Root << '\n';
 }
 
 ReproducerUse::~ReproducerUse() = default;
 
 ReproducerUse::ReproducerUse(StringRef Root, std::error_code &EC) {
   SmallString<128> Mapping(Root);
   sys::path::append(Mapping, "mapping.yaml");
-  ErrorOr<std::unique_ptr<MemoryBuffer>> Buffer =
-      vfs::getRealFileSystem()->getBufferForFile(Mapping.str());
 
-  if (!Buffer) {
-    EC = Buffer.getError();
+  auto OverlayFS = llvm::vfs::getVFSFromYAMLs(Mapping.str());
+  if (auto Err = OverlayFS.takeError()) {
+    llvm::handleAllErrors(std::move(Err), [&](const llvm::ErrorInfoBase &E) {
+      EC = E.convertToErrorCode();
+    });
     return;
   }
-
-  VFS = llvm::vfs::getVFSFromYAML(std::move(Buffer.get()), nullptr, Mapping);
+  VFS = std::move(*OverlayFS);
 }
 
 llvm::Expected<std::unique_ptr<Reproducer>>
 Reproducer::createReproducer(ReproducerMode Mode, StringRef Root) {
   switch (Mode) {
   case ReproducerMode::Generate: {
     std::error_code EC;
     std::unique_ptr<Reproducer> Repro =

llvm/unittests/Support/VirtualFileSystemTest.cpp

 //===- unittests/Support/VirtualFileSystem.cpp -------------- VFS tests ---===//
 //
 // 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 "llvm/Support/VirtualFileSystem.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Testing/Support/Error.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <map>
 #include <string>
 
 using namespace llvm;
 using llvm::sys::fs::UniqueID;
TEST_F(VFSFromYAMLTest, GetRealPath) {
   EXPECT_EQ(RealPath.str(), "//dir/");
 
   // Try a non-existing file.
   EXPECT_EQ(FS->getRealPath("/non_existing", RealPath),
             errc::no_such_file_or_directory);
 }
 
 TEST_F(VFSFromYAMLTest, WorkingDirectory) {
-  IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
+  IntrusiveRefCntPtr<ErrorDummyFileSystem> Lower(new ErrorDummyFileSystem());
   Lower->addDirectory("//root/");
   Lower->addDirectory("//root/foo");
   Lower->addRegularFile("//root/foo/a");
   Lower->addRegularFile("//root/foo/b");
   IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
       "{ 'use-external-names': false,\n"
       "  'roots': [\n"
       "{\n"
       "  'type': 'directory',\n"
       "  'name': '//root/bar',\n"
       "  'contents': [ {\n"
       "                  'type': 'file',\n"
       "                  'name': 'a',\n"
       "                  'external-contents': '//root/foo/a'\n"
       "                }\n"
       "              ]\n"
       "}\n"
       "]\n"
       "}",
       Lower);
   ASSERT_NE(FS.get(), nullptr);
+
   std::error_code EC = FS->setCurrentWorkingDirectory("//root/bar");
   ASSERT_FALSE(EC);
 
   llvm::ErrorOr<std::string> WorkingDir = FS->getCurrentWorkingDirectory();
   ASSERT_TRUE(WorkingDir);
   EXPECT_EQ(*WorkingDir, "//root/bar");
 
   llvm::ErrorOr<vfs::Status> Status = FS->status("./a");
TEST_F(VFSFromYAMLTest, WorkingDirectory) {
   EXPECT_EQ(*WorkingDir, "//root/bar");
 
   EC = FS->setCurrentWorkingDirectory("//root/");
   ASSERT_FALSE(EC);
   WorkingDir = FS->getCurrentWorkingDirectory();
   ASSERT_TRUE(WorkingDir);
   EXPECT_EQ(*WorkingDir, "//root/");
 
+  Status = FS->status("bar/a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+
+  Status = FS->status("foo/a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+
   EC = FS->setCurrentWorkingDirectory("bar");
   ASSERT_FALSE(EC);
   WorkingDir = FS->getCurrentWorkingDirectory();
   ASSERT_TRUE(WorkingDir);
   EXPECT_EQ(*WorkingDir, "//root/bar");
 }
 
 TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthrough) {
TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthrough) {
   ASSERT_FALSE(Status.getError());
   EXPECT_TRUE(Status->exists());
 
   Status = FS->status("../bar/baz/a");
   ASSERT_FALSE(Status.getError());
   EXPECT_TRUE(Status->exists());
 }
 
-TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthroughInvalid) {
-  IntrusiveRefCntPtr<ErrorDummyFileSystem> Lower(new ErrorDummyFileSystem());
-  Lower->addDirectory("//root/");
-  Lower->addDirectory("//root/foo");
-  Lower->addRegularFile("//root/foo/a");
-  Lower->addRegularFile("//root/foo/b");
-  Lower->addRegularFile("//root/c");
-  IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
-      "{ 'use-external-names': false,\n"
-      "  'roots': [\n"
-      "{\n"
-      "  'type': 'directory',\n"
-      "  'name': '//root/bar',\n"
-      "  'contents': [ {\n"
-      "                  'type': 'file',\n"
-      "                  'name': 'a',\n"
-      "                  'external-contents': '//root/foo/a'\n"
-      "                }\n"
-      "              ]\n"
-      "}\n"
-      "]\n"
-      "}",
-      Lower);
-  ASSERT_NE(FS.get(), nullptr);
-  std::error_code EC = FS->setCurrentWorkingDirectory("//root/");
-  ASSERT_FALSE(EC);
-  ASSERT_NE(FS.get(), nullptr);
-
-  llvm::ErrorOr<vfs::Status> Status = FS->status("bar/a");
-  ASSERT_FALSE(Status.getError());
-  EXPECT_TRUE(Status->exists());
-
-  Status = FS->status("foo/a");
-  ASSERT_FALSE(Status.getError());
-  EXPECT_TRUE(Status->exists());
-}
-
 TEST_F(VFSFromYAMLTest, VirtualWorkingDirectory) {
   IntrusiveRefCntPtr<ErrorDummyFileSystem> Lower(new ErrorDummyFileSystem());
   Lower->addDirectory("//root/");
   Lower->addDirectory("//root/foo");
   Lower->addRegularFile("//root/foo/a");
   Lower->addRegularFile("//root/foo/b");
   Lower->addRegularFile("//root/c");
   IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
ASSERT_EQ("RedirectingFileSystem (UseExternalNames: true)\n"
             "  'dremap' -> '/a'\n"
             "  'vdir'\n"
             "    'dremap' -> '/b' (UseExternalName: true)\n"
             "    'vfile' -> '/c' (UseExternalName: false)\n"
             "ExternalFS:\n"
             "  DummyFileSystem (RecursiveContents)\n",
             Output);
 }
+
+static std::unique_ptr<vfs::OverlayFileSystem>
+getVFSOrNull(ArrayRef<std::string> YAMLOverlays,
+             IntrusiveRefCntPtr<vfs::FileSystem> ExternalFS) {
+  SmallVector<MemoryBufferRef> OverlayRefs;
+  for (const auto &Overlay : YAMLOverlays) {
+    OverlayRefs.emplace_back(Overlay, "");
+  }
+
+  auto ExpectedFS = vfs::getVFSFromYAMLs(OverlayRefs, ExternalFS);
+  if (auto Err = ExpectedFS.takeError()) {
+    consumeError(std::move(Err));
+    return nullptr;
+  }
+  return std::move(*ExpectedFS);
+}
+
+static std::string createSimpleOverlay(StringRef RedirectKind, StringRef From,
+                                       StringRef To) {
+  return ("{\n"
+          "  'version': 0,\n"
+          "  'redirecting-with': '" +
+          RedirectKind +
+          "'\n"
+          "  'roots': [\n"
+          "     {\n"
+          "       'type': 'directory-remap',\n"
+          "       'name': '" +
+          From +
+          "',\n"
+          "       'external-contents': '" +
+          To +
+          "',\n"
+          "       }]\n"
+          "     }"
+          "  ]")
+      .str();
+}
+
+// Make sure that overlays are not transitive. Given A -> B and B -> C, if a
+// file in A is requested, it should not end up mapping to C.
+TEST(VFSFromYAMLsTest, NotTransitive) {
+  auto C = makeIntrusiveRefCnt<DummyFileSystem>();
+  C->addDirectory("/real/c");
+  C->addRegularFile("/real/c/f");
+
+  SmallVector<std::string> Overlays;
+  Overlays.push_back(createSimpleOverlay("fallthrough", "/b", "/real/c"));
+  Overlays.push_back(createSimpleOverlay("fallthrough", "/a", "/b"));
+  auto FS = getVFSOrNull(Overlays, C);
+  ASSERT_TRUE(FS);
+
+  EXPECT_PATHS(*FS, "/b/f", "/real/c/f", "/real/c/f");
+  auto S = FS->status("/b/f");
+  ASSERT_FALSE(S.getError());
+  EXPECT_EQ("/real/c/f", S->getName());
+
+  EXPECT_TRUE(FS->status("/a/f").getError());
+}
+
+// When fallback is the first overlay, the external FS should be checked before
+// it. Given B -> A, A should only be used if the file does not exist in B.
+TEST(VFSFromYAMLsTest, FallbackChecksExternalFirst) {
+  auto AOnly = makeIntrusiveRefCnt<DummyFileSystem>();
+  AOnly->addDirectory("/a");
+  AOnly->addRegularFile("/a/f");
+
+  auto BOnly = makeIntrusiveRefCnt<DummyFileSystem>();
+  BOnly->addDirectory("/b");
+  BOnly->addRegularFile("/b/f");
+
+  auto Both = makeIntrusiveRefCnt<DummyFileSystem>();
+  Both->addDirectory("/a");
+  Both->addRegularFile("/a/f");
+  Both->addDirectory("/b");
+  Both->addRegularFile("/b/f");
+
+  auto FallbackOverlay = createSimpleOverlay("fallback", "/b", "/a");
+
+  auto FS = getVFSOrNull(FallbackOverlay, AOnly);
+  ASSERT_TRUE(FS);
+  EXPECT_PATHS(*FS, "/b/f", "/a/f", "/a/f");
+
+  FS = getVFSOrNull(FallbackOverlay, BOnly);
+  ASSERT_TRUE(FS);
+  EXPECT_PATHS(*FS, "/b/f", "/b/f", "/b/f");
+
+  FS = getVFSOrNull(FallbackOverlay, Both);
+  ASSERT_TRUE(FS);
+  EXPECT_PATHS(*FS, "/b/f", "/b/f", "/b/f");
+}
+
+// Ensure the last overlay with redirect-only specified is the final FS
+TEST(VFSFromYAMLsTest, RedirectOnlyIsFinalFS) {
+  auto Real = makeIntrusiveRefCnt<DummyFileSystem>();
+  Real->addDirectory("/real");
+  Real->addRegularFile("/real/f");
+  Real->setCurrentWorkingDirectory("/real");
+
+  SmallVector<std::string> Overlays;
+  Overlays.push_back(createSimpleOverlay("redirect-only", "/ro1", "/real"));
+  Overlays.push_back(createSimpleOverlay("fallthrough", "/ft1", "/real"));
+  Overlays.push_back(createSimpleOverlay("redirect-only", "/ro2", "/real"));
+  Overlays.push_back(createSimpleOverlay("fallback", "/fb", "/real"));
+  Overlays.push_back(createSimpleOverlay("fallthrough", "/ft2", "/real"));
+  auto FS = getVFSOrNull(Overlays, Real);
+  ASSERT_TRUE(FS);
+
+  // Should have the same CWD as the external FS we passed down, even if that
+  // FS isn't actually being used in the overlay any more.
+  auto CWD = FS->getCurrentWorkingDirectory();
+  ASSERT_FALSE(CWD.getError());
+  EXPECT_EQ(*CWD, "/real");
+
+  // Only //ft2/f and //ro2/f should be valid:
+  //  - //ro1 and //ft1 are specified before //ro2 so never run
+  //  - //fb is specified after, but it's set to fallback and hence //ro2
+  //    should run first (and not continue)
+
+  EXPECT_TRUE(FS->status("/ro1/f").getError());
+  EXPECT_TRUE(FS->status("/ft1/f").getError());
+  EXPECT_TRUE(FS->status("/fb/f").getError());
+
+  EXPECT_PATHS(*FS, "/ro2/f", "/real/f", "/real/f");
+  EXPECT_PATHS(*FS, "/ft2/f", "/real/f", "/real/f");
+}
+
+// Ensure overlays are read from the OverlayFS built so far
+TEST(VFSFromYAMLsTest, OverlayFromVFS) {
+  std::string FT1 = createSimpleOverlay("fallthrough", "/vfs1", "/a");
+  std::string FT2 = createSimpleOverlay("fallthrough", "/vfs2", "/b");
+  std::string FT3 = createSimpleOverlay("fallthrough", "/vfs3", "/c");
+  std::string RO = createSimpleOverlay("redirect-only", "/vfs", "/a");
+
+  auto Real = makeIntrusiveRefCnt<vfs::InMemoryFileSystem>();
+  Real->addFile("/ft1.yaml", 0, MemoryBuffer::getMemBuffer(FT1));
+  Real->addFile("/ft2.yaml", 0, MemoryBuffer::getMemBuffer(FT2));
+  Real->addFile("/a/ft3.yaml", 0, MemoryBuffer::getMemBuffer(FT3));
+  Real->addFile("/ro.yaml", 0, MemoryBuffer::getMemBuffer(RO));
+  Real->addFile("/a/f", 0, MemoryBuffer::getMemBuffer("a"));
+  Real->addFile("/b/f", 0, MemoryBuffer::getMemBuffer("b"));
+  Real->addFile("/c/f", 0, MemoryBuffer::getMemBuffer("c"));
+  Real->setCurrentWorkingDirectory("/");
+
+  // 3 to check we're not just using the last overlay
+  auto ExpectedFS =
+      vfs::getVFSFromYAMLs({"/ft1.yaml", "/ft2.yaml", "/vfs1/ft3.yaml"}, Real);
+  ASSERT_THAT_EXPECTED(ExpectedFS, Succeeded());
+  auto FS = std::move(*ExpectedFS);
+  FS->print(llvm::errs(), vfs::FileSystem::PrintType::RecursiveContents);
+  ASSERT_TRUE(FS);
+  EXPECT_PATHS(*FS, "/vfs1/f", "/a/f", "/a/f");
+  EXPECT_PATHS(*FS, "/vfs2/f", "/b/f", "/b/f");
+  EXPECT_PATHS(*FS, "/vfs3/f", "/c/f", "/c/f");
+
+  // If redirect-only is given, any further overlays *must* be specified by it
+  ExpectedFS = vfs::getVFSFromYAMLs({"/ro.yaml", "/a/ft3.yaml"}, Real);
+  EXPECT_THAT_EXPECTED(ExpectedFS, Failed());
+
+  ExpectedFS = vfs::getVFSFromYAMLs({"/ro.yaml", "/vfs/ft3.yaml"}, Real);
+  ASSERT_THAT_EXPECTED(ExpectedFS, Succeeded());
+  FS = std::move(*ExpectedFS);
+  ASSERT_TRUE(FS);
+  EXPECT_PATHS(*FS, "/vfs/f", "/a/f", "/a/f");
+}