libstdc++: Fix filesystem::remove_all for Windows [PR104161]

The recursive_directory_iterator::__erase member was failing for
Windows, because the entry._M_type value is always file_type::none
(because _Dir_base::advance doesn't populate it for Windows) and
top.unlink uses fs::remove which sets an error using the
system_category. That meant that ec.value() was a Windows error code and
not an errno value, so the comparisons to EPERM and EISDIR failed.
Instead of depending on a specific Windows error code for attempting to
remove a directory, just use directory_entry::refresh() to query the
type first. This doesn't avoid the TOCTTOU races with directory
symlinks, but we can't avoid them on Windows without openat and
unlinkat, and creating symlinks requires admin privs on Windows anyway.

This also fixes the fs::remove_all(const path&) overload, which was
supposed to use the same logic as the other overload, but I forgot to
change it before my previous commit.

libstdc++-v3/ChangeLog:

	PR libstdc++/104161
	* src/c++17/fs_dir.cc (fs::recursive_directory_iterator::__erase):
	[i_GLIBCXX_FILESYSTEM_IS_WINDOWS]: Refresh entry._M_type member,
	instead of checking for errno values indicating a directory.
	* src/c++17/fs_ops.cc (fs::remove_all(const path&)): Use similar
	logic to non-throwing overload.
	(fs::remove_all(const path&, error_code&)): Add comments.
	* src/filesystem/ops-common.h: Likewise.

(cherry picked from commit 5750952bec1e632d1f804f4a1bed2f74c0f3b189)

diff --git a/libstdc++-v3/src/c++17/fs_dir.cc b/libstdc++-v3/src/c++17/fs_dir.cc
index 936914a349c5a978cc382d2203dc2774a6b4ccdb..42042751276831b5a0f7e8d63301d798b75faa78 100644 (file)
--- a/libstdc++-v3/src/c++17/fs_dir.cc
+++ b/libstdc++-v3/src/c++17/fs_dir.cc
@@ -476,6 +476,16 @@ fs::recursive_directory_iterator::__erase(error_code* ecptr)
     {
       auto& top = _M_dirs->top();
 
+#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
+      // _Dir::unlink uses fs::remove which uses std::system_category() for
+      // Windows errror codes, so we can't just check for EPERM and EISDIR.
+      // Use directory_entry::refresh() here to check if we have a directory.
+      // This can be a TOCTTOU race, but we don't have openat or unlinkat to
+      // solve that on Windows, and generally don't support symlinks anyway.
+      if (top.entry._M_type == file_type::none)
+       top.entry.refresh();
+#endif
+
       if (top.entry._M_type == file_type::directory)
        {
          _Dir dir = top.open_subdir(skip_permission_denied, nofollow, ec);
@@ -498,12 +508,13 @@ fs::recursive_directory_iterator::__erase(error_code* ecptr)
        }
       else if (top.unlink(ec))
        break; // Success
+#if ! _GLIBCXX_FILESYSTEM_IS_WINDOWS
       else if (top.entry._M_type == file_type::none)
        {
          // We did not have a cached type, so it's possible that top.entry
          // is actually a directory, and that's why the unlink above failed.
 #ifdef EPERM
-         // POSIX.1-2017 says unlinking a directory returns EPERM,
+         // POSIX.1-2017 says unlink on a directory returns EPERM,
          // but LSB allows EISDIR too. Some targets don't even define EPERM.
          if (ec.value() == EPERM || ec.value() == EISDIR)
 #else
@@ -516,6 +527,7 @@ fs::recursive_directory_iterator::__erase(error_code* ecptr)
              continue;
            }
        }
+#endif
     }
 
   if (!ec)

diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index 683c8f14fed93612207548586d31f9ae79f732d2..84737453ada714272c7168adcaf3db72a34b1a1c 100644 (file)
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -1284,21 +1284,38 @@ fs::remove(const path& p, error_code& ec) noexcept
 std::uintmax_t
 fs::remove_all(const path& p)
 {
+  error_code ec;
   uintmax_t count = 0;
-  auto st = filesystem::status(p);
-  if (!exists(st))
-    return 0;
-  if (is_directory(st))
+  recursive_directory_iterator dir(p, directory_options{64|128}, ec);
+  switch (ec.value()) // N.B. assumes ec.category() == std::generic_category()
+  {
+  case 0:
+    // Iterate over the directory removing everything.
     {
-      recursive_directory_iterator dir(p, directory_options{64|128}), end;
-      path failed;
+      const recursive_directory_iterator end;
       while (dir != end)
        {
-         failed = dir->path();
-         dir.__erase();
+         dir.__erase(); // throws on error
          ++count;
        }
     }
+    // Directory is empty now, will remove it below.
+    break;
+#ifndef __AVR__
+  case ENOENT:
+    // Our work here is done.
+    return 0;
+  case ENOTDIR:
+  case ELOOP:
+    // Not a directory, will remove below.
+    break;
+#endif
+  default:
+    // An error occurred.
+    _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove all", p, ec));
+  }
+
+  // Remove p itself, which is either a non-directory or is now empty.
   return count + fs::remove(p);
 }
 
@@ -1307,11 +1324,12 @@ fs::remove_all(const path& p, error_code& ec)
 {
   uintmax_t count = 0;
   recursive_directory_iterator dir(p, directory_options{64|128}, ec);
-  switch (ec.value())
+  switch (ec.value()) // N.B. assumes ec.category() == std::generic_category()
   {
   case 0:
+    // Iterate over the directory removing everything.
     {
-      recursive_directory_iterator end;
+      const recursive_directory_iterator end;
       while (dir != end)
        {
          dir.__erase(&ec);
@@ -1320,6 +1338,7 @@ fs::remove_all(const path& p, error_code& ec)
          ++count;
        }
     }
+    // Directory is empty now, will remove it below.
     break;
 #ifndef __AVR__
   case ENOENT:
@@ -1335,6 +1354,7 @@ fs::remove_all(const path& p, error_code& ec)
     // An error occurred.
     return -1;
   }
+
   // Remove p itself, which is either a non-directory or is now empty.
   if (int last = fs::remove(p, ec); !ec)
     return count + last;

diff --git a/libstdc++-v3/src/filesystem/ops-common.h b/libstdc++-v3/src/filesystem/ops-common.h
index 087c3fb655e68d0cb029bf73b216b1e6f7c2d834..ac17cb5fcd571a2263ccabce83d04aef2970da28 100644 (file)
--- a/libstdc++-v3/src/filesystem/ops-common.h
+++ b/libstdc++-v3/src/filesystem/ops-common.h
@@ -62,6 +62,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __last_system_error() noexcept
   {
 #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+    // N.B. use error_code::default_error_condition() to convert to generic.
     return {(int)::GetLastError(), std::system_category()};
 #else
     return {errno, std::generic_category()};