On 14/11/20 14:23 +0000, Jonathan Wakely via Libstdc++ wrote:
On Sat, 14 Nov 2020, 13:30 Mike Crowe via Libstdc++, <libstd...@gcc.gnu.org>
wrote:
On Saturday 14 November 2020 at 00:17:59 +0000, Jonathan Wakely via
Libstdc++ wrote:
> 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.
>
> Tested x86_64-linux, -m32 too. Committed to trunk.
>
> commit 4d039cb9a1d0ffc6910fe09b726c3561b64527dc
> Author: Jonathan Wakely <jwak...@redhat.com>
> Date: Thu Sep 24 17:35:52 2020
>
> 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.
Presumably this is change is only good for the short term? We really want
to be calling the 64-bit time_t versions of SYS_futex and SYS_clock_gettime
passing 64-bit struct timespec so that this code continues to work
correctly after 2038 (for CLOCK_REALTIME) or if someone is unlucky enough
to have a system uptime of over 68 years (for CLOCK_MONOTONIC.) Perhaps
that's part of the post-GCC11 work that you plan to do.
Right. This is definitely a short term solution, but I ran out of time to
do something better for GCC 11.
We can still do a bit better than that patch though, which was just
broken (it's SYS_clock_gettime64 not SYS_clock_gettime_time64 for a
start).
This partially reverts it, to remove the conditional compilation
related to SYS_clock_gettime64, because that's almost certainly never
going to be needed.
Tested x86_64-linux, with glibc 2.12 and 2.31, committed to trunk.
> @@ -195,7 +205,7 @@ namespace
> if (__s.count() < 0) [[unlikely]]
> return false;
>
> - struct timespec rt;
> + syscall_timespec rt;
> if (__s.count() > __int_traits<time_t>::__max) [[unlikely]]
> rt.tv_sec = __int_traits<time_t>::__max;
Do these now need to be __int_traits<long>::__max in case time_t is 64-bit
yet syscall_timespec is using 32-bit long?
Ah yes. Maybe decltype(rt.tv_sec).
I'll fix that in the next patch.
commit 1e3e6c700f04fe6992b9077541e434172c1cbdae
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Tue Nov 17 16:13:02 2020
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.
diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 650d63ab3d75..486347b34d94 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -1561,13 +1561,34 @@ AC_DEFUN([GLIBCXX_ENABLE_LIBSTDCXX_TIME], [
#endif
syscall(SYS_clock_gettime, CLOCK_MONOTONIC, &tp);
syscall(SYS_clock_gettime, CLOCK_REALTIME, &tp);
- ], [ac_has_clock_monotonic_syscall=yes], [ac_has_clock_monotonic_syscall=no])
- AC_MSG_RESULT($ac_has_clock_monotonic_syscall)
- if test x"$ac_has_clock_monotonic_syscall" = x"yes"; then
+ ], [ac_has_clock_gettime_syscall=yes], [ac_has_clock_gettime_syscall=no])
+ AC_MSG_RESULT($ac_has_clock_gettime_syscall)
+ if test x"$ac_has_clock_gettime_syscall" = x"yes"; then
AC_DEFINE(_GLIBCXX_USE_CLOCK_GETTIME_SYSCALL, 1,
- [ Defined if clock_gettime syscall has monotonic and realtime clock support. ])
+ [Defined if clock_gettime syscall has monotonic and realtime clock support. ])
ac_has_clock_monotonic=yes
ac_has_clock_realtime=yes
+ AC_MSG_CHECKING([for struct timespec that matches syscall])
+ AC_TRY_COMPILE(
+ [#include <time.h>
+ #include <sys/syscall.h>
+ ],
+ [#ifdef SYS_clock_gettime64
+ #if SYS_clock_gettime64 != SYS_clock_gettime
+ // We need to use SYS_clock_gettime and libc appears to
+ // also know about the SYS_clock_gettime64 syscall.
+ // Check that userspace doesn't use time64 version of timespec.
+ static_assert(sizeof(timespec::tv_sec) == sizeof(long),
+ "struct timespec must be compatible with SYS_clock_gettime");
+ #endif
+ #endif
+ ],
+ [ac_timespec_matches_syscall=yes],
+ [ac_timespec_matches_syscall=no])
+ AC_MSG_RESULT($ac_timespec_matches_syscall)
+ if test x"$ac_timespec_matches_syscall" = no; then
+ AC_MSG_ERROR([struct timespec is not compatible with SYS_clock_gettime, please report a bug to http://gcc.gnu.org/bugzilla])
+ fi
fi;;
esac
fi
diff --git a/libstdc++-v3/src/c++11/chrono.cc b/libstdc++-v3/src/c++11/chrono.cc
index f10be7d8c073..723f3002d11a 100644
--- a/libstdc++-v3/src/c++11/chrono.cc
+++ b/libstdc++-v3/src/c++11/chrono.cc
@@ -35,17 +35,6 @@
#ifdef _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL
#include <unistd.h>
#include <sys/syscall.h>
-
-# if defined(SYS_clock_gettime_time64) \
- && SYS_clock_gettime_time64 != SYS_clock_gettime
- // Userspace knows about the new time64 syscalls, so it's possible that
- // userspace has also updated timespec to use a 64-bit tv_sec.
- // The SYS_clock_gettime syscall still uses the old definition
- // of timespec where tv_sec is 32 bits, so define a type that matches that.
- struct syscall_timespec { long tv_sec; long tv_nsec; };
-# else
- using syscall_timespec = ::timespec;
-# endif
#endif
namespace std _GLIBCXX_VISIBILITY(default)
@@ -63,12 +52,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
system_clock::now() noexcept
{
#ifdef _GLIBCXX_USE_CLOCK_REALTIME
+ timespec tp;
// -EINVAL, -EFAULT
#ifdef _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL
- syscall_timespec tp;
syscall(SYS_clock_gettime, CLOCK_REALTIME, &tp);
#else
- timespec tp;
clock_gettime(CLOCK_REALTIME, &tp);
#endif
return time_point(duration(chrono::seconds(tp.tv_sec)
@@ -92,12 +80,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
steady_clock::now() noexcept
{
#ifdef _GLIBCXX_USE_CLOCK_MONOTONIC
+ timespec tp;
// -EINVAL, -EFAULT
#ifdef _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL
- syscall_timespec tp;
syscall(SYS_clock_gettime, CLOCK_MONOTONIC, &tp);
#else
- timespec tp;
clock_gettime(CLOCK_MONOTONIC, &tp);
#endif
return time_point(duration(chrono::seconds(tp.tv_sec)
diff --git a/libstdc++-v3/src/c++11/futex.cc b/libstdc++-v3/src/c++11/futex.cc
index 9adfb898b4f1..33e2097e19cf 100644
--- a/libstdc++-v3/src/c++11/futex.cc
+++ b/libstdc++-v3/src/c++11/futex.cc
@@ -61,8 +61,8 @@ namespace
#if defined(SYS_futex_time64) && SYS_futex_time64 != SYS_futex
// Userspace knows about the new time64 syscalls, so it's possible that
// userspace has also updated timespec to use a 64-bit tv_sec.
- // The SYS_futex and SYS_clock_gettime syscalls still use the old definition
- // of timespec where tv_sec is 32 bits, so define a type that matches that.
+ // The SYS_futex syscall still uses the old definition of timespec
+ // where tv_sec is 32 bits, so define a type that matches that.
struct syscall_timespec { long tv_sec; long tv_nsec; };
#else
using syscall_timespec = ::timespec;
@@ -234,11 +234,10 @@ namespace
// We only get to here if futex_clock_monotonic_unavailable was
// true or has just been set to true.
+ struct timespec ts;
#ifdef _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL
- syscall_timespec ts;
syscall(SYS_clock_gettime, CLOCK_MONOTONIC, &ts);
#else
- struct timespec ts;
clock_gettime(CLOCK_MONOTONIC, &ts);
#endif
