Bug 94681 - filesystem::sysmlink_status using stat instead of lstat when --disable-libstdcxx-filesystem-ts
Summary: filesystem::sysmlink_status using stat instead of lstat when --disable-libstd...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 9.3.0
: P3 normal
Target Milestone: 9.4
Assignee: Jonathan Wakely
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2020-04-21 08:41 UTC by Valentin David
Modified: 2020-09-21 23:14 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-04-21 00:00:00


Attachments
Proposed fix (300 bytes, patch)
2020-04-21 08:41 UTC, Valentin David
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Valentin David 2020-04-21 08:41:10 UTC
Created attachment 48321 [details]
Proposed fix

std::filesystem::sysmlink_status follows symlinks when GCC has been configured with --disable-libstdcxx-filesystem-ts, but it works fine when it has been configured with --enable-libstdcxx-filesystem-ts.

The reason is that libstdc++-v3/src/c++17/fs_ops.cc includes libstdc++-v3/src/filesystem/ops-common.h which has some requirements in the configure script that are not executed.

In libstdc++-v3/acinclude.m4, GLIBCXX_CHECK_FILESYSTEM_DEPS is guarded by test "$enable_libstdcxx_filesystem_ts = yes". But this should be run even if enable_libstdcxx_filesystem_ts is "no" since libstdc++-v3/src/filesystem/ops-common.h is used when --disable-libstdcxx-filesystem-ts is used.

Removing this "if" fixed the issue for me.

While I have not tested master, I believe the issue is still there.
Comment 1 Jonathan Wakely 2020-04-21 08:42:45 UTC
Oops, that configure option should have no impact on the std::filesystem implementation.
Comment 2 Jonathan Wakely 2020-04-21 08:55:32 UTC
I'm not too concerned about the impact on people using the --disable option, because the default for most targets is to enable it, and everything works.

But for systems where the default is --disable it means the std::filesystem support is borked.

I think it's a bit too late to fix this for GCC 10.1 because it would enable new code paths on systems where --disable is the default, and those are the less well-tested targets. I don't want to break those right before we release GCC 10.

So I'll fix this on master as soon as GCC 10.1 is released, and backport it for 10.2 and 9.4 later.
Comment 3 GCC Commits 2020-08-10 12:22:33 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:90f7636bf8df50940e0f749af60a6b374a8f09b4

commit r11-2633-g90f7636bf8df50940e0f749af60a6b374a8f09b4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Aug 10 13:21:59 2020 +0100

    libstdc++: Make C++17 ignore --disable-libstdcxx-filesystem-ts [PR 94681]
    
    The configure switch should only affect the optional Filesystem TS, not
    the std::filesystem features of C++17.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/94681
            * acinclude.m4 (GLIBCXX_CHECK_FILESYSTEM_DEPS): Do not depend on
            $enable_libstdcxx_filesystem_ts.
            * configure: Regenerate.
Comment 4 Jonathan Wakely 2020-08-10 12:25:17 UTC
Only fixed on master so far.
Comment 5 Christophe Lyon 2020-08-10 14:37:12 UTC
The commit r11-2633 broke the build of libstdc++ on aarch64-none-elf. My build logs say:
/tmp/7968837_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc: In function 'std::filesystem::__cxx11::path std::filesystem::read_symlink(const std::filesystem::__cxx11::path&, std::error_code&)':
/tmp/7968837_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc:1178:9: error: '::lstat' has not been declared; did you mean 'std::filesystem::__gnu_posix::lstat'?
 1178 |   if (::lstat(p.c_str(), &st))
      |         ^~~~~
      |         std::filesystem::__gnu_posix::lstat
In file included from /tmp/7968837_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc:58:
/tmp/7968837_9.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/../filesystem/ops-common.h:131:14: note: 'std::filesystem::__gnu_posix::lstat' declared here
  131 |   inline int lstat(const char* path, stat_type* buffer)
      |              ^~~~~
