From fd958fca0b4c2244463dd3e7bd688c8da9af2554 Mon Sep 17 00:00:00 2001 From: Juergen Ributzka Date: Fri, 10 Mar 2017 21:23:27 +0000 Subject: [PATCH] [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 llvm-svn: 297510 --- clang/include/clang/Basic/VirtualFileSystem.h | 2 +- clang/lib/Basic/VirtualFileSystem.cpp | 7 +- .../unittests/Basic/VirtualFileSystemTest.cpp | 83 +++++++++++++++++++ 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Basic/VirtualFileSystem.h b/clang/include/clang/Basic/VirtualFileSystem.h index 39dab6cbf045..e52b345e286c 100644 --- a/clang/include/clang/Basic/VirtualFileSystem.h +++ b/clang/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/clang/lib/Basic/VirtualFileSystem.cpp b/clang/lib/Basic/VirtualFileSystem.cpp index 9e13bf7cd543..63348724b1ca 100644 --- a/clang/lib/Basic/VirtualFileSystem.cpp +++ b/clang/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(); 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/clang/unittests/Basic/VirtualFileSystemTest.cpp b/clang/unittests/Basic/VirtualFileSystemTest.cpp index 580343d93ea1..738e25b2b023 100644 --- a/clang/unittests/Basic/VirtualFileSystemTest.cpp +++ b/clang/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 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 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 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 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 static void checkContents(DirIter I, ArrayRef ExpectedOut) { std::error_code EC;