[llvm][vfs] Implement in-memory symlinks

[jansvoboda11]

Summary

This patch implements symlinks for the in-memory VFS. Original author: @erik.pilkington.

Depends on D117648 & D117649.

[dexonsmith]

@sammccall, you've had objections in the past to exposing symbolic links in the vfs::FileSystem API, but this patch doesn't do that. It's just modelling symlink behaviour in an InMemoryFileSystem. Any concerns?

(I have none.)

llvm/lib/Support/VirtualFileSystem.cpp

597

Add a trailing , to avoid a diff if this gets extended again?

[sammccall]

In D117650#3256232, @dexonsmith wrote:

@sammccall, you've had objections in the past to exposing symbolic links in the vfs::FileSystem API, but this patch doesn't do that. It's just modelling symlink behaviour in an InMemoryFileSystem. Any concerns?

No, this seems fine to me. And implementation seems solid :-)

(IIRC my previous concerns weren't about VFS->VFS symlinks, but rather ability to "resolve" a VFS file to a non-virtual file in order to open it through native APIs. It's possible I misunderstood that proposal of course)

In principle I don't see a problem with exposing symlinks in the VFS layer either, but I guess it'd be really complicated to get right. (e.g. ensure handling symlinks externally vs within the VFS matches, conceptual problems with overlay filesystems, etc)

llvm/include/llvm/Support/VirtualFileSystem.h

565

FWIW I find From/to confusing, because I had to rewire my brain to think of ln as a variant of cp in order to remember which order the args go in :-)

I think NewLink/Target would be much clearer, but there's value in being consistent with the existing addHardLink too, so up to you.

llvm/lib/Support/VirtualFileSystem.cpp

930

FollowFinalSymlink?
It's wordy, but it's not clear to me what the behavior is if you look up "link/link/file" with FollowSymlinks=false. Also the plural is a bit misleading.

It might be worth adding a comment here with the most important contract: if Follow is true, then it will never return a symlink node.

1013

This fails if ToNode is a symbolic link. Linux allows hardlinks to symlinks, apparently macOS resolves the symlink and hardlinks to the target, posix allows either (implementation-defined) but doesn't allow it to just fail.

This isn't posix so you can do whatever you like, but I think the choice deserves a comment.

1083

This doesn't look correct to me, if the symlink points to a directory we'll claim it's a file and then operations on it will fail.

Since the VFS interface doesn't know about symlinks, it's probably best to resolve the symlink (chain) here and report the target. But returning symlink_file might be ok...

If you resolve and it dangles, you have to choose between reporting regular_file, symlink_file, or hiding the entry entirely... not really sure which is best.

[sammccall]

In D117650#3262164, @sammccall wrote:

(IIRC my previous concerns weren't about VFS->VFS symlinks, but rather ability to "resolve" a VFS file to a non-virtual file in order to open it through native APIs. It's possible I misunderstood that proposal of course)

Oops, I just remembered about D97288, which is probably what you meant. Yeah, this change is much less expensive because it doesn't affect (other) VFS implementers nor consumers.

[dexonsmith]

In D117650#3262196, @sammccall wrote:

In D117650#3262164, @sammccall wrote:

(IIRC my previous concerns weren't about VFS->VFS symlinks, but rather ability to "resolve" a VFS file to a non-virtual file in order to open it through native APIs. It's possible I misunderstood that proposal of course)

Oops, I just remembered about D97288, which is probably what you meant. Yeah, this change is much less expensive because it doesn't affect (other) VFS implementers nor consumers.

Yes, I think that's what I was remembering!

[jansvoboda11]

llvm/include/llvm/Support/VirtualFileSystem.h

565

