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.

llvm-svn: 316939

diff --git a/libcxx/src/experimental/filesystem/directory_iterator.cpp b/libcxx/src/experimental/filesystem/directory_iterator.cpp
index 2513585..a552fdc 100644 (file)
--- a/libcxx/src/experimental/filesystem/directory_iterator.cpp
+++ b/libcxx/src/experimental/filesystem/directory_iterator.cpp
@@ -296,24 +296,43 @@ void recursive_directory_iterator::__advance(error_code* ec) {
 }
 
 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;

diff --git a/libcxx/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp b/libcxx/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
index 26fc3ca..e91bd50 100644 (file)
--- a/libcxx/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
+++ b/libcxx/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
@@ -237,4 +237,250 @@ TEST_CASE(access_denied_on_recursion_test_case)
     }
 }
 
+// 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()