[libcxx] r316939 - Fix PR35078 - recursive directory iterator's increment method throws incorrectly.
Eric Fiselier via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 30 11:43:21 PDT 2017
Author: ericwf
Date: Mon Oct 30 11:43:21 2017
New Revision: 316939
URL: http://llvm.org/viewvc/llvm-project?rev=316939&view=rev
Log:
Fix PR35078 - recursive directory iterator's increment method throws incorrectly.
The guts of the increment method for recursive_directory_iterator
was failing to pass an error code object to calls to status/symlink_status,
which can throw under certain conditions.
This patch fixes the issues by correctly propagating the error codes.
However the noexcept still needs to be removed from the signature, as
mentioned in LWG 3014, but that change will be made in a separate commit.
Modified:
libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp
libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
Modified: libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp?rev=316939&r1=316938&r2=316939&view=diff
==============================================================================
--- libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp Mon Oct 30 11:43:21 2017
@@ -296,24 +296,43 @@ void recursive_directory_iterator::__adv
}
bool recursive_directory_iterator::__try_recursion(error_code *ec) {
-
bool rec_sym =
bool(options() & directory_options::follow_directory_symlink);
+
auto& curr_it = __imp_->__stack_.top();
- if (is_directory(curr_it.__entry_.status()) &&
- (!is_symlink(curr_it.__entry_.symlink_status()) || rec_sym))
- {
- std::error_code m_ec;
+ bool skip_rec = false;
+ std::error_code m_ec;
+ if (!rec_sym) {
+ file_status st = curr_it.__entry_.symlink_status(m_ec);
+ if (m_ec && status_known(st))
+ m_ec.clear();
+ if (m_ec || is_symlink(st) || !is_directory(st))
+ skip_rec = true;
+ } else {
+ file_status st = curr_it.__entry_.status(m_ec);
+ if (m_ec && status_known(st))
+ m_ec.clear();
+ if (m_ec || !is_directory(st))
+ skip_rec = true;
+ }
+
+ if (!skip_rec) {
__dir_stream new_it(curr_it.__entry_.path(), __imp_->__options_, m_ec);
if (new_it.good()) {
__imp_->__stack_.push(_VSTD::move(new_it));
return true;
}
- if (m_ec) {
- __imp_.reset();
- set_or_throw(m_ec, ec,
- "recursive_directory_iterator::operator++()");
+ }
+ if (m_ec) {
+ const bool allow_eacess = bool(__imp_->__options_
+ & directory_options::skip_permission_denied);
+ if (m_ec.value() == EACCES && allow_eacess) {
+ if (ec) ec->clear();
+ } else {
+ __imp_.reset();
+ set_or_throw(m_ec, ec,
+ "recursive_directory_iterator::operator++()");
}
}
return false;
Modified: libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp?rev=316939&r1=316938&r2=316939&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp Mon Oct 30 11:43:21 2017
@@ -237,4 +237,250 @@ TEST_CASE(access_denied_on_recursion_tes
}
}
+// See llvm.org/PR35078
+TEST_CASE(test_PR35078)
+{
+ using namespace std::experimental::filesystem;
+ scoped_test_env env;
+ const path testFiles[] = {
+ env.create_dir("dir1"),
+ env.create_dir("dir1/dir2"),
+ env.create_dir("dir1/dir2/dir3"),
+ env.create_file("dir1/file1"),
+ env.create_file("dir1/dir2/dir3/file2")
+ };
+ const path startDir = testFiles[0];
+ const path permDeniedDir = testFiles[1];
+ const path nestedDir = testFiles[2];
+ const path nestedFile = testFiles[3];
+
+ // Change the permissions so we can no longer iterate
+ permissions(permDeniedDir,
+ perms::remove_perms|perms::group_exec
+ |perms::owner_exec|perms::others_exec);
+
+ const std::error_code eacess_ec =
+ std::make_error_code(std::errc::permission_denied);
+ std::error_code ec = GetTestEC();
+
+ const recursive_directory_iterator endIt;
+
+ auto SetupState = [&](bool AllowEAccess, bool& SeenFile3) {
+ SeenFile3 = false;
+ auto Opts = AllowEAccess ? directory_options::skip_permission_denied
+ : directory_options::none;
+ recursive_directory_iterator it(startDir, Opts, ec);
+ while (!ec && it != endIt && *it != nestedDir) {
+ if (*it == nestedFile)
+ SeenFile3 = true;
+ it.increment(ec);
+ }
+ return it;
+ };
+
+ {
+ bool SeenNestedFile = false;
+ recursive_directory_iterator it = SetupState(false, SeenNestedFile);
+ TEST_REQUIRE(it != endIt);
+ TEST_REQUIRE(*it == nestedDir);
+ ec = GetTestEC();
+ it.increment(ec);
+ TEST_CHECK(ec);
+ TEST_CHECK(ec == eacess_ec);
+ TEST_CHECK(it == endIt);
+ }
+ {
+ bool SeenNestedFile = false;
+ recursive_directory_iterator it = SetupState(true, SeenNestedFile);
+ TEST_REQUIRE(it != endIt);
+ TEST_REQUIRE(*it == nestedDir);
+ ec = GetTestEC();
+ it.increment(ec);
+ TEST_CHECK(!ec);
+ if (SeenNestedFile) {
+ TEST_CHECK(it == endIt);
+ } else {
+ TEST_REQUIRE(it != endIt);
+ TEST_CHECK(*it == nestedFile);
+ }
+ }
+}
+
+
+// See llvm.org/PR35078
+TEST_CASE(test_PR35078_with_symlink)
+{
+ using namespace std::experimental::filesystem;
+ scoped_test_env env;
+ const path testFiles[] = {
+ env.create_dir("dir1"),
+ env.create_file("dir1/file1"),
+ env.create_dir("sym_dir"),
+ env.create_dir("sym_dir/nested_sym_dir"),
+ env.create_symlink("sym_dir/nested_sym_dir", "dir1/dir2"),
+ env.create_dir("sym_dir/dir1"),
+ env.create_dir("sym_dir/dir1/dir2"),
+
+ };
+ // const unsigned TestFilesSize = sizeof(testFiles) / sizeof(testFiles[0]);
+ const path startDir = testFiles[0];
+ const path nestedFile = testFiles[1];
+ const path permDeniedDir = testFiles[2];
+ const path symDir = testFiles[4];
+
+ // Change the permissions so we can no longer iterate
+ permissions(permDeniedDir,
+ perms::remove_perms|perms::group_exec
+ |perms::owner_exec|perms::others_exec);
+
+ const std::error_code eacess_ec =
+ std::make_error_code(std::errc::permission_denied);
+ std::error_code ec = GetTestEC();
+
+ const recursive_directory_iterator endIt;
+
+ auto SetupState = [&](bool AllowEAccess, bool FollowSym, bool& SeenFile3) {
+ SeenFile3 = false;
+ auto Opts = AllowEAccess ? directory_options::skip_permission_denied
+ : directory_options::none;
+ if (FollowSym)
+ Opts |= directory_options::follow_directory_symlink;
+ recursive_directory_iterator it(startDir, Opts, ec);
+ while (!ec && it != endIt && *it != symDir) {
+ std::cout << *it << std::endl;
+ if (*it == nestedFile)
+ SeenFile3 = true;
+ it.increment(ec);
+ }
+ return it;
+ };
+
+ struct {
+ bool SkipPermDenied;
+ bool FollowSymlinks;
+ bool ExpectSuccess;
+ } TestCases[] = {
+ // Passing cases
+ {false, false, true}, {true, true, true}, {true, false, true},
+ // Failing cases
+ {false, true, false}
+ };
+ for (auto TC : TestCases) {
+ bool SeenNestedFile = false;
+ recursive_directory_iterator it = SetupState(TC.SkipPermDenied,
+ TC.FollowSymlinks,
+ SeenNestedFile);
+ TEST_REQUIRE(!ec);
+ TEST_REQUIRE(it != endIt);
+ TEST_REQUIRE(*it == symDir);
+ ec = GetTestEC();
+ it.increment(ec);
+ if (TC.ExpectSuccess) {
+ TEST_CHECK(!ec);
+ if (SeenNestedFile) {
+ TEST_CHECK(it == endIt);
+ } else {
+ TEST_REQUIRE(it != endIt);
+ TEST_CHECK(*it == nestedFile);
+ }
+ } else {
+ TEST_CHECK(ec);
+ TEST_CHECK(ec == eacess_ec);
+ TEST_CHECK(it == endIt);
+ }
+ }
+}
+
+
+// See llvm.org/PR35078
+TEST_CASE(test_PR35078_with_symlink_file)
+{
+ using namespace std::experimental::filesystem;
+ scoped_test_env env;
+ const path testFiles[] = {
+ env.create_dir("dir1"),
+ env.create_dir("dir1/dir2"),
+ env.create_file("dir1/file2"),
+ env.create_dir("sym_dir"),
+ env.create_dir("sym_dir/sdir1"),
+ env.create_file("sym_dir/sdir1/sfile1"),
+ env.create_symlink("sym_dir/sdir1/sfile1", "dir1/dir2/file1")
+ };
+ const unsigned TestFilesSize = sizeof(testFiles) / sizeof(testFiles[0]);
+ const path startDir = testFiles[0];
+ const path nestedDir = testFiles[1];
+ const path nestedFile = testFiles[2];
+ const path permDeniedDir = testFiles[3];
+ const path symFile = testFiles[TestFilesSize - 1];
+
+ // Change the permissions so we can no longer iterate
+ permissions(permDeniedDir,
+ perms::remove_perms|perms::group_exec
+ |perms::owner_exec|perms::others_exec);
+
+ const std::error_code eacess_ec =
+ std::make_error_code(std::errc::permission_denied);
+ std::error_code ec = GetTestEC();
+
+ const recursive_directory_iterator EndIt;
+
+ auto SetupState = [&](bool AllowEAccess, bool FollowSym, bool& SeenNestedFile) {
+ SeenNestedFile = false;
+ auto Opts = AllowEAccess ? directory_options::skip_permission_denied
+ : directory_options::none;
+ if (FollowSym)
+ Opts |= directory_options::follow_directory_symlink;
+ recursive_directory_iterator it(startDir, Opts, ec);
+ while (!ec && it != EndIt && *it != nestedDir) {
+ if (*it == nestedFile)
+ SeenNestedFile = true;
+ it.increment(ec);
+ }
+ return it;
+ };
+
+ struct {
+ bool SkipPermDenied;
+ bool FollowSymlinks;
+ bool ExpectSuccess;
+ } TestCases[] = {
+ // Passing cases
+ {false, false, true}, {true, true, true}, {true, false, true},
+ // Failing cases
+ {false, true, false}
+ };
+ for (auto TC : TestCases){
+ bool SeenNestedFile = false;
+ recursive_directory_iterator it = SetupState(TC.SkipPermDenied,
+ TC.FollowSymlinks,
+ SeenNestedFile);
+ TEST_REQUIRE(!ec);
+ TEST_REQUIRE(it != EndIt);
+ TEST_REQUIRE(*it == nestedDir);
+ ec = GetTestEC();
+ it.increment(ec);
+ TEST_REQUIRE(it != EndIt);
+ TEST_CHECK(!ec);
+ TEST_CHECK(*it == symFile);
+ ec = GetTestEC();
+ it.increment(ec);
+ if (TC.ExpectSuccess) {
+ if (!SeenNestedFile) {
+ TEST_CHECK(!ec);
+ TEST_REQUIRE(it != EndIt);
+ TEST_CHECK(*it == nestedFile);
+ ec = GetTestEC();
+ it.increment(ec);
+ }
+ TEST_CHECK(!ec);
+ TEST_CHECK(it == EndIt);
+ } else {
+ TEST_CHECK(ec);
+ TEST_CHECK(ec == eacess_ec);
+ TEST_CHECK(it == EndIt);
+ }
+ }
+}
+
+
TEST_SUITE_END()
More information about the cfe-commits
mailing list