[VirtualFileSystem] Make the RedirectingFileSystem hold on to its own working directory.

[JDevlieghere]

Summary

For reproducers it is useful to encode the notion of the current working directory in the VFS mapping. This enables us to handle relative files transparently, as long as the current working directory doesn't change over time. This change is motivated by a current bug in LLDB's reproducers. When a relative path to a binary is passed to the debugger, we fail to resolve it during reproducer replay, because the resolved (absolute) path doesn't match any entry in the VFS.

[sammccall]

It seems conceptually a little strange to have the working directory be part of a serialized "FS", as it's fundamentally a property of a process and only transiently a property of the VFS.
I don't know this is fatal (not sure what else the RedirectingFileSystem might be used for.

What's the reason to store this as part of the VFS rather than as a separate attribute of the larger reproducer structure?

[sammccall]

Oops, didn't mean to mark as accepted.

[JDevlieghere]

In D65677#1614595, @sammccall wrote:

It seems conceptually a little strange to have the working directory be part of a serialized "FS", as it's fundamentally a property of a process and only transiently a property of the VFS.
I don't know this is fatal (not sure what else the RedirectingFileSystem might be used for.

What's the reason to store this as part of the VFS rather than as a separate attribute of the larger reproducer structure?

From the reproducer's perspective I would argue that it is a property of the VFS. More often than not, the working directory won't exist during replay. That needs to be modeled somewhere, and VFS is the obvious candidate. Indeed, the VFS already deals with the working directories today. You need special handling for this scenario though, because you cannot rely on the underlying (external) FS to do the right thing and might have side effects. At the same time, you might need to change the current working directory, which is fully supported with the current implementation as well. Also note that this functionality is completely optional. If no current working directory is set in the yaml mapping, things behave like they do today.

[bruno]

It seems conceptually a little strange to have the working directory be part of a serialized "FS", as it's fundamentally a property of a process and only transiently a property of the VFS.

I tend to agree with @sammccall, it's odd that something that's usually transient to be considered an intrinsic part of the described filesystem. However, I also understand that the current way things are modeled in LLVM might not help with the big picture here. I'd like to know if you tried other approaches and why they failed. Would it be reasonable to instead have a property that list something resembling "preferred search paths" for the VFS in question? Seems more natural that the VFS could have the notion of where to look at things first (which can also be a list) instead of encoding a property that is usually transient.

From the reproducer's perspective I would argue that it is a property of the VFS. More often than not, the working directory won't exist during replay. That needs to be modeled somewhere, and VFS is the obvious candidate.

What happens when besides a current-working-directory in the YAML, the user also passes -working-directory to clang? What's the expected semantics? On the LLDB side, could the reproducer be passing something like clang does with -working-directory for the purpose of having extra ways to find things? I'd imagine such mechanism to prepend some path to relative searches, which would yield a virtual absolute path to lookup at the VFS?

[labath]

I just wanted to note that the CWD is not the only thing about a filesystem that's transient -- it's contents can change over time too. This is particularly important for a long-lived process like lldb, which explicitly supports things like rebuilding an executable and then relaunching it within the same debugger session, and it also creates and deletes some files itself. So, I expect that lldb reproducers will, sooner or later, need to learn to deal with filesystems which change over time, but AFAIK the current yaml vfs thingy does not support anything like that.

Now, I am not familiar with the design principles or the implementation of the VFS classes, so I have no idea whether this speaks for or against this change, but I though it may be interesting to note that..

[sammccall]

I'd like to know if you tried other approaches and why they failed.

+1. In particular, what goes wrong if you try to make the working directory a sibling of VFS (within the reproducer container) rather than a child of it (within shared infrastructure)?

[JDevlieghere]

After some brainstorming I've identified a few other approaches that should better reflect the transience of the current working directory:

• We can modify the VFS to have a notion of search paths. The adjustPath function could iterate over the search paths until it finds an absolute path that exists. If none exist we'd return the same path we return today. This would be the most general approach.
• We could create a new virtual file system that we put on top of the RedirectingFileSystem which does something similar to what I've described above. This would require us to override every function that calls adjustPath, so it would be pretty heavyweight and rather inflexible.

