[VFS] Ignore broken symlinks in the directory iterator.

The VFS directory iterator and recursive directory iterator behave differently
from the LLVM counterparts. Once the VFS iterators hit a broken symlink they
immediately abort. The LLVM counterparts allow to recover from this issue by
clearing the error code and skipping to the next entry.

This change adds the same functionality to the VFS iterators. There should be
no change in current behavior in the current CLANG source base, because all
clients have loop exit conditions that also check the error code.

This fixes rdar://problem/30934619.

Differential Revision: https://reviews.llvm.org/D30768

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@297510 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/VirtualFileSystem.h b/include/clang/Basic/VirtualFileSystem.h
index 39dab6cbf045898cacde635ec10647395d652911..e52b345e286c694e283f1a52a563d87281ce9574 100644 (file)
--- a/include/clang/Basic/VirtualFileSystem.h
+++ b/include/clang/Basic/VirtualFileSystem.h
@@ -161,7 +161,7 @@ public:
   directory_iterator &increment(std::error_code &EC) {
     assert(Impl && "attempting to increment past end");
     EC = Impl->increment();
-    if (EC || !Impl->CurrentEntry.isStatusKnown())
+    if (!Impl->CurrentEntry.isStatusKnown())
       Impl.reset(); // Normalize the end iterator to Impl == nullptr.
     return *this;
   }

diff --git a/lib/Basic/VirtualFileSystem.cpp b/lib/Basic/VirtualFileSystem.cpp
index 9e13bf7cd5434dcc05c95cae9ba97ebbb1cee8ff..63348724b1ca9a8c3ac3aed92cd49820cce0d610 100644 (file)
--- a/lib/Basic/VirtualFileSystem.cpp
+++ b/lib/Basic/VirtualFileSystem.cpp
@@ -246,8 +246,7 @@ public:
     if (!EC && Iter != llvm::sys::fs::directory_iterator()) {
       llvm::sys::fs::file_status S;
       EC = Iter->status(S);
-      if (!EC)
-        CurrentEntry = Status::copyWithNewName(S, Iter->path());
+      CurrentEntry = Status::copyWithNewName(S, Iter->path());
     }
   }
 