You're right, I struggle with that too. I changed that to NewLink/Target for both the new addSymbolicLink() and the existing addHardLink(). Let me know if you'd prefer to extract the change to addHardLink() into a prep-commit. (I assume review wouldn't be necessary.)

llvm/lib/Support/VirtualFileSystem.cpp

930

Fair enough, changed.

1013

Good catch! I chose to follow symlinks to be consistent with macOS for now.

1083

I chose to resolve the symlink to keep the VFS interface symlink-free. In case of dangling symlink, I think it would make sense to report type_unknown, WDYT?

BTW, since I chose to resolve the symlink and directory_entry needs the target path, I was forced to tweak the lookupInMemoryNode to return the resolved target path as well, since with symlinks it will differ from the requested path.

[jansvoboda11]

Follow symlinks when creating hardlinks and iterating over directory entries.
Return the resolved name from InMemoryFileSystem::lookupNode(), update some
documentation.

[jansvoboda11]

llvm/lib/Support/VirtualFileSystem.cpp

1133

We're passing a lambda here since lookupNode is (and should remain) private to InMemoryFileSystem and InMemoryDirIterator is in anonymous namespace so it can't be befriended. An alternative to this solution would be to pull InMemoryDirIterator into the detail namespace and forward-declare it in the friend declaration inside InMemoryFileSystem. I don't have strong feelings either way.

[sammccall]

LG, thanks for the changes!
I wouldn't bother splitting out the commits for something this size.

llvm/lib/Support/VirtualFileSystem.cpp

1133

Yeah :-( I find friending it slightly cleaner, but don't feel strongly, feel free to leave it.

An alternative very similar to friending: forward-declare it as a nested class of InMemoryFileSystem (which makes it implicitly a friend, but also InMemoryFileSystem::DirIterator seems a slightly more obvious name than detail::InMemoryDirIterator.

llvm/include/llvm/Support/VirtualFileSystem.h

 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
 #include <cassert>
 #include <cstdint>
 #include <ctime>
 #include <memory>
 #include <stack>
struct NewInMemoryNodeInfo {
   uint32_t User;
   uint32_t Group;
   llvm::sys::fs::file_type Type;
   llvm::sys::fs::perms Perms;
 
   Status makeStatus() const;
 };
 
+class NamedNodeOrError {
+  ErrorOr<std::pair<llvm::SmallString<128>, const detail::InMemoryNode *>>
+      Value;
+
+public:
+  NamedNodeOrError(llvm::SmallString<128> Name,
+                   const detail::InMemoryNode *Node)
+      : Value(std::make_pair(Name, Node)) {}
+  NamedNodeOrError(std::error_code EC) : Value(EC) {}
+  NamedNodeOrError(llvm::errc EC) : Value(EC) {}
+
+  StringRef getName() const { return (*Value).first; }
+  explicit operator bool() const { return static_cast<bool>(Value); }
+  operator std::error_code() const { return Value.getError(); }
+  std::error_code getError() const { return Value.getError(); }
+  const detail::InMemoryNode *operator*() const { return (*Value).second; }
+};
+
 } // namespace detail
 
 /// An in-memory file system.
 class InMemoryFileSystem : public FileSystem {
   std::unique_ptr<detail::InMemoryDirectory> Root;
   std::string WorkingDirectory;
   bool UseNormalizedPaths = true;
 
   using MakeNodeFn = llvm::function_ref<std::unique_ptr<detail::InMemoryNode>(
       detail::NewInMemoryNodeInfo)>;
 
   /// Create node with \p MakeNode and add it into this filesystem at \p Path.
   bool addFile(const Twine &Path, time_t ModificationTime,
                std::unique_ptr<llvm::MemoryBuffer> Buffer,
                Optional<uint32_t> User, Optional<uint32_t> Group,
                Optional<llvm::sys::fs::file_type> Type,
                Optional<llvm::sys::fs::perms> Perms, MakeNodeFn MakeNode);
 
-  ErrorOr<const detail::InMemoryNode *> lookupNode(const Twine &P) const;
+  /// Looks up the in-memory node for the path \param P.
+  /// If \param FollowFinalSymlink is true, the returned node is guaranteed to
+  /// not be a symlink and its path may differ from \param P.
+  detail::NamedNodeOrError lookupNode(const Twine &P, bool FollowFinalSymlink,
+                                      size_t SymlinkDepth = 0) const;
 
   class DirIterator;
 
 public:
   explicit InMemoryFileSystem(bool UseNormalizedPaths = true);
   ~InMemoryFileSystem() override;
 
   /// Add a file containing a buffer or a directory to the VFS with a
public:
   /// The \param Target path must be an existing file or a hardlink. The
   /// \param NewLink file must not have been added before. The \param Target
   /// path must not be a directory. The \param NewLink node is added as a hard
   /// link which points to the resolved file of \param Target node.
   /// \return true if the above condition is satisfied and hardlink was
   /// successfully created, false otherwise.
   bool addHardLink(const Twine &NewLink, const Twine &Target);
 
+  /// Arbitrary max depth to search through symlinks. We can get into problems
+  /// if a link links to a link that links back to the link, for example.
+  static constexpr size_t MaxSymlinkDepth = 16;
+
+  /// Add a symbolic link. Unlike a HardLink, because \param Target doesn't need
+  /// to refer to a file (or refer to anything, as it happens). Also, an
+  /// in-memory directory for \param Target isn't automatically created.
+  bool addSymbolicLink(const Twine &NewLink, const Twine &Target,
+                       time_t ModificationTime, Optional<uint32_t> User = None,
+                       Optional<uint32_t> Group = None,
+                       Optional<llvm::sys::fs::perms> Perms = None);
+
   /// Add a buffer to the VFS with a path. The VFS does not own the buffer.
   /// If present, User, Group, Type and Perms apply to the newly-created file
   /// or directory.
   /// \return true if the file or directory was successfully added,
   /// false if the file or directory already exists in the file system with
   /// different contents.
   bool addFileNoOwn(const Twine &Path, time_t ModificationTime,
                     const llvm::MemoryBufferRef &Buffer,

llvm/lib/Support/VirtualFileSystem.cpp

 
 void ProxyFileSystem::anchor() {}
 
 namespace llvm {
 namespace vfs {
 
 namespace detail {
 
-enum InMemoryNodeKind { IME_File, IME_Directory, IME_HardLink };
+enum InMemoryNodeKind {
+  IME_File,
+  IME_Directory,
+  IME_HardLink,
+  IME_SymbolicLink,
+};
 
 /// The in memory file system is a tree of Nodes. Every node can either be a
-/// file , hardlink or a directory.
+/// file, symlink, hardlink or a directory.
 class InMemoryNode {
   InMemoryNodeKind Kind;
   std::string FileName;
 
 public:
   InMemoryNode(llvm::StringRef FileName, InMemoryNodeKind Kind)
       : Kind(Kind), FileName(std::string(llvm::sys::path::filename(FileName))) {
   }
return std::string(Indent, ' ') + "HardLink to -> " +
            ResolvedFile.toString(0);
   }
 
   static bool classof(const InMemoryNode *N) {
     return N->getKind() == IME_HardLink;
   }
 };
 
+class InMemorySymbolicLink : public InMemoryNode {
+  std::string TargetPath;
+  Status Stat;
+
+public:
+  InMemorySymbolicLink(StringRef Path, StringRef TargetPath, Status Stat)
+      : InMemoryNode(Path, IME_SymbolicLink), TargetPath(std::move(TargetPath)),
+        Stat(Stat) {}
+
+  std::string toString(unsigned Indent) const override {
+    return std::string(Indent, ' ') + "SymbolicLink to -> " + TargetPath;
+  }
+
+  Status getStatus(const Twine &RequestedName) const override {
+    return Status::copyWithNewName(Stat, RequestedName);
+  }
+
+  StringRef getTargetPath() const { return TargetPath; }
+
+  static bool classof(const InMemoryNode *N) {
+    return N->getKind() == IME_SymbolicLink;
+  }
+};
+
 /// Adapt a InMemoryFile for VFS' File interface.  The goal is to make
 /// \p InMemoryFileAdaptor mimic as much as possible the behavior of
 /// \p RealFile.
 class InMemoryFileAdaptor : public File {
   const InMemoryFile &Node;
   /// The name to use when returning a Status for this file.
   std::string RequestedName;
 
return addFile(P, ModificationTime, llvm::MemoryBuffer::getMemBuffer(Buffer),
                    Status Stat = NNI.makeStatus();
                    if (Stat.getType() == sys::fs::file_type::directory_file)
                      return std::make_unique<detail::InMemoryDirectory>(Stat);
                    return std::make_unique<detail::InMemoryFile>(
                        Stat, std::move(NNI.Buffer));
                  });
 }
 
-ErrorOr<const detail::InMemoryNode *>
-InMemoryFileSystem::lookupNode(const Twine &P) const {
+detail::NamedNodeOrError
+InMemoryFileSystem::lookupNode(const Twine &P, bool FollowFinalSymlink,
+                               size_t SymlinkDepth) const {
   SmallString<128> Path;
   P.toVector(Path);
 
   // Fix up relative paths. This just prepends the current working directory.
   std::error_code EC = makeAbsolute(Path);
   assert(!EC);
   (void)EC;
 
   if (useNormalizedPaths())
     llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
 
   const detail::InMemoryDirectory *Dir = Root.get();
   if (Path.empty())
-    return Dir;
+    return detail::NamedNodeOrError(Path, Dir);
 
   auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);
   while (true) {
     detail::InMemoryNode *Node = Dir->getChild(*I);
     ++I;
     if (!Node)
       return errc::no_such_file_or_directory;
 
+    if (auto Symlink = dyn_cast<detail::InMemorySymbolicLink>(Node)) {
+      // If we're at the end of the path, and we're not following through
+      // terminal symlinks, then we're done.
+      if (I == E && !FollowFinalSymlink)
+        return detail::NamedNodeOrError(Path, Symlink);
+
+      if (SymlinkDepth > InMemoryFileSystem::MaxSymlinkDepth)
+        return errc::no_such_file_or_directory;
+
+      SmallString<128> TargetPath = Symlink->getTargetPath();
+      if (std::error_code EC = makeAbsolute(TargetPath))
+        return EC;
+
+      // Keep going with the target. We always want to follow symlinks here
+      // because we're either at the end of a path that we want to follow, or
+      // not at the end of a path, in which case we need to follow the symlink
+      // regardless.
+      auto Target =
+          lookupNode(TargetPath, /*FollowFinalSymlink=*/true, SymlinkDepth + 1);
+      if (!Target || I == E)
+        return Target;
+
+      if (!isa<detail::InMemoryDirectory>(*Target))
+        return errc::no_such_file_or_directory;
+
+      // Otherwise, continue on the search in the symlinked directory.
+      Dir = cast<detail::InMemoryDirectory>(*Target);
+      continue;
+    }
+
     // Return the file if it's at the end of the path.
     if (auto File = dyn_cast<detail::InMemoryFile>(Node)) {
       if (I == E)
-        return File;
+        return detail::NamedNodeOrError(Path, File);
       return errc::no_such_file_or_directory;
     }
 
     // If Node is HardLink then return the resolved file.
     if (auto File = dyn_cast<detail::InMemoryHardLink>(Node)) {
       if (I == E)
-        return &File->getResolvedFile();
+        return detail::NamedNodeOrError(Path, &File->getResolvedFile());
       return errc::no_such_file_or_directory;
     }
     // Traverse directories.
     Dir = cast<detail::InMemoryDirectory>(Node);
     if (I == E)
-      return Dir;
+      return detail::NamedNodeOrError(Path, Dir);
   }
 }
 
 bool InMemoryFileSystem::addHardLink(const Twine &NewLink,
                                      const Twine &Target) {
-  auto NewLinkNode = lookupNode(NewLink);
-  auto TargetNode = lookupNode(Target);
+  auto NewLinkNode = lookupNode(NewLink, /*FollowFinalSymlink=*/false);
+  // Whether symlinks in the hardlink target are followed is
+  // implementation-defined in POSIX.
+  // We're following symlinks here to be consistent with macOS.
+  auto TargetNode = lookupNode(Target, /*FollowFinalSymlink=*/true);
   // FromPath must not have been added before. ToPath must have been added
   // before. Resolved ToPath must be a File.
   if (!TargetNode || NewLinkNode || !isa<detail::InMemoryFile>(*TargetNode))
     return false;
   return addFile(NewLink, 0, nullptr, None, None, None, None,
                  [&](detail::NewInMemoryNodeInfo NNI) {
                    return std::make_unique<detail::InMemoryHardLink>(
                        NNI.Path.str(),
                        *cast<detail::InMemoryFile>(*TargetNode));
                  });
 }
 
+bool InMemoryFileSystem::addSymbolicLink(const Twine &NewLink,
+                                         const Twine &Target,
+                                         time_t ModificationTime,
+                                         Optional<uint32_t> User,
+                                         Optional<uint32_t> Group,
+                                         Optional<llvm::sys::fs::perms> Perms) {
+  auto NewLinkNode = lookupNode(NewLink, /*FollowFinalSymlink=*/false);
+  if (NewLinkNode)
+    return false;
+
+  SmallString<128> NewLinkStr, TargetStr;
+  NewLink.toVector(NewLinkStr);
+  Target.toVector(TargetStr);
+
+  return addFile(NewLinkStr, ModificationTime, nullptr, User, Group,
+                 sys::fs::file_type::symlink_file, Perms,
+                 [&](detail::NewInMemoryNodeInfo NNI) {
+                   return std::make_unique<detail::InMemorySymbolicLink>(
+                       NewLinkStr, TargetStr, NNI.makeStatus());
+                 });
+}
+
 llvm::ErrorOr<Status> InMemoryFileSystem::status(const Twine &Path) {
-  auto Node = lookupNode(Path);
+  auto Node = lookupNode(Path, /*FollowFinalSymlink=*/true);
   if (Node)
     return (*Node)->getStatus(Path);
   return Node.getError();
 }
 
 llvm::ErrorOr<std::unique_ptr<File>>
 InMemoryFileSystem::openFileForRead(const Twine &Path) {
-  auto Node = lookupNode(Path);
+  auto Node = lookupNode(Path,/*FollowFinalSymlink=*/true);
   if (!Node)
     return Node.getError();
 
   // When we have a file provide a heap-allocated wrapper for the memory buffer
   // to match the ownership semantics for File.
   if (auto *F = dyn_cast<detail::InMemoryFile>(*Node))
     return std::unique_ptr<File>(
         new detail::InMemoryFileAdaptor(*F, Path.str()));
 
   // FIXME: errc::not_a_file?
   return make_error_code(llvm::errc::invalid_argument);
 }
 
 /// Adaptor from InMemoryDir::iterator to directory_iterator.
 class InMemoryFileSystem::DirIterator : public llvm::vfs::detail::DirIterImpl {
+  const InMemoryFileSystem *FS;
   detail::InMemoryDirectory::const_iterator I;
   detail::InMemoryDirectory::const_iterator E;
   std::string RequestedDirName;
 
   void setCurrentEntry() {
     if (I != E) {
       SmallString<256> Path(RequestedDirName);
       llvm::sys::path::append(Path, I->second->getFileName());
       sys::fs::file_type Type = sys::fs::file_type::type_unknown;
       switch (I->second->getKind()) {
       case detail::IME_File:
       case detail::IME_HardLink:
         Type = sys::fs::file_type::regular_file;
         break;
       case detail::IME_Directory:
         Type = sys::fs::file_type::directory_file;
         break;
+      case detail::IME_SymbolicLink:
+        if (auto SymlinkTarget =
+                FS->lookupNode(Path, /*FollowFinalSymlink=*/true)) {
+          Path = SymlinkTarget.getName();
+          Type = (*SymlinkTarget)->getStatus(Path).getType();
+        }
+        break;
       }
       CurrentEntry = directory_entry(std::string(Path.str()), Type);
     } else {
       // When we're at the end, make CurrentEntry invalid and DirIterImpl will
       // do the rest.
       CurrentEntry = directory_entry();
     }
   }
 
 public:
   DirIterator() = default;
 
-  explicit DirIterator(const detail::InMemoryDirectory &Dir,
-                       std::string RequestedDirName)
-      : I(Dir.begin()), E(Dir.end()),
+  DirIterator(const InMemoryFileSystem *FS,
+              const detail::InMemoryDirectory &Dir,
+      : FS(FS), I(Dir.begin()), E(Dir.end()),
         RequestedDirName(std::move(RequestedDirName)) {
     setCurrentEntry();
   }
 
   std::error_code increment() override {
     ++I;
     setCurrentEntry();
     return {};
   }
 };
 
 directory_iterator InMemoryFileSystem::dir_begin(const Twine &Dir,
                                                  std::error_code &EC) {
-  auto Node = lookupNode(Dir);
+  auto Node = lookupNode(Dir, /*FollowFinalSymlink=*/true);
   if (!Node) {
     EC = Node.getError();
     return directory_iterator(std::make_shared<DirIterator>());
   }
 
   if (auto *DirNode = dyn_cast<detail::InMemoryDirectory>(*Node))
     return directory_iterator(
-        std::make_shared<DirIterator>(*DirNode, Dir.str()));
+        std::make_shared<DirIterator>(this, *DirNode, Dir.str()));
 
   EC = make_error_code(llvm::errc::not_a_directory);
   return directory_iterator(std::make_shared<DirIterator>());
 }
 
 std::error_code InMemoryFileSystem::setCurrentWorkingDirectory(const Twine &P) {
   SmallString<128> Path;
   P.toVector(Path);
 

llvm/unittests/Support/VirtualFileSystemTest.cpp

TEST_F(InMemoryFileSystemTest, AddHardLinkToADirectory) {
   StringRef Dir = "path/to/dummy/dir";
   StringRef Link = "/path/to/link";
   StringRef File = "path/to/dummy/dir/target";
   StringRef Content = "content of target";
   EXPECT_TRUE(FS.addFile(File, 0, MemoryBuffer::getMemBuffer(Content)));
   EXPECT_FALSE(FS.addHardLink(Link, Dir));
 }
 
+TEST_F(InMemoryFileSystemTest, AddHardLinkToASymlink) {
+  EXPECT_TRUE(FS.addFile("/file", 0, MemoryBuffer::getMemBuffer("content")));
+  EXPECT_TRUE(FS.addSymbolicLink("/symlink", "/file", 0));
+  EXPECT_TRUE(FS.addHardLink("/hardlink", "/symlink"));
+  EXPECT_EQ((*FS.getBufferForFile("/hardlink"))->getBuffer(), "content");
+}
+
 TEST_F(InMemoryFileSystemTest, AddHardLinkFromADirectory) {
   StringRef Dir = "path/to/dummy/dir";
   StringRef Target = "path/to/dummy/dir/target";
   StringRef Content = "content of target";
   EXPECT_TRUE(FS.addFile(Target, 0, MemoryBuffer::getMemBuffer(Content)));
   EXPECT_FALSE(FS.addHardLink(Dir, Target));
 }
 
TEST_F(InMemoryFileSystemTest, UniqueID) {
   // Note: FS2 should match FS with respect to path normalization.
   vfs::InMemoryFileSystem FS2(/*UseNormalizedPath=*/false);
   ASSERT_TRUE(FS2.addFile("/a/b", 0, MemoryBuffer::getMemBuffer("text")));
   EXPECT_EQ(FS.status("/a/b")->getUniqueID(),
             FS2.status("/a/b")->getUniqueID());
   EXPECT_EQ(FS.status("/a")->getUniqueID(), FS2.status("/a")->getUniqueID());
 }
 
+TEST_F(InMemoryFileSystemTest, AddSymlinkToAFile) {
+  EXPECT_TRUE(
+      FS.addFile("/some/file", 0, MemoryBuffer::getMemBuffer("contents")));
+  EXPECT_TRUE(FS.addSymbolicLink("/other/file/link", "/some/file", 0));
+  ErrorOr<vfs::Status> Stat = FS.status("/some/file");
+  EXPECT_TRUE(Stat->isRegularFile());
+}
+
+TEST_F(InMemoryFileSystemTest, AddSymlinkToADirectory) {
+  EXPECT_TRUE(FS.addSymbolicLink("/link", "/target", 0));
+  EXPECT_TRUE(
+      FS.addFile("/target/foo.h", 0, MemoryBuffer::getMemBuffer("foo")));
+  ErrorOr<vfs::Status> Stat = FS.status("/link/foo.h");
+  EXPECT_TRUE(Stat);
+  EXPECT_EQ((*Stat).getName(), "/link/foo.h");
+  EXPECT_TRUE(Stat->isRegularFile());
+}
+
+TEST_F(InMemoryFileSystemTest, AddSymlinkToASymlink) {
+  EXPECT_TRUE(FS.addSymbolicLink("/first", "/second", 0));
+  EXPECT_TRUE(FS.addSymbolicLink("/second", "/third", 0));
+  EXPECT_TRUE(FS.addFile("/third", 0, MemoryBuffer::getMemBuffer("")));
+  ErrorOr<vfs::Status> Stat = FS.status("/first");
+  EXPECT_TRUE(Stat);
+  EXPECT_EQ((*Stat).getName(), "/first");
+  // Follow-through symlinks by default. This matches RealFileSystem's
+  // semantics.
+  EXPECT_TRUE(Stat->isRegularFile());
+  Stat = FS.status("/second");
+  EXPECT_TRUE(Stat);
+  EXPECT_EQ((*Stat).getName(), "/second");
+  EXPECT_TRUE(Stat->isRegularFile());
+  Stat = FS.status("/third");
+  EXPECT_TRUE(Stat);
+  EXPECT_EQ((*Stat).getName(), "/third");
+  EXPECT_TRUE(Stat->isRegularFile());
+}
+
+TEST_F(InMemoryFileSystemTest, AddRecursiveSymlink) {
+  EXPECT_TRUE(FS.addSymbolicLink("/link-a", "/link-b", 0));
+  EXPECT_TRUE(FS.addSymbolicLink("/link-b", "/link-a", 0));
+  ErrorOr<vfs::Status> Stat = FS.status("/link-a/foo");
+  EXPECT_FALSE(Stat);
+  EXPECT_EQ(Stat.getError(), errc::no_such_file_or_directory);
+}
+
+TEST_F(InMemoryFileSystemTest, DirectoryIteratorWithSymlinkToAFile) {
+  std::error_code EC;
+
+  EXPECT_TRUE(FS.addFile("/file", 0, MemoryBuffer::getMemBuffer("")));
+  EXPECT_TRUE(FS.addSymbolicLink("/symlink", "/file", 0));
+
+  vfs::directory_iterator I = FS.dir_begin("/", EC), E;
+  ASSERT_FALSE(EC);
+
+  std::vector<std::string> Nodes;
+  for (; !EC && I != E; I.increment(EC))
+    Nodes.push_back(getPosixPath(std::string(I->path())));
+
+  EXPECT_THAT(Nodes, testing::UnorderedElementsAre("/file", "/file"));
+}
+
+TEST_F(InMemoryFileSystemTest, RecursiveDirectoryIteratorWithSymlinkToADir) {
+  std::error_code EC;
+
+  EXPECT_TRUE(FS.addFile("/dir/file", 0, MemoryBuffer::getMemBuffer("")));
+  EXPECT_TRUE(FS.addSymbolicLink("/dir_symlink", "/dir", 0));
+
+  vfs::recursive_directory_iterator I(FS, "/", EC), E;
+  ASSERT_FALSE(EC);
+
+  std::vector<std::string> Nodes;
+  for (; !EC && I != E; I.increment(EC))
+    Nodes.push_back(getPosixPath(std::string(I->path())));
+
+  EXPECT_THAT(Nodes, testing::UnorderedElementsAre("/dir", "/dir/file", "/dir",
+                                                   "/dir/file"));
+}
+
 // NOTE: in the tests below, we use '//root/' as our root directory, since it is
 // a legal *absolute* path on Windows as well as *nix.
 class VFSFromYAMLTest : public ::testing::Test {
 public:
   int NumDiagnostics;
 
   void SetUp() override { NumDiagnostics = 0; }