I'd like to get your feedback before I start implementing these. What do you think? Is there another approach that's worth considering?

[JDevlieghere]

Support multiple working directories.

[sammccall]

In D65677#1627576, @JDevlieghere wrote:

After some brainstorming I've identified a few other approaches that should better reflect the transience of the current working directory:

• We can modify the VFS to have a notion of search paths. The adjustPath function could iterate over the search paths until it finds an absolute path that exists. If none exist we'd return the same path we return today. This would be the most general approach.
• We could create a new virtual file system that we put on top of the RedirectingFileSystem which does something similar to what I've described above. This would require us to override every function that calls adjustPath, so it would be pretty heavyweight and rather inflexible.

I'd like to get your feedback before I start implementing these. What do you think? Is there another approach that's worth considering?

I'm really sorry for missing this comment, I was out sick and missed the email.

I'd like to get your feedback before I start implementing these.

Honestly, this still seems way too complicated and this doesn't seem like a feature that needs to be part of VFS.

What do you think? Is there another approach that's worth considering?

Per my previous comment, what goes wrong if you try to make the working directory a sibling of VFS (within the reproducer container) rather than a child of it (within shared infrastructure)?

[JDevlieghere]

In D65677#1648506, @sammccall wrote:

In D65677#1627576, @JDevlieghere wrote:

After some brainstorming I've identified a few other approaches that should better reflect the transience of the current working directory:

• We can modify the VFS to have a notion of search paths. The adjustPath function could iterate over the search paths until it finds an absolute path that exists. If none exist we'd return the same path we return today. This would be the most general approach.
• We could create a new virtual file system that we put on top of the RedirectingFileSystem which does something similar to what I've described above. This would require us to override every function that calls adjustPath, so it would be pretty heavyweight and rather inflexible.

I'd like to get your feedback before I start implementing these. What do you think? Is there another approach that's worth considering?

I'm really sorry for missing this comment, I was out sick and missed the email.

I'd like to get your feedback before I start implementing these.

Honestly, this still seems way too complicated and this doesn't seem like a feature that needs to be part of VFS.

It's funny you say that, because the code to resolve relative paths is almost identical to the thing you added for supporting different working directories in different threads. :-)

What do you think? Is there another approach that's worth considering?

Per my previous comment, what goes wrong if you try to make the working directory a sibling of VFS (within the reproducer container) rather than a child of it (within shared infrastructure)?

Oops, seems like I didn't see your question either :-) Please clarify what you mean by sibling and child. Do you mean that the working directories are part of the mapping or that the redirecting file system knows about it? I don't care where we store the entries, I'm happy to have a separate YAML mapping that only the LLDB reproducers know about. However, I do need the underlying logic to be part of the (redirecting) VFS. Just like clang, LLDB is agnostic to the VFS, so this whole thing should be transparent. The only reason I didn't keep them separate is because then you need a way to tell the redirecting file system about the working directories, which means you need to get the concrete VFS, i.e. casting the VFS to a RedirectingVFS, which I try to avoid.

[sammccall]

In D65677#1649291, @JDevlieghere wrote:

In D65677#1648506, @sammccall wrote:

In D65677#1627576, @JDevlieghere wrote:

After some brainstorming I've identified a few other approaches that should better reflect the transience of the current working directory:

• We can modify the VFS to have a notion of search paths. The adjustPath function could iterate over the search paths until it finds an absolute path that exists. If none exist we'd return the same path we return today. This would be the most general approach.
• We could create a new virtual file system that we put on top of the RedirectingFileSystem which does something similar to what I've described above. This would require us to override every function that calls adjustPath, so it would be pretty heavyweight and rather inflexible.

I'd like to get your feedback before I start implementing these. What do you think? Is there another approach that's worth considering?

I'm really sorry for missing this comment, I was out sick and missed the email.

I'd like to get your feedback before I start implementing these.

