[Bug libstdc++/97731] New: terminate called in std::experimental::filesystem::recursive_directory_iterator

[Bug libstdc++/97731] New: terminate called in std::experimental::filesystem::recursive_directory_iterator

[torsten.hilbrich at secunet dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97731

            Bug ID: 97731
           Summary: terminate called in
                    std::experimental::filesystem::recursive_directory_ite
                    rator
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: torsten.hilbrich at secunet dot com
  Target Milestone: ---

Created attachment 49506
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49506&action=edit
Reproducer for the problem

The recursive_directory_iterator causes a call to terminate when an I/O error
is happening on the directory to iterate. It originally occured in an
environment where access to a directory was no longer possible and caused EIO
reported through a linux device-mapper target. In the test program this
behaviour is simulated by replacing the readdir function with a mockup always
failing with errno set to EIO.

We initially found this problem with libstdc++ as provided with gcc 8.3.0. But
I also checked the version from gcc 10.2 and could reproduce the problem.

The testing code test.cpp is attached to this ticket.

Here is an example session:

$ g++ -o test test.cpp -lstdc++fs
$ ./test
terminate called after throwing an instance of
'std::experimental::filesystem::v1::__cxx11::filesystem_error'
  what():  filesystem error: directory iterator cannot advance: Input/output
error
Aborted (core dumped)

I already looked at the code and thing I found the culprit.

In recursive_directory_iterator(const path&, directory_options options,
error_code*) (dir.cc:199) the following method call is found:

   if (sp->top().advance(ec))

This calls the method "bool advance(bool skip_permission_denied = false)"
within _Dir, implicitly converting the std::error_code* to bool.

This way, we get roughly the following stack trace:

#0  std::filesystem::_Dir_base::advance (this=0x55555559a050,
skip_permission_denied=true, ec=...)
    at dir-common.h:110
#1  0x000055555555eb39 in
std::experimental::filesystem::v1::__cxx11::_Dir::advance (
    this=0x55555559a050, skip_permission_denied=true, ec=...) at dir.cc:65
#2  0x000055555555ed39 in
std::experimental::filesystem::v1::__cxx11::_Dir::advance (
    this=0x55555559a050, skip_permission_denied=true) at dir.cc:87
#3  0x000055555555d110 in
std::experimental::filesystem::v1::__cxx11::recursive_directory_iterator::recursive_directory_iterator
(this=0x7fffffffdd90, p=filesystem::path "/tmp" = {...}, 
    options=std::experimental::filesystem::v1::directory_options::none,
ec=0x7fffffffdd60) at dir.cc:199
#4  0x000055555555c07c in
std::experimental::filesystem::v1::__cxx11::recursive_directory_iterator::recursive_directory_iterator
(this=0x7fffffffdd90, __p=filesystem::path "/tmp" = {...}, __ec=...)
    at /usr/include/c++/9/experimental/bits/fs_dir.h:280
#5  0x000055555555bb5d in main () at ./test.cpp:17

and finally, an exception is thrown via a method declared noexcept, causing the
termination of the program.

Torsten

[Bug libstdc++/97731] terminate called in std::experimental::filesystem::recursive_directory_iterator

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97731

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
   Last reconfirmed|                            |2020-11-05

[Bug libstdc++/97731] terminate called in std::experimental::filesystem::recursive_directory_iterator

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97731

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Looks like I fixed it for std::filesystem::recursive_directory_iterator but not
the experimental version:

      if (ecptr ? sp->top().advance(*ecptr) : sp->top().advance())
        _M_dirs.swap(sp);

[Bug libstdc++/97731] terminate called in std::experimental::filesystem::recursive_directory_iterator

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97731

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:2f93a2a03a343a29f614a530d7657f1ed6347ed5

commit r11-4750-g2f93a2a03a343a29f614a530d7657f1ed6347ed5
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Nov 5 17:26:13 2020 +0000

    libstdc++: Use non-throwing increment in recursive_directory_iterator [PR
97731]

    As described in the PR, the recursive_directory_iterator constructor
    calls advance(ec), but ec is a pointer so it calls _Dir::advance(bool).
    The intention was to either call advance() or advance(*ec) depending
    whether the pointer is null or not.

    This fixes the bug and renames the parameter to ecptr to make similar
    mistakes less likely in future.

    libstdc++-v3/ChangeLog:

            PR libstdc++/97731
            * src/filesystem/dir.cc (recursive_directory_iterator): Call the
            right overload of _Dir::advance.
            * testsuite/experimental/filesystem/iterators/97731.cc: New test.

