浏览代码

[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
Juergen Ributzka 8 年之前
父节点
当前提交
6cd8b3d4eb
共有 3 个文件被更改,包括 86 次插入6 次删除
  1. 1 1
      include/clang/Basic/VirtualFileSystem.h
  2. 2 5
      lib/Basic/VirtualFileSystem.cpp
  3. 83 0
      unittests/Basic/VirtualFileSystemTest.cpp

+ 1 - 1
include/clang/Basic/VirtualFileSystem.h

@@ -161,7 +161,7 @@ public:
   directory_iterator &increment(std::error_code &EC) {
   directory_iterator &increment(std::error_code &EC) {
     assert(Impl && "attempting to increment past end");
     assert(Impl && "attempting to increment past end");
     EC = Impl->increment();
     EC = Impl->increment();
-    if (EC || !Impl->CurrentEntry.isStatusKnown())
+    if (!Impl->CurrentEntry.isStatusKnown())
       Impl.reset(); // Normalize the end iterator to Impl == nullptr.
       Impl.reset(); // Normalize the end iterator to Impl == nullptr.
     return *this;
     return *this;
   }
   }

+ 2 - 5
lib/Basic/VirtualFileSystem.cpp

@@ -246,8 +246,7 @@ public:
     if (!EC && Iter != llvm::sys::fs::directory_iterator()) {
     if (!EC && Iter != llvm::sys::fs::directory_iterator()) {
       llvm::sys::fs::file_status S;
       llvm::sys::fs::file_status S;
       EC = Iter->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)
                                                            std::error_code &EC)
     : FS(&FS_) {
     : FS(&FS_) {
   directory_iterator I = FS->dir_begin(Path, EC);
   directory_iterator I = FS->dir_begin(Path, EC);
-  if (!EC && I != directory_iterator()) {
+  if (I != directory_iterator()) {
     State = std::make_shared<IterState>();
     State = std::make_shared<IterState>();
     State->push(I);
     State->push(I);
   }
   }
@@ -1871,8 +1870,6 @@ recursive_directory_iterator::increment(std::error_code &EC) {
   vfs::directory_iterator End;
   vfs::directory_iterator End;
   if (State->top()->isDirectory()) {
   if (State->top()->isDirectory()) {
     vfs::directory_iterator I = FS->dir_begin(State->top()->getName(), EC);
     vfs::directory_iterator I = FS->dir_begin(State->top()->getName(), EC);
-    if (EC)
-      return *this;
     if (I != End) {
     if (I != End) {
       State->push(I);
       State->push(I);
       return *this;
       return *this;

+ 83 - 0
unittests/Basic/VirtualFileSystemTest.cpp

@@ -305,6 +305,22 @@ struct ScopedDir {
   }
   }
   operator StringRef() { return Path.str(); }
   operator StringRef() { return Path.str(); }
 };
 };
+
+struct ScopedLink {
+  SmallString<128> Path;
+  ScopedLink(const Twine &To, const Twine &From) {
+    Path = From.str();
+    std::error_code EC = sys::fs::create_link(To, From);
+    if (EC)
+      Path = "";
+    EXPECT_FALSE(EC);
+  }
+  ~ScopedLink() {
+    if (Path != "")
+      EXPECT_FALSE(llvm::sys::fs::remove(Path.str()));
+  }
+  operator StringRef() { return Path.str(); }
+};
 } // end anonymous namespace
 } // end anonymous namespace
 
 
 TEST(VirtualFileSystemTest, BasicRealFSIteration) {
 TEST(VirtualFileSystemTest, BasicRealFSIteration) {
@@ -334,6 +350,35 @@ TEST(VirtualFileSystemTest, BasicRealFSIteration) {
   EXPECT_EQ(vfs::directory_iterator(), I);
   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) {
 TEST(VirtualFileSystemTest, BasicRealFSRecursiveIteration) {
   ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/true);
   ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/true);
   IntrusiveRefCntPtr<vfs::FileSystem> FS = vfs::getRealFileSystem();
   IntrusiveRefCntPtr<vfs::FileSystem> FS = vfs::getRealFileSystem();
@@ -373,6 +418,44 @@ TEST(VirtualFileSystemTest, BasicRealFSRecursiveIteration) {
   EXPECT_EQ(1, Counts[3]); // d
   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>
 template <typename DirIter>
 static void checkContents(DirIter I, ArrayRef<StringRef> ExpectedOut) {
 static void checkContents(DirIter I, ArrayRef<StringRef> ExpectedOut) {
   std::error_code EC;
   std::error_code EC;