Honestly, this still seems way too complicated and this doesn't seem like a feature that needs to be part of VFS.

It's funny you say that, because the code to resolve relative paths is almost identical to the thing you added for supporting different working directories in different threads. :-)

Right! I think the key distinction is that there wasn't any functional change to the APIs, because the abstraction didn't change.
(There was a second factory function, which basically lets callers choose between the two implementations that have different sets of bugs)

What do you think? Is there another approach that's worth considering?

Per my previous comment, what goes wrong if you try to make the working directory a sibling of VFS (within the reproducer container) rather than a child of it (within shared infrastructure)?

Oops, seems like I didn't see your question either :-) Please clarify what you mean by sibling and child. Do you mean that the working directories are part of the mapping or that the redirecting file system knows about it? I don't care where we store the entries, I'm happy to have a separate YAML mapping that only the LLDB reproducers know about. However, I do need the underlying logic to be part of the (redirecting) VFS. Just like clang, LLDB is agnostic to the VFS, so this whole thing should be transparent. The only reason I didn't keep them separate is because then you need a way to tell the redirecting file system about the working directories, which means you need to get the concrete VFS, i.e. casting the VFS to a RedirectingVFS, which I try to avoid.

I mean why can't you just call setCurrentWorkingDirectory before starting? something like this:

struct Reproducer { string FSYaml; string CWD; ... };
void run(Reproducer &R) {
  auto* FS = RedirectingFileSystem::create(R.FSYaml, ...);
  FS->setCurrentWorkingDirectory(R.CWD);
  runLLDBWith(FS, ...);
}

[JDevlieghere]

In D65677#1649329, @sammccall wrote:

In D65677#1649291, @JDevlieghere wrote:

It's funny you say that, because the code to resolve relative paths is almost identical to the thing you added for supporting different working directories in different threads. :-)

Right! I think the key distinction is that there wasn't any functional change to the APIs, because the abstraction didn't change.
(There was a second factory function, which basically lets callers choose between the two implementations that have different sets of bugs)

What do you think? Is there another approach that's worth considering?

Per my previous comment, what goes wrong if you try to make the working directory a sibling of VFS (within the reproducer container) rather than a child of it (within shared infrastructure)?

Oops, seems like I didn't see your question either :-) Please clarify what you mean by sibling and child. Do you mean that the working directories are part of the mapping or that the redirecting file system knows about it? I don't care where we store the entries, I'm happy to have a separate YAML mapping that only the LLDB reproducers know about. However, I do need the underlying logic to be part of the (redirecting) VFS. Just like clang, LLDB is agnostic to the VFS, so this whole thing should be transparent. The only reason I didn't keep them separate is because then you need a way to tell the redirecting file system about the working directories, which means you need to get the concrete VFS, i.e. casting the VFS to a RedirectingVFS, which I try to avoid.

I mean why can't you just call setCurrentWorkingDirectory before starting? something like this:

struct Reproducer { string FSYaml; string CWD; ... };
void run(Reproducer &R) {
  auto* FS = RedirectingFileSystem::create(R.FSYaml, ...);
  FS->setCurrentWorkingDirectory(R.CWD);
  runLLDBWith(FS, ...);
}

That doesn't work for the reproducer because the working directory likely doesn't exist during replay. The redirecting file system just forwards the setCurrentWorkingDirectory call to it's underlying (real) FS, so this becomes a no-op.

[JDevlieghere]

Ping

[sammccall]

