diff mbox series

[1/2] libstdc++: Enable debug assertions for filesystem directory iterators

Message ID 20241031200620.1615582-1-jwakely@redhat.com
State Committed
Commit f7979b8bfa6542e6861f44c78d18cc1cf8dae4d6
Headers
Series [1/2] libstdc++: Enable debug assertions for filesystem directory iterators |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed

Commit Message

Jonathan Wakely Oct. 31, 2024, 7:59 p.m. UTC 
  Several member functions of filesystem::directory_iterator and
filesystem::recursive_directory_iterator currently dereference their
shared_ptr data member without checking for non-null. Because they use
operator-> and that function only uses _GLIBCXX_DEBUG_PEDASSERT rather
than __glibcxx_assert there is no assertion even when the library is
built with _GLIBCXX_ASSERTIONS defined. This means that dereferencing
invalid directory iterators gives an unhelpful segfault.

By using (*p). instead of p-> we get an assertion when the library is
built with _GLIBCXX_ASSERTIONS, with a "_M_get() != nullptr" message.

libstdc++-v3/ChangeLog:

	* src/c++17/fs_dir.cc (fs::directory_iterator::operator*): Use
	shared_ptr::operator* instead of shared_ptr::operator->.
	(fs::recursive_directory_iterator::options): Likewise.
	(fs::recursive_directory_iterator::depth): Likewise.
	(fs::recursive_directory_iterator::recursion_pending): Likewise.
	(fs::recursive_directory_iterator::operator*): Likewise.
	(fs::recursive_directory_iterator::disable_recursion_pending):
	Likewise.
---
Tested x86_64-linux.

Also available for review at:
https://forge.sourceware.org/gcc/gcc-TEST/pulls/13


 libstdc++-v3/src/c++17/fs_dir.cc | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Jonathan Wakely Nov. 6, 2024, 12:49 p.m. UTC | #1 
Pushed

On Thu, 31 Oct 2024 at 20:06, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> Several member functions of filesystem::directory_iterator and
> filesystem::recursive_directory_iterator currently dereference their
> shared_ptr data member without checking for non-null. Because they use
> operator-> and that function only uses _GLIBCXX_DEBUG_PEDASSERT rather
> than __glibcxx_assert there is no assertion even when the library is
> built with _GLIBCXX_ASSERTIONS defined. This means that dereferencing
> invalid directory iterators gives an unhelpful segfault.
>
> By using (*p). instead of p-> we get an assertion when the library is
> built with _GLIBCXX_ASSERTIONS, with a "_M_get() != nullptr" message.
>
> libstdc++-v3/ChangeLog:
>
>         * src/c++17/fs_dir.cc (fs::directory_iterator::operator*): Use
>         shared_ptr::operator* instead of shared_ptr::operator->.
>         (fs::recursive_directory_iterator::options): Likewise.
>         (fs::recursive_directory_iterator::depth): Likewise.
>         (fs::recursive_directory_iterator::recursion_pending): Likewise.
>         (fs::recursive_directory_iterator::operator*): Likewise.
>         (fs::recursive_directory_iterator::disable_recursion_pending):
>         Likewise.
> ---
> Tested x86_64-linux.
>
> Also available for review at:
> https://forge.sourceware.org/gcc/gcc-TEST/pulls/13
>
>
>  libstdc++-v3/src/c++17/fs_dir.cc | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/libstdc++-v3/src/c++17/fs_dir.cc b/libstdc++-v3/src/c++17/fs_dir.cc
> index 28d27f6a9fa..8fe9e5e4cc8 100644
> --- a/libstdc++-v3/src/c++17/fs_dir.cc
> +++ b/libstdc++-v3/src/c++17/fs_dir.cc
> @@ -230,7 +230,7 @@ directory_iterator(const path& p, directory_options options, error_code* ecptr)
>  const fs::directory_entry&
>  fs::directory_iterator::operator*() const noexcept
>  {
> -  return _M_dir->entry;
> +  return (*_M_dir).entry;
>  }
>
>  fs::directory_iterator&
> @@ -327,25 +327,25 @@ fs::recursive_directory_iterator::~recursive_directory_iterator() = default;
>  fs::directory_options
>  fs::recursive_directory_iterator::options() const noexcept
>  {
> -  return _M_dirs->options;
> +  return (*_M_dirs).options;
>  }
>
>  int
>  fs::recursive_directory_iterator::depth() const noexcept
>  {
> -  return int(_M_dirs->size()) - 1;
> +  return int((*_M_dirs).size()) - 1;
>  }
>
>  bool
>  fs::recursive_directory_iterator::recursion_pending() const noexcept
>  {
> -  return _M_dirs->pending;
> +  return (*_M_dirs).pending;
>  }
>
>  const fs::directory_entry&
>  fs::recursive_directory_iterator::operator*() const noexcept
>  {
> -  return _M_dirs->top().entry;
> +  return (*_M_dirs).top().entry;
>  }
>
>  fs::recursive_directory_iterator&
> @@ -453,7 +453,7 @@ fs::recursive_directory_iterator::pop()
>  void
>  fs::recursive_directory_iterator::disable_recursion_pending() noexcept
>  {
> -  _M_dirs->pending = false;
> +  (*_M_dirs).pending = false;
>  }
>
>  // Used to implement filesystem::remove_all.
> --
> 2.47.0
>
diff mbox series

Patch

diff --git a/libstdc++-v3/src/c++17/fs_dir.cc b/libstdc++-v3/src/c++17/fs_dir.cc
index 28d27f6a9fa..8fe9e5e4cc8 100644
--- a/libstdc++-v3/src/c++17/fs_dir.cc
+++ b/libstdc++-v3/src/c++17/fs_dir.cc
@@ -230,7 +230,7 @@  directory_iterator(const path& p, directory_options options, error_code* ecptr)
 const fs::directory_entry&
 fs::directory_iterator::operator*() const noexcept
 {
-  return _M_dir->entry;
+  return (*_M_dir).entry;
 }
 
 fs::directory_iterator&
@@ -327,25 +327,25 @@  fs::recursive_directory_iterator::~recursive_directory_iterator() = default;
 fs::directory_options
 fs::recursive_directory_iterator::options() const noexcept
 {
-  return _M_dirs->options;
+  return (*_M_dirs).options;
 }
 
 int
 fs::recursive_directory_iterator::depth() const noexcept
 {
-  return int(_M_dirs->size()) - 1;
+  return int((*_M_dirs).size()) - 1;
 }
 
 bool
 fs::recursive_directory_iterator::recursion_pending() const noexcept
 {
-  return _M_dirs->pending;
+  return (*_M_dirs).pending;
 }
 
 const fs::directory_entry&
 fs::recursive_directory_iterator::operator*() const noexcept
 {
-  return _M_dirs->top().entry;
+  return (*_M_dirs).top().entry;
 }
 
 fs::recursive_directory_iterator&
@@ -453,7 +453,7 @@  fs::recursive_directory_iterator::pop()
 void
 fs::recursive_directory_iterator::disable_recursion_pending() noexcept
 {
-  _M_dirs->pending = false;
+  (*_M_dirs).pending = false;
 }
 
 // Used to implement filesystem::remove_all.