r297693 - Reapply [VFS] Ignore broken symlinks in the directory iterator.
Juergen Ributzka via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 13 17:14:40 PDT 2017
Author: ributzka
Date: Mon Mar 13 19:14:40 2017
New Revision: 297693
URL: http://llvm.org/viewvc/llvm-project?rev=297693&view=rev
Log:
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
Modified:
cfe/trunk/include/clang/Basic/VirtualFileSystem.h
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=297693&r1=297692&r2=297693&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Mon Mar 13 19:14:40 2017
@@ -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;
}
Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=297693&r1=297692&r2=297693&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Mon Mar 13 19:14:40 2017
@@ -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::recur
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);
}
@@ -1869,8 +1868,6 @@ recursive_directory_iterator::increment(
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;
Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=297693&r1=297692&r2=297693&view=diff
==============================================================================
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Mon Mar 13 19:14:40 2017
@@ -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, BasicRealFSI
EXPECT_EQ(vfs::directory_iterator(), I);
}
+#ifdef LLVM_ON_UNIX
+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;
+ 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<vfs::FileSystem> FS = vfs::getRealFileSystem();
@@ -373,6 +419,50 @@ TEST(VirtualFileSystemTest, BasicRealFSR
EXPECT_EQ(1, Counts[3]); // d
}
+#ifdef LLVM_ON_UNIX
+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<StringRef> Expected = {_b, _bb, _d, _dd, _ddd};
+
+ 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;
+ }
+ // 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 <typename DirIter>
static void checkContents(DirIter I, ArrayRef<StringRef> ExpectedOut) {
std::error_code EC;
More information about the cfe-commits
mailing list