(I replied by email, it seems phabricator doesn't pick it up.)

In D65677#1649731, @JDevlieghere wrote:

In D65677#1649329, @sammccall wrote:

In D65677#1649291, @JDevlieghere wrote:

It's funny you say that, because the code to resolve relative paths is almost identical to the thing you added for supporting different working directories in different threads. :-)

Right! I think the key distinction is that there wasn't any functional change to the APIs, because the abstraction didn't change.
(There was a second factory function, which basically lets callers choose between the two implementations that have different sets of bugs)

What do you think? Is there another approach that's worth considering?

Per my previous comment, what goes wrong if you try to make the working directory a sibling of VFS (within the reproducer container) rather than a child of it (within shared infrastructure)?

Oops, seems like I didn't see your question either :-) Please clarify what you mean by sibling and child. Do you mean that the working directories are part of the mapping or that the redirecting file system knows about it? I don't care where we store the entries, I'm happy to have a separate YAML mapping that only the LLDB reproducers know about. However, I do need the underlying logic to be part of the (redirecting) VFS. Just like clang, LLDB is agnostic to the VFS, so this whole thing should be transparent. The only reason I didn't keep them separate is because then you need a way to tell the redirecting file system about the working directories, which means you need to get the concrete VFS, i.e. casting the VFS to a RedirectingVFS, which I try to avoid.

I mean why can't you just call setCurrentWorkingDirectory before starting? something like this:

struct Reproducer { string FSYaml; string CWD; ... };
void run(Reproducer &R) {
  auto* FS = RedirectingFileSystem::create(R.FSYaml, ...);
  FS->setCurrentWorkingDirectory(R.CWD);
  runLLDBWith(FS, ...);
}

That doesn't work for the reproducer because the working directory likely doesn't exist during replay. The redirecting file system just forwards the setCurrentWorkingDirectory call to it's underlying (real) FS, so this becomes a no-op.

Can we fix that instead? The RedirectingVFS already knows about virtual directory entries, being able to cd to them makes sense.

This seems mostly like fixing a bug/limitation that wouldn't involve API changes.

Is there a part of the problem that needs the new search path concept/multiple paths?

[JDevlieghere]

Make the RedirectingFileSystem hold on to its own working directory.

[labath]

Though I am not very familiar VFS, this seems like the most intuitive solution out of everything that we had so far. But... shouldn't you also check that the directory you're chdir-ing into "exists" before you actually change the cwd? Also, the chdir operation should probably follow the usual semantics of a non-absolute chdir path being treated as relative to the previous cwd. And lastly :), what is the initial cwd value? Since previously the class shared the cwd with the underlying filesystem, one option might be to fetch the initial cwd from there (though I don't know if that is actually desired). Another option might be to just pick the first directory in the yaml file or something...

[JDevlieghere]

Thanks for the feedback, Pavel.

[vsapsai]

Adding Michael Spencer as he was doing some work for a working directory in VFS.

[labath]

Thanks Jonas. This looks fine to me, but someone who actually works on VFS (/me looks at @sammccall) will need to ack that.

[sammccall]

llvm/lib/Support/VirtualFileSystem.cpp

1052

This is a change in behavior, in that the WD of the underlying filesystem is no longer set, so e.g. in overlay mode RedirectingFileSystem::status() will look for non-overlaid relative paths in the wrong directory.

e.g. if we're using an overlay VFS with no files mapped and external WD is /foo, cd /bar; stat baz reads /bar/baz before this patch and reads /foo/baz after it. The old behavior seems more logical.

I think the right thing to do is to have setCurrentWorkingDirectory() call ExternalFS->setCurrentWorkingDirectory() if overlay mode is on. It shouldn't propagate failure, but rather record it bool ExternalFSValidWD, and only allow fallthrough operations when this flag is true.
This means cd into a dir which is invalid in the underlying FS would put it into a "broken" state where it's no longer consulted, but an (absolute) cd to a valid directory would fix it. (Variations are possible, depending on how you want to handle relative cd when the CWD is invalid in the underlying FS).

If you would rather just change the behavior here, you probably need to talk to someone familiar with the existing clients. It seems mostly used by clang's -ivfsoverlay, which was added by @benlangmuir way back when. I don't really know what it's used for.

[sammccall]

BTW, This approach does look much better to me, it fits well into the VFS abstraction and I'd argue is fixing a bug in RedirectingFileSystem.
We do need to be careful about introducing new bugs, though. (or avoid the generality and just implement the parts LLDB needs, as a separate vfs::FS)

[JDevlieghere]

Implement Sam's suggestion for changing the external file system's working directory.

[JDevlieghere]

ping

[JDevlieghere]

ping

[sammccall]

Mostly LG, just a couple of possible logic bugs.

Apologies, I was out on vacation and hoped someone else would see this.

llvm/include/llvm/Support/VirtualFileSystem.h

650

this name seems less than ideal:

• it's very similar to IsFallthrough but has different semantics
• "fallthrough" itself is not a very clear description of this functionality IMO
• it's spelled wrong per the style guide

I'd suggest shouldUseExternalFS() or so

llvm/lib/Support/VirtualFileSystem.cpp

1056

this seems backwards - error_code converts to true if it's an *error*

add tests?

1060

this seems like it should go at the top? cd to a nonexistent directory should avoid changing state and return an error (which means not marking ExternalFSValidWD as false, I think)

1064

makeAbsolute already does this check

[bruno]

This approach looks overall much better! Unless @sammccall has any extra comments, it LGTM.

[JDevlieghere]

ping @sammccall

[thakis]

This fails on Windows: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-win-fast/builds/5779/steps/test-check-llvm-unit-check-clang-unit/logs/stdio

llvm/include/llvm/Support/VirtualFileSystem.h

public:
 
     static bool classof(const Entry *E) { return E->getKind() == EK_File; }
   };
 
 private:
   friend class VFSFromYamlDirIterImpl;
   friend class RedirectingFileSystemParser;
 