@@ -1858,7 +1857,7 @@ vfs::recursive_directory_iterator::recursive_directory_iterator(FileSystem &FS_,
                                                            std::error_code &EC)
     : FS(&FS_) {
   directory_iterator I = FS->dir_begin(Path, EC);
-  if (!EC && I != directory_iterator()) {
+  if (I != directory_iterator()) {
     State = std::make_shared<IterState>();
     State->push(I);
   }
@@ -1871,8 +1870,6 @@ recursive_directory_iterator::increment(std::error_code &EC) {
   vfs::directory_iterator End;
   if (State->top()->isDirectory()) {
     vfs::directory_iterator I = FS->dir_begin(State->top()->getName(), EC);
-    if (EC)
-      return *this;
     if (I != End) {
       State->push(I);
       return *this;

diff --git a/unittests/Basic/VirtualFileSystemTest.cpp b/unittests/Basic/VirtualFileSystemTest.cpp
index 580343d93ea1f0d6e02268679cf97b0aa1c3b36d..738e25b2b023cb643126acf78adba9f6758f77a3 100644 (file)
--- a/unittests/Basic/VirtualFileSystemTest.cpp
+++ b/unittests/Basic/VirtualFileSystemTest.cpp
@@ -305,6 +305,22 @@ struct ScopedDir {
   }
   operator StringRef() { return Path.str(); }
 };
+
+struct ScopedLink {
+  SmallString<128> Path;
+  ScopedLink(const Twine &To, const Twine &From) {
+    Path = From.str();
+    std::error_code EC = sys::fs::create_link(To, From);
+    if (EC)
+      Path = "";
+    EXPECT_FALSE(EC);
+  }
+  ~ScopedLink() {
+    if (Path != "")
+      EXPECT_FALSE(llvm::sys::fs::remove(Path.str()));
+  }
+  operator StringRef() { return Path.str(); }
+};
 } // end anonymous namespace
 
 TEST(VirtualFileSystemTest, BasicRealFSIteration) {
@@ -334,6 +350,35 @@ TEST(VirtualFileSystemTest, BasicRealFSIteration) {
   EXPECT_EQ(vfs::directory_iterator(), I);
 }
 
+TEST(VirtualFileSystemTest, BrokenSymlinkRealFSIteration) {
+  ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  IntrusiveRefCntPtr<vfs::FileSystem> FS = vfs::getRealFileSystem();
+
+  ScopedLink _a("no_such_file", TestDirectory + "/a");
+  ScopedDir _b(TestDirectory + "/b");
+  ScopedLink _c("no_such_file", TestDirectory + "/c");
+
+  std::error_code EC;
+  vfs::directory_iterator I = FS->dir_begin(Twine(TestDirectory), EC);
+  EXPECT_TRUE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EC = std::error_code();
+  EXPECT_TRUE(I->getName() == _a);
+  I.increment(EC);
+  EXPECT_FALSE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EXPECT_TRUE(I->getName() == _b);
+  I.increment(EC);
+  EXPECT_TRUE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EC = std::error_code();
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EXPECT_TRUE(I->getName() == _c);
+  I.increment(EC);
+  EXPECT_FALSE(EC);
+  EXPECT_EQ(vfs::directory_iterator(), I);
+}
+
 TEST(VirtualFileSystemTest, BasicRealFSRecursiveIteration) {
   ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/true);
   IntrusiveRefCntPtr<vfs::FileSystem> FS = vfs::getRealFileSystem();
@@ -373,6 +418,44 @@ TEST(VirtualFileSystemTest, BasicRealFSRecursiveIteration) {
   EXPECT_EQ(1, Counts[3]); // d
 }
 
+TEST(VirtualFileSystemTest, BrokenSymlinkRealFSRecursiveIteration) {
+  ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  IntrusiveRefCntPtr<vfs::FileSystem> FS = vfs::getRealFileSystem();
+
+  ScopedLink _a("no_such_file", TestDirectory + "/a");
+  ScopedDir _b(TestDirectory + "/b");
+  ScopedLink _ba("no_such_file", TestDirectory + "/b/a");
+  ScopedDir _bb(TestDirectory + "/b/b");
+  ScopedLink _bc("no_such_file", TestDirectory + "/b/c");
+  ScopedLink _c("no_such_file", TestDirectory + "/c");
+  ScopedDir _d(TestDirectory + "/d");
+  ScopedDir _dd(TestDirectory + "/d/d");
+  ScopedDir _ddd(TestDirectory + "/d/d/d");
+  ScopedLink _e("no_such_file", TestDirectory + "/e");
+
+  std::vector<std::string> Contents;
+  std::error_code EC;
+  for (vfs::recursive_directory_iterator I(*FS, Twine(TestDirectory), EC), E;
+       I != E; I.increment(EC)) {
+    // Skip broken symlinks.
+    if (EC == std::errc::no_such_file_or_directory) {
+      EC = std::error_code();
+      continue;
+    } else if (EC) {
+      break;
+    }
+    Contents.push_back(I->getName());
+  }
+
+  // Check contents.
+  EXPECT_EQ(5U, Contents.size());
+  EXPECT_TRUE(Contents[0] == _b);
+  EXPECT_TRUE(Contents[1] == _bb);
+  EXPECT_TRUE(Contents[2] == _d);
+  EXPECT_TRUE(Contents[3] == _dd);
+  EXPECT_TRUE(Contents[4] == _ddd);
+}
+
 template <typename DirIter>
 static void checkContents(DirIter I, ArrayRef<StringRef> ExpectedOut) {
   std::error_code EC;