make[5]: *** [Makefile:572: fs_ops.lo] Error 1
make[5]: Leaving directory '/tmp/7968837_9.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64_be-none-elf/gcc3/aarch64_be-none-elf/libstdc++-v3/src/c++17'
make[4]: *** [Makefile:732: all-recursive] Error 1
Comment 6 Jonathan Wakely 2020-08-10 16:04:52 UTC
Thanks, I thought this might reveal some new issues :-)

I'll fix it asap.
Comment 7 GCC Commits 2020-08-10 18:12:36 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:5b065f0563262a0d6cd1fea8426913bfdd841301

commit r11-2638-g5b065f0563262a0d6cd1fea8426913bfdd841301
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Aug 10 18:58:14 2020 +0100

    libstdc++: Fix build for targets without lstat [PR 94681]
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/94681
            * src/c++17/fs_ops.cc (read_symlink): Use posix::lstat instead
            of calling ::lstat directly.
            * src/filesystem/ops.cc (read_symlink): Likewise.
Comment 8 GCC Commits 2020-09-21 23:00:38 UTC
The releases/gcc-10 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:4e00119c780f9bcd33181db4580754522f64c60e

commit r10-8783-g4e00119c780f9bcd33181db4580754522f64c60e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Aug 10 13:21:59 2020 +0100

    libstdc++: Make C++17 ignore --disable-libstdcxx-filesystem-ts [PR 94681]
    
    The configure switch should only affect the optional Filesystem TS, not
    the std::filesystem features of C++17.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/94681
            * acinclude.m4 (GLIBCXX_CHECK_FILESYSTEM_DEPS): Do not depend on
            $enable_libstdcxx_filesystem_ts.
            * configure: Regenerate.
    
    (cherry picked from commit 90f7636bf8df50940e0f749af60a6b374a8f09b4)
Comment 9 GCC Commits 2020-09-21 23:00:43 UTC
The releases/gcc-10 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

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

commit r10-8784-geef40b0037b90ff117322423fc297dc528285524
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Aug 10 18:58:14 2020 +0100

    libstdc++: Fix build for targets without lstat [PR 94681]
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/94681
            * src/c++17/fs_ops.cc (read_symlink): Use posix::lstat instead
            of calling ::lstat directly.
            * src/filesystem/ops.cc (read_symlink): Likewise.
    
    (cherry picked from commit 5b065f0563262a0d6cd1fea8426913bfdd841301)
Comment 10 GCC Commits 2020-09-21 23:14:00 UTC
The releases/gcc-9 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:3ec14c9a49aa6c35609eaf04f74132e28a2dc9d5

commit r9-8926-g3ec14c9a49aa6c35609eaf04f74132e28a2dc9d5
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Aug 10 13:21:59 2020 +0100

    libstdc++: Make C++17 ignore --disable-libstdcxx-filesystem-ts [PR 94681]
    
    The configure switch should only affect the optional Filesystem TS, not
    the std::filesystem features of C++17.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/94681
            * acinclude.m4 (GLIBCXX_CHECK_FILESYSTEM_DEPS): Do not depend on
            $enable_libstdcxx_filesystem_ts.
            * configure: Regenerate.
    
    (cherry picked from commit 90f7636bf8df50940e0f749af60a6b374a8f09b4)
Comment 11 GCC Commits 2020-09-21 23:14:06 UTC
The releases/gcc-9 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:90f845adff0379682080a1253d477e0b621768b7

commit r9-8927-g90f845adff0379682080a1253d477e0b621768b7
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Aug 10 18:58:14 2020 +0100

    libstdc++: Fix build for targets without lstat [PR 94681]
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/94681
            * src/c++17/fs_ops.cc (read_symlink): Use posix::lstat instead
            of calling ::lstat directly.
            * src/filesystem/ops.cc (read_symlink): Likewise.
    
    (cherry picked from commit 5b065f0563262a0d6cd1fea8426913bfdd841301)
Comment 12 Jonathan Wakely 2020-09-21 23:14:48 UTC
Fixed for 9.4 and 10.3
Comment 13 Jonathan Wakely 2020-09-21 23:14:56 UTC
.