+  bool shouldUseExternalFS() const {
+    return ExternalFSValidWD && IsFallthrough;
+  }
+
   /// The root(s) of the virtual file system.
   std::vector<std::unique_ptr<Entry>> Roots;
 
+  /// The current working directory of the file system.
+  std::string WorkingDirectory;
+
+  /// Whether the current working directory is valid for the external FS.
+  bool ExternalFSValidWD = false;
+
   /// 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;
 
private:
   /// win32 when using remove_dots and remove_leading_dotslash on paths.
   bool UseCanonicalizedPaths =
 #ifdef _WIN32
       false;
 #else
       true;
 #endif
 
-  RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> ExternalFS)
-      : ExternalFS(std::move(ExternalFS)) {}
+  RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> ExternalFS);
 
   /// Looks up the path <tt>[Start, End)</tt> in \p From, possibly
   /// recursing into the contents of \p From if it is a directory.
   ErrorOr<Entry *> lookupPath(llvm::sys::path::const_iterator Start,
                               llvm::sys::path::const_iterator End,
                               Entry *From) const;
 
   /// Get the status of a given an \c Entry.

llvm/lib/Support/VirtualFileSystem.cpp

 
 } // namespace vfs
 } // namespace llvm
 
 //===-----------------------------------------------------------------------===/
 // RedirectingFileSystem implementation
 //===-----------------------------------------------------------------------===/
 
