From: Juergen Ributzka Date: Tue, 14 Mar 2017 00:14:40 +0000 (+0000) Subject: Reapply [VFS] Ignore broken symlinks in the directory iterator. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5aeef7a4e0a4ca73eccdaef2119166c1f33083f3;p=clang Reapply [VFS] Ignore broken symlinks in the directory iterator. Modified the tests to accept any iteration order, to run only on Unix, and added additional error reporting to investigate SystemZ bot issue. 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 don't stat entries unless they have to descend into the next directory, which allows to recover from this issue by clearing the error code and skipping to the next entry. This change adds similar behavior 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@297693 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/VirtualFileSystem.h b/include/clang/Basic/VirtualFileSystem.h index 39dab6cbf0..e52b345e28 100644 --- 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 43cd9f17ce..f5db717866 100644 --- a/lib/Basic/VirtualFileSystem.cpp +++ b/lib/Basic/VirtualFileSystem.cpp @@ -244,8 +244,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()); } } @@ -1856,7 +1855,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); } @@ -1869,8 +1868,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 580343d93e..92e3663f92 100644 --- 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,36 @@ TEST(VirtualFileSystemTest, BasicRealFSIteration) { EXPECT_EQ(vfs::directory_iterator(), I); } +#ifdef LLVM_ON_UNIX +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; + for (vfs::directory_iterator I = FS->dir_begin(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; + } + // For bot debugging. + if (EC) { + outs() << "std::errc::no_such_file_or_directory: " + << (int)std::errc::no_such_file_or_directory << "\n"; + outs() << "EC: " << EC.value() << "\n"; + outs() << "EC message: " << EC.message() << "\n"; + } + ASSERT_FALSE(EC); + EXPECT_TRUE(I->getName() == _b); + } +} +#endif + TEST(VirtualFileSystemTest, BasicRealFSRecursiveIteration) { ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/true); IntrusiveRefCntPtr FS = vfs::getRealFileSystem(); @@ -373,6 +419,50 @@ TEST(VirtualFileSystemTest, BasicRealFSRecursiveIteration) { EXPECT_EQ(1, Counts[3]); // d } +#ifdef LLVM_ON_UNIX +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 Expected = {_b, _bb, _d, _dd, _ddd}; + + 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; + } + // For bot debugging. + if (EC) { + outs() << "std::errc::no_such_file_or_directory: " + << (int)std::errc::no_such_file_or_directory << "\n"; + outs() << "EC: " << EC.value() << "\n"; + outs() << "EC message: " << EC.message() << "\n"; + } + ASSERT_FALSE(EC); + Contents.push_back(I->getName()); + } + + // Check sorted contents. + std::sort(Contents.begin(), Contents.end()); + EXPECT_EQ(Expected.size(), Contents.size()); + EXPECT_TRUE(std::equal(Contents.begin(), Contents.end(), Expected.begin())); +} +#endif + template static void checkContents(DirIter I, ArrayRef ExpectedOut) { std::error_code EC;