[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[bugdal at aerifal dot cx]

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

--- Comment #2 from Rich Felker <bugdal at aerifal dot cx> ---
Rather than #if defined(SYS_futex_time64), I think it should be made:

#if defined(SYS_futex_time64) && SYS_futex_time64 != SYS_futex

This is in consideration of support for riscv32 and future archs without legacy
syscalls. It's my intent in musl to accept the riscv32 port with SYS_futex
defined to be equal to SYS_futex_time64; otherwise all software making use of
SYS_futex gratuitously breaks.

[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Ack. I assume you saw PR 97182 and that's what prompted your comment. Otherwise
it's quite a coincidence :)

[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[bugdal at aerifal dot cx]

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

--- Comment #4 from Rich Felker <bugdal at aerifal dot cx> ---
Actually I didn't see it, I just saw Florian added to CC and it reminded me of
the issue, which reminded me I needed to check this for riscv32 issues with the
riscv32 port pending merge. :-)

[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Created attachment 49267
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49267&action=edit
Proposed fix

Rich, does this work for musl?

As glibc hasn't updated its userspace timespec yet I have no way to test
whether this fixes the problem, because I don't have the problem.

Is inspecting __USE_TIME_BITS64 allowed with musl? The current proposal at 
https://sourceware.org/glibc/wiki/Y2038ProofnessDesign#API_design says users
can check if that's defined, although usually the __USE_xxx macros are not
meant for public consumption.

[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
On second thoughts, we probably don't need to worry about
SYS_clock_gettime_time64. We only use SYS_clock_gettime syscalls on old glibc
systems where clock_gettime is in librt not libc, and those systems aren't
going to get updated with the time64 version.

[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[bugdal at aerifal dot cx]

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

--- Comment #7 from Rich Felker <bugdal at aerifal dot cx> ---
Indeed, the direct clock_gettime syscall stuff is just unnecessary on any
modern system, certainly any time64 one. I read the patch briefly and I don't
see anywhere it would break anything, but it also wouldn't produce a useful
Y2038-ready configuration, so I don't think it makes sense. Configure or
source-level assertions should just ensure that, if time_t is larger than long
and there's a distinct time64 syscall, the direct syscall is never used.

[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 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:4d039cb9a1d0ffc6910fe09b726c3561b64527dc

commit r11-5022-g4d039cb9a1d0ffc6910fe09b726c3561b64527dc
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Sep 24 17:35:52 2020 +0100

    libstdc++: Use custom timespec in system calls [PR 93421]

    On 32-bit targets where userspace has switched to 64-bit time_t, we
    cannot pass struct timespec to SYS_futex or SYS_clock_gettime, because
    the userspace definition of struct timespec will not match what the
    kernel expects.

    We use the existence of the SYS_futex_time64 or SYS_clock_gettime_time64
    macros to imply that userspace *might* have switched to the new timespec
    definition.  This is a conservative assumption. It's possible that the
    new syscall numbers are defined in the libc headers but that timespec
    hasn't been updated yet (as is the case for glibc currently). But using
    the alternative struct with two longs is still OK, it's just redundant
    if userspace timespec still uses a 32-bit time_t.

    We also check that SYS_futex_time64 != SYS_futex so that we don't try
    to use a 32-bit tv_sec on modern targets that only support the 64-bit
    system calls and define the old macro to the same value as the new one.

    We could possibly check #ifdef __USE_TIME_BITS64 to see whether
    userspace has actually been updated, but it's not clear if user code
    is meant to inspect that or if it's only for libc internal use.

    libstdc++-v3/ChangeLog:

            PR libstdc++/93421
            * src/c++11/chrono.cc [_GLIBCXX_USE_CLOCK_GETTIME_SYSCALL]
            (syscall_timespec): Define a type suitable for SYS_clock_gettime
            calls.
            (system_clock::now(), steady_clock::now()): Use syscall_timespec
            instead of timespec.
            * src/c++11/futex.cc (syscall_timespec): Define a type suitable
            for SYS_futex and SYS_clock_gettime calls.
            (relative_timespec): Use syscall_timespec instead of timespec.
            (__atomic_futex_unsigned_base::_M_futex_wait_until): Likewise.
            (__atomic_futex_unsigned_base::_M_futex_wait_until_steady):
            Likewise.

[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Rich Felker from comment #7)
> Indeed, the direct clock_gettime syscall stuff is just unnecessary on any
> modern system, certainly any time64 one. I read the patch briefly and I
> don't see anywhere it would break anything, but it also wouldn't produce a
> useful Y2038-ready configuration, so I don't think it makes sense. Configure
> or source-level assertions should just ensure that, if time_t is larger than
> long and there's a distinct time64 syscall, the direct syscall is never used.

I didn't get around to making use of the new syscalls for GCC 11, so I've
committed that patch. I'll keep this bug open to revisit it and do it properly
later.

[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 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:1e3e6c700f04fe6992b9077541e434172c1cbdae

commit r11-5114-g1e3e6c700f04fe6992b9077541e434172c1cbdae
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Nov 17 16:13:02 2020 +0000

    libstdc++: Revert changes for SYS_clock_gettime64 [PR 93421]

    As discussed in the PR, it's incredibly unlikely that a system that
    needs to use the SYS_clock_gettime syscall (e.g. glibc 2.16 or older) is
    going to define the SYS_clock_gettime64 macro. Ancient systems that need
    to use the syscall aren't going to have time64 support.

    This reverts the recent changes to try and make clock_gettime syscalls
    be compatible with systems that have been updated for time64 (those
    changes were wrong anyway as they misspelled the SYS_clock_gettime64
    macro). The changes for futex syscalls are retained, because we still
    use them on modern systems that might be using time64.

    To ensure that the clock_gettime syscalls are safe, configure will fail
    if SYS_clock_gettime is needed, and SYS_clock_gettime64 is also defined
    (but to a distinct value from SYS_clock_gettime), and the tv_sec member
    of timespec is larger than long. This means we will be unable to build
    on a hypothetical system where we need the time32 version of
    SYS_clock_gettime but where userspace is using a time64 struct timespec.
    In the unlikely event that this failure is triggered on any real
    systems, we can fix it later. But we probably won't need to.

    libstdc++-v3/ChangeLog:

            PR libstdc++/93421
            * acinclude.m4 (GLIBCXX_ENABLE_LIBSTDCXX_TIME): Fail if struct
            timespec isn't compatible with SYS_clock_gettime.
            * configure: Regenerate.
            * src/c++11/chrono.cc: Revert changes for time64 compatibility.
            Add static_assert instead.
            * src/c++11/futex.cc (_M_futex_wait_until_steady): Assume
            SYS_clock_gettime can use struct timespec.

[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 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:b108faa9400e13a3d00dd7f71cff0ac45e29c5c9

commit r11-5167-gb108faa9400e13a3d00dd7f71cff0ac45e29c5c9
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Nov 19 13:33:11 2020 +0000

    libstdc++: Fix overflow checks to use the correct "time_t" [PR 93456]

    I recently added overflow checks to src/c++11/futex.cc for PR 93456, but
    then changed the type of the timespec for PR 93421. This meant the
    overflow checks were no longer using the right range, because the
    variable being written to might be smaller than time_t.

    This introduces new typedef that corresponds to the tv_sec member of the
    struct being passed to the syscall, and uses that typedef in the range
    checks.

    libstdc++-v3/ChangeLog:

            PR libstdc++/93421
            PR libstdc++/93456
            * src/c++11/futex.cc (syscall_time_t): New typedef for
            the type of the syscall_timespec::tv_sec member.
            (relative_timespec, _M_futex_wait_until)
            (_M_futex_wait_until_steady): Use syscall_time_t in overflow
            checks, not time_t.

[Bug libstdc++/93421] futex.cc use of futex syscall is not time64-compatible

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
           Assignee|redi at gcc dot gnu.org            |unassigned at gcc dot gnu.org