+RedirectingFileSystem::RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> FS)
+    : ExternalFS(std::move(FS)) {
+  if (ExternalFS)
+    if (auto ExternalWorkingDirectory =
+            ExternalFS->getCurrentWorkingDirectory()) {
+      WorkingDirectory = *ExternalWorkingDirectory;
+      ExternalFSValidWD = true;
+    }
+}
+
 // FIXME: reuse implementation common with OverlayFSDirIterImpl as these
 // iterators are conceptually similar.
 class llvm::vfs::VFSFromYamlDirIterImpl
     : public llvm::vfs::detail::DirIterImpl {
   std::string Dir;
   RedirectingFileSystem::RedirectingDirectoryEntry::iterator Current, End;
 
   // To handle 'fallthrough' mode we need to iterate at first through
VFSFromYamlDirIterImpl(
       RedirectingFileSystem::RedirectingDirectoryEntry::iterator End,
       bool IterateExternalFS, FileSystem &ExternalFS, std::error_code &EC);
 
   std::error_code increment() override;
 };
 
 llvm::ErrorOr<std::string>
 RedirectingFileSystem::getCurrentWorkingDirectory() const {
-  return ExternalFS->getCurrentWorkingDirectory();
+  return WorkingDirectory;
 }
 
 std::error_code
 RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) {
-  return ExternalFS->setCurrentWorkingDirectory(Path);
+  // Don't change the working directory if the path doesn't exist.
+  if (!exists(Path))
+    return errc::no_such_file_or_directory;
+
+  // Always change the external FS but ignore its result.
+  if (ExternalFS) {
+    auto EC = ExternalFS->setCurrentWorkingDirectory(Path);
+    ExternalFSValidWD = !static_cast<bool>(EC);
+  }
+
+  SmallString<128> AbsolutePath;
+  Path.toVector(AbsolutePath);
+  if (std::error_code EC = makeAbsolute(AbsolutePath))
+    return EC;
+  WorkingDirectory = AbsolutePath.str();
+  return {};
 }
 
 std::error_code RedirectingFileSystem::isLocal(const Twine &Path,
                                                bool &Result) {
   return ExternalFS->isLocal(Path, Result);
 }
 
 directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
                                                     std::error_code &EC) {
   ErrorOr<RedirectingFileSystem::Entry *> E = lookupPath(Dir);
   if (!E) {
     EC = E.getError();
-    if (IsFallthrough && EC == errc::no_such_file_or_directory)
+    if (shouldUseExternalFS() && EC == errc::no_such_file_or_directory)
       return ExternalFS->dir_begin(Dir, EC);
     return {};
   }
   ErrorOr<Status> S = status(Dir, *E);
   if (!S) {
     EC = S.getError();
     return {};
   }
   if (!S->isDirectory()) {
     EC = std::error_code(static_cast<int>(errc::not_a_directory),
                          std::system_category());
     return {};
   }
 
   auto *D = cast<RedirectingFileSystem::RedirectingDirectoryEntry>(*E);
   return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(
       Dir, D->contents_begin(), D->contents_end(),
-      /*IterateExternalFS=*/IsFallthrough, *ExternalFS, EC));
+      /*IterateExternalFS=*/shouldUseExternalFS(), *ExternalFS, EC));
 }
 
 void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) {
   ExternalContentsPrefixDir = PrefixDir.str();
 }
 
 StringRef RedirectingFileSystem::getExternalContentsPrefixDir() const {
   return ExternalContentsPrefixDir;
RedirectingFileSystem::create(std::unique_ptr<MemoryBuffer> Buffer,
   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(std::move(ExternalFS)));
+      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:
if (auto *F = dyn_cast<RedirectingFileSystem::RedirectingFileEntry>(E)) {
     auto *DE = cast<RedirectingFileSystem::RedirectingDirectoryEntry>(E);
     return Status::copyWithNewName(DE->getStatus(), Path);
   }
 }
 
 ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path) {
   ErrorOr<RedirectingFileSystem::Entry *> Result = lookupPath(Path);
   if (!Result) {
-    if (IsFallthrough &&
+    if (shouldUseExternalFS() &&
         Result.getError() == llvm::errc::no_such_file_or_directory) {
       return ExternalFS->status(Path);
     }
     return Result.getError();
   }
   return status(Path, *Result);
 }
 

 };
 
 } // namespace
 
 ErrorOr<std::unique_ptr<File>>
 RedirectingFileSystem::openFileForRead(const Twine &Path) {
   ErrorOr<RedirectingFileSystem::Entry *> E = lookupPath(Path);
   if (!E) {
-    if (IsFallthrough &&
+    if (shouldUseExternalFS() &
         E.getError() == llvm::errc::no_such_file_or_directory) {
       return ExternalFS->openFileForRead(Path);
     }
     return E.getError();
   }
 
   auto *F = dyn_cast<RedirectingFileSystem::RedirectingFileEntry>(*E);
   if (!F) // FIXME: errc::not_a_file?
return std::unique_ptr<File>(
       std::make_unique<FileWithFixedStatus>(std::move(*Result), S));
 }
 
 std::error_code
 RedirectingFileSystem::getRealPath(const Twine &Path,
                                    SmallVectorImpl<char> &Output) const {
   ErrorOr<RedirectingFileSystem::Entry *> Result = lookupPath(Path);
   if (!Result) {
-    if (IsFallthrough &&
+    if (shouldUseExternalFS() &&
         Result.getError() == llvm::errc::no_such_file_or_directory) {
       return ExternalFS->getRealPath(Path, Output);
     }
     return Result.getError();
   }
 
   if (auto *F =
           dyn_cast<RedirectingFileSystem::RedirectingFileEntry>(*Result)) {
     return ExternalFS->getRealPath(F->getExternalContentsPath(), Output);
   }
   // Even if there is a directory entry, fall back to ExternalFS if allowed,
   // because directories don't have a single external contents path.
-  return IsFallthrough ? ExternalFS->getRealPath(Path, Output)
+  return shouldUseExternalFS() ? ExternalFS->getRealPath(Path, Output)
                                : llvm::errc::invalid_argument;
 }
 
 IntrusiveRefCntPtr<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,

llvm/unittests/Support/VirtualFileSystemTest.cpp

getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
     llvm_unreachable("unimplemented");
   }
   std::error_code close() override { return std::error_code(); }
 };
 
 class DummyFileSystem : public vfs::FileSystem {
   int FSID;   // used to produce UniqueIDs
   int FileID; // used to produce UniqueIDs
+  std::string WorkingDirectory;
   std::map<std::string, vfs::Status> FilesAndDirs;
+  typedef std::map<std::string, vfs::Status>::const_iterator const_iterator;
 
   static int getNextFSID() {
     static int Count = 0;
     return Count++;
   }
 
 public:
   DummyFileSystem() : FSID(getNextFSID()), FileID(0) {}
 
   ErrorOr<vfs::Status> status(const Twine &Path) override {
-    std::map<std::string, vfs::Status>::iterator I =
-        FilesAndDirs.find(Path.str());
+    auto I = findEntry(Path);
     if (I == FilesAndDirs.end())
       return make_error_code(llvm::errc::no_such_file_or_directory);
     return I->second;
   }
   ErrorOr<std::unique_ptr<vfs::File>>
   openFileForRead(const Twine &Path) override {
     auto S = status(Path);
     if (S)
       return std::unique_ptr<vfs::File>(new DummyFile{*S});
     return S.getError();
   }
   llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
-    return std::string();
+    return WorkingDirectory;
   }
   std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