[Bug libstdc++/97731] terminate called in std::experimental::filesystem::recursive_directory_iterator

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97731

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |8.5

[Bug libstdc++/97731] terminate called in std::experimental::filesystem::recursive_directory_iterator

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97731

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:c9769a6eee38c396b797cffd819957fa9f1926b1

commit r9-9034-gc9769a6eee38c396b797cffd819957fa9f1926b1
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Nov 5 17:26:13 2020 +0000

    libstdc++: Use non-throwing increment in recursive_directory_iterator [PR
97731]

    As described in the PR, the recursive_directory_iterator constructor
    calls advance(ec), but ec is a pointer so it calls _Dir::advance(bool).
    The intention was to either call advance() or advance(*ec) depending
    whether the pointer is null or not.

    This fixes the bug and renames the parameter to ecptr to make similar
    mistakes less likely in future.

    libstdc++-v3/ChangeLog:

            PR libstdc++/97731
            * src/filesystem/dir.cc (recursive_directory_iterator): Call the
            right overload of _Dir::advance.
            * testsuite/experimental/filesystem/iterators/97731.cc: New test.

    (cherry picked from commit 2f93a2a03a343a29f614a530d7657f1ed6347ed5)

[Bug libstdc++/97731] terminate called in std::experimental::filesystem::recursive_directory_iterator

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97731

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:749cfa1f150d1e0749feb7aed8f68b8d0294b03f

commit r8-10616-g749cfa1f150d1e0749feb7aed8f68b8d0294b03f
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Nov 5 17:26:13 2020 +0000

    libstdc++: Use non-throwing increment in recursive_directory_iterator [PR
97731]

    As described in the PR, the recursive_directory_iterator constructor
    calls advance(ec), but ec is a pointer so it calls _Dir::advance(bool).
    The intention was to either call advance() or advance(*ec) depending
    whether the pointer is null or not.

    This fixes the bug and renames the parameter to ecptr to make similar
    mistakes less likely in future.

    libstdc++-v3/ChangeLog:

            PR libstdc++/97731
            * src/filesystem/dir.cc (recursive_directory_iterator): Call the
            right overload of _Dir::advance.
            * testsuite/experimental/filesystem/iterators/97731.cc: New test.

    (cherry picked from commit 2f93a2a03a343a29f614a530d7657f1ed6347ed5)

[Bug libstdc++/97731] terminate called in std::experimental::filesystem::recursive_directory_iterator

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97731

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 8.5, 9.4, 10.3 and 11.1

The gcc-10 commit was r10-8982 but didn't get added above.

[Bug libstdc++/97731] terminate called in std::experimental::filesystem::recursive_directory_iterator

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97731

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:2dc2f417288d4f0839b4bc01388e676ee343f941

commit r12-6982-g2dc2f417288d4f0839b4bc01388e676ee343f941
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Feb 1 14:02:56 2022 +0000

    libstdc++: Add more tests for filesystem directory iterators

    The PR 97731 test was added to verify a fix to the Filesystem TS code,
    but we should also have the same test to avoid similar regressions in
    the C++17 std::filesystem code.

    Also add tests for directory_options::follow_directory_symlink

    libstdc++-v3/ChangeLog:

            * testsuite/27_io/filesystem/iterators/97731.cc: New test.
            *
testsuite/27_io/filesystem/iterators/recursive_directory_iterator.cc:
            Check follow_directory_symlink option.
            *
testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc:
            Likewise.

[Bug libstdc++/97731] terminate called in std::experimental::filesystem::recursive_directory_iterator

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97731

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:9eed5712110b63a0021358cbf195d32c5b372638

commit r11-11049-g9eed5712110b63a0021358cbf195d32c5b372638
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Feb 1 14:02:56 2022 +0000

    libstdc++: Add more tests for filesystem directory iterators

    The PR 97731 test was added to verify a fix to the Filesystem TS code,
    but we should also have the same test to avoid similar regressions in
    the C++17 std::filesystem code.

    Also add tests for directory_options::follow_directory_symlink

    libstdc++-v3/ChangeLog:

            * testsuite/27_io/filesystem/iterators/97731.cc: New test.
            *
testsuite/27_io/filesystem/iterators/recursive_directory_iterator.cc:
            Check follow_directory_symlink option.
            *
testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc:
            Likewise.

    (cherry picked from commit 2dc2f417288d4f0839b4bc01388e676ee343f941)