On Mon, 28 Sept 2026 at 12:42, Gokbey Gazi Keskin <[email protected]> wrote: > > When neither unlinkat nor dirent::d_type is available, _Dir::advance > leaves every entry with file_type::none, so __erase takes the unlink > path. Without unlinkat _Dir::do_unlink ignores its is_directory > argument and always calls fs::remove, i.e. ::remove, which retries as > rmdir for a directory. Unlinking a non-empty directory therefore > reports the rmdir errno for "not empty" rather than the EPERM or EISDIR > that unlink on a directory would give, the retry-as-directory path is > not taken, and remove_all fails with (uintmax_t)-1 on the first > non-empty sub-directory. POSIX allows either ENOTEMPTY or EEXIST for > that condition, so accept both, but only when unlinkat is unavailable. > > An empty directory of unknown type is still removed correctly, because > ::remove falls back to rmdir and succeeds, so reproducing this needs a > tree at least two levels deep. > > PR libstdc++/127555 > > libstdc++-v3/ChangeLog: > > * src/c++17/fs_dir.cc (recursive_directory_iterator::__erase): > Retry as a directory when unlink fails with ENOTEMPTY or EEXIST > and unlinkat is not available. > > Signed-off-by: Gokbey Gazi Keskin <[email protected]> > ---
Thanks for the patch, including the updates based on my comment in bugzilla. This looks good to me and I will take care of merging it. I actually proposed a patch to drop the #ifdef EPERM check which I think is completely unnecessary: https://gcc.gnu.org/pipermail/gcc-patches/2026-September/732625.html I can rebase that on top of yours, or vice versa, it's trivial to resolve. > Tested on x86_64-pc-linux-gnu. > > libstdc++-v3/src/c++17/fs_dir.cc | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/libstdc++-v3/src/c++17/fs_dir.cc > b/libstdc++-v3/src/c++17/fs_dir.cc > index f6860e26411..7cced0db823 100644 > --- a/libstdc++-v3/src/c++17/fs_dir.cc > +++ b/libstdc++-v3/src/c++17/fs_dir.cc > @@ -514,13 +514,23 @@ fs::recursive_directory_iterator::__erase(error_code* > ecptr) > { > // 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. > + const int err = ec.value(); > #ifdef 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) > + bool is_dir = err == EPERM || err == EISDIR; > #else > - if (ec.value() == EISDIR) > + bool is_dir = err == EISDIR; > #endif > +#if ! _GLIBCXX_HAVE_UNLINKAT > + // Without unlinkat, _Dir::unlink uses ::remove, which retries as > + // rmdir for a directory. Unlinking a *non-empty* directory then > + // reports "directory not empty" instead of the errno for unlink > + // on a directory, so treat that as a directory too. POSIX allows > + // either ENOTEMPTY or EEXIST for that condition. > + is_dir = is_dir || err == ENOTEMPTY || err == EEXIST; > +#endif > + if (is_dir) > { > // Retry, treating it as a directory. > top.entry._M_type = file_type::directory; > -- > 2.43.0 >