+    WorkingDirectory = Path.str();
     return std::error_code();
   }
   // Map any symlink to "/symlink".
   std::error_code getRealPath(const Twine &Path,
                               SmallVectorImpl<char> &Output) const override {
-    auto I = FilesAndDirs.find(Path.str());
+    auto I = findEntry(Path);
     if (I == FilesAndDirs.end())
       return make_error_code(llvm::errc::no_such_file_or_directory);
     if (I->second.isSymlink()) {
       Output.clear();
       Twine("/symlink").toVector(Output);
       return std::error_code();
     }
     Output.clear();
vfs::directory_iterator dir_begin(const Twine &Dir,
     return vfs::directory_iterator(
         std::make_shared<DirIterImpl>(FilesAndDirs, Dir));
   }
 
   void addEntry(StringRef Path, const vfs::Status &Status) {
     FilesAndDirs[Path] = Status;
   }
 
+  const_iterator findEntry(const Twine &Path) const {
+    SmallString<128> P;
+    Path.toVector(P);
+    std::error_code EC = makeAbsolute(P);
+    assert(!EC);
+    return FilesAndDirs.find(P.str());
+  }
+
   void addRegularFile(StringRef Path, sys::fs::perms Perms = sys::fs::all_all) {
     vfs::Status S(Path, UniqueID(FSID, FileID++),
                   std::chrono::system_clock::now(), 0, 0, 1024,
                   sys::fs::file_type::regular_file, Perms);
     addEntry(Path, S);
   }
 
   void addDirectory(StringRef Path, sys::fs::perms Perms = sys::fs::all_all) {
     vfs::Status S(Path, UniqueID(FSID, FileID++),
                   std::chrono::system_clock::now(), 0, 0, 0,
                   sys::fs::file_type::directory_file, Perms);
     addEntry(Path, S);
   }
 
   void addSymlink(StringRef Path) {
     vfs::Status S(Path, UniqueID(FSID, FileID++),
                   std::chrono::system_clock::now(), 0, 0, 0,
                   sys::fs::file_type::symlink_file, sys::fs::all_all);
     addEntry(Path, S);
   }
 };
 
+class ErrorDummyFileSystem : public DummyFileSystem {
+  std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
+    return llvm::errc::no_such_file_or_directory;
+  }
+};
+
 /// Replace back-slashes by front-slashes.
 std::string getPosixPath(std::string S) {
   SmallString<128> Result;
   llvm::sys::path::native(S, Result, llvm::sys::path::Style::posix);
   return Result.str();
 }
 } // end anonymous namespace
 
TEST_F(VFSFromYAMLTest, GetRealPath) {
   // Directories should fall back to the underlying file system is possible.
   EXPECT_FALSE(FS->getRealPath("//dir/", RealPath));
   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());
+  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/',\n"
+      "  'contents': [ {\n"
+      "                  'type': 'file',\n"
+      "                  'name': 'bar/a',\n"
+      "                  'external-contents': '//root/foo/a'\n"
+      "                }\n"
+      "              ]\n"
+      "}\n"
+      "]\n"
+      "}",
+      Lower);
+  ASSERT_TRUE(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");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->isStatusKnown());
+  EXPECT_FALSE(Status->isDirectory());
+  EXPECT_TRUE(Status->isRegularFile());
+  EXPECT_FALSE(Status->isSymlink());
+  EXPECT_FALSE(Status->isOther());
+  EXPECT_TRUE(Status->exists());
+
+  EC = FS->setCurrentWorkingDirectory("bogus");
+  ASSERT_TRUE(EC);
+  WorkingDir = FS->getCurrentWorkingDirectory();
+  ASSERT_TRUE(WorkingDir);
+  EXPECT_EQ(*WorkingDir, "//root/bar/");
+
+  EC = FS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+  WorkingDir = FS->getCurrentWorkingDirectory();
+  ASSERT_TRUE(WorkingDir);
+  EXPECT_EQ(*WorkingDir, "//root/");
+
+  EC = FS->setCurrentWorkingDirectory("bar/");
+  ASSERT_FALSE(EC);
+  WorkingDir = FS->getCurrentWorkingDirectory();
+  ASSERT_TRUE(WorkingDir);
+  EXPECT_EQ(*WorkingDir, "//root/bar/");
+}
+
+TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthrough) {
+  IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
+  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/',\n"
+      "  'contents': [ {\n"
+      "                  'type': 'file',\n"
+      "                  'name': 'bar/a',\n"
+      "                  'external-contents': '//root/foo/a'\n"
+      "                }\n"
+      "              ]\n"
+      "}\n"
+      "]\n"
+      "}",
+      Lower);
+  ASSERT_TRUE(FS.get() != nullptr);
+  std::error_code EC = FS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+  ASSERT_TRUE(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());
+
+  EC = FS->setCurrentWorkingDirectory("//root/bar/");
+  ASSERT_FALSE(EC);
+
+  Status = FS->status("./a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+
+  Status = FS->status("./b");
+  ASSERT_TRUE(Status.getError());
+
+  Status = FS->status("./c");
+  ASSERT_TRUE(Status.getError());
+
+  EC = FS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+
+  Status = FS->status("c");
+  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/',\n"
+      "  'contents': [ {\n"
+      "                  'type': 'file',\n"
+      "                  'name': 'bar/a',\n"
+      "                  'external-contents': '//root/foo/a'\n"
+      "                }\n"
+      "              ]\n"
+      "}\n"
+      "]\n"
+      "}",
+      Lower);
+  ASSERT_TRUE(FS.get() != nullptr);
+  std::error_code EC = FS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+  ASSERT_TRUE(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_TRUE(Status.getError());
+}