[2/6] libstdc++: Fix shared_timed_mutex for negative timeouts [PR116586]

  Passing a timeout from before the epoch to
std::shared_timed_mutex::try_lock_until or
std::shared_timed_mutex::try_lock_shared_until causes the POSIX rwlock
functions to be passed an invalid negative timeout which results in them
returning EINVAL.

thread.timedmutex.requirements.general in the C++ standard says:
> If abs_time has already passed, the function attempts to obtain
> ownership without blocking (as if by calling try_lock()).

and a time before the epoch has clearly already passed (see description
of libstdc++/PR116586). Let's treat such negative times as being at the
epoch so that these methods work correctly. Add test cases to prove that
this, and potential similar problems are no longer present.

    PR libstdc++-v3/116586

libstdc++-v3/ChangeLog:

        * include/std/shared_mutex: (shared_timed_mutex::try_lock_until,
          shared_timed_mutex::try_lock_shared_until): Ensure that
          negative timeout is not passed to POSIX rwlock functions.
        * testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc:
          New test.

Signed-off-by: Mike Crowe <mac@mcrowe.com>
---
 libstdc++-v3/include/std/shared_mutex         | 20 ++++
 .../try_lock_until/116586.cc                  | 94 +++++++++++++++++++
 2 files changed, 114 insertions(+)
 create mode 100644 libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc
  

On Sun, 14 Sep 2025 at 21:21 +0100, Mike Crowe wrote:
>Passing a timeout from before the epoch to
>std::shared_timed_mutex::try_lock_until or
>std::shared_timed_mutex::try_lock_shared_until causes the POSIX rwlock
>functions to be passed an invalid negative timeout which results in them
>returning EINVAL.
>
>thread.timedmutex.requirements.general in the C++ standard says:
>> If abs_time has already passed, the function attempts to obtain
>> ownership without blocking (as if by calling try_lock()).
>
>and a time before the epoch has clearly already passed (see description
>of libstdc++/PR116586). Let's treat such negative times as being at the
>epoch so that these methods work correctly. Add test cases to prove that
>this, and potential similar problems are no longer present.
>
>    PR libstdc++-v3/116586
>
>libstdc++-v3/ChangeLog:
>
>        * include/std/shared_mutex: (shared_timed_mutex::try_lock_until,
>          shared_timed_mutex::try_lock_shared_until): Ensure that
>          negative timeout is not passed to POSIX rwlock functions.
>        * testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc:
>          New test.
>
>Signed-off-by: Mike Crowe <mac@mcrowe.com>
>---
> libstdc++-v3/include/std/shared_mutex         | 20 ++++
> .../try_lock_until/116586.cc                  | 94 +++++++++++++++++++
> 2 files changed, 114 insertions(+)
> create mode 100644 libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc
>
>diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex
>index 94c8532399d..ef0f1df5a2b 100644
>--- a/libstdc++-v3/include/std/shared_mutex
>+++ b/libstdc++-v3/include/std/shared_mutex
>@@ -529,6 +529,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret = __glibcxx_rwlock_timedwrlock(&_M_rwlock, &__ts);
> 	// On self-deadlock, we just fail to acquire the lock.  Technically,
> 	// the program violated the precondition.
>@@ -555,6 +560,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret = pthread_rwlock_clockwrlock(&_M_rwlock, CLOCK_MONOTONIC,
> 					       &__ts);
> 	// On self-deadlock, we just fail to acquire the lock.  Technically,
>@@ -605,6 +615,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret;
> 	// Unlike for lock(), we are not allowed to throw an exception so if
> 	// the maximum number of read locks has been exceeded, or we would
>@@ -645,6 +660,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret = pthread_rwlock_clockrdlock(&_M_rwlock, CLOCK_MONOTONIC,
> 					       &__ts);
> 	// On self-deadlock, we just fail to acquire the lock.  Technically,

A few months ago I was testing the patch below, which replaces all the
repeated code that creates timespec objects with a call to a single
function. That function avoids overflows, e.g. if a timeout of
chrono::years::max() is used, and also turns negative timeouts into
the epoch.

This avoids solving the same problems in several places across the
library, like the four places above in <shared_mutex>.

I got distracted by other fixes and never submitted this patch to the
mailing list (we were in the final stages of gcc 15 when I last worked
on it). I think it's a good approach but would appreciate your review.
I think what I'll do is apply your patches locally on top of this one
(resolving the conflicts) and test everything again.


commit 27a12d66c96ebc107cc997faa63372392436de33
Author:     Jonathan Wakely <jwakely@redhat.com>
AuthorDate: Wed Apr 30 10:58:51 2025
Commit:     Jonathan Wakely <redi@gcc.gnu.org>
CommitDate: Mon May 19 14:25:49 2025

     libstdc++: Avoid overflow in timeout conversions [PR113327]
     
     When converting from a coarse duration with a very large value, the
     existing code scales that up to chrono::seconds which overflows the
     chrono::seconds::rep type. For example, sleep_for(chrono::hours::max())
     tries to calculate LLONG_MAX * 3600, which overflows to -3600 and so the
     sleep returns immediately.
     
     The solution in this commit is inspired by this_thread::sleep_for in
     libc++ which compares the duration argument to
     chrono::duration<long double>(nanoseconds::max()) and limits the
     duration to nanoseconds::max(). Because we split the duration into
     seconds and nanoseconds, we can use seconds::max() as our upper limit.
     
     We might need to limit further if seconds::max() doesn't fit in the
     type used for sleeping, which is one of std::time_t, unsigned int, or
     chrono::milliseconds.
     
     libstdc++-v3/ChangeLog:
     
             PR libstdc++/113327
             PR libstdc++/119258
             PR libstdc++/58931
             * include/bits/chrono.h (__to_timespec): New overloaded function
             templates for converting chrono types to timespec.
             (__to_gthread_time_t): New function template for converting
             time_point to __gthread_time_t.
             * include/bits/this_thread_sleep.h (sleep_for): Use
             __to_timespec.
             (__sleep_for): Remove namespace-scope declaration.
             * include/bits/atomic_timed_wait.h: Use __to_timespec and
             __to_gthread_time_t for timeouts.
             * include/std/condition_variable: Likewise.
             * include/std/mutex: Likewise.
             * include/std/shared_mutex: Likewise.
             * src/c++11/thread.cc (limit): New helper function.
             (__sleep_for): Use limit to prevent overflow when converting
             chrono::seconds to time_t, unsigned, or chrono::milliseconds.
             * testsuite/30_threads/this_thread/113327.cc: New test.

diff --git a/libstdc++-v3/include/bits/atomic_timed_wait.h b/libstdc++-v3/include/bits/atomic_timed_wait.h
index 9a6ac95b7d0e..9b915d11f672 100644
--- a/libstdc++-v3/include/bits/atomic_timed_wait.h
+++ b/libstdc++-v3/include/bits/atomic_timed_wait.h
@@ -83,14 +83,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  				 const chrono::time_point<__wait_clock_t, _Dur>&
  				      __atime) noexcept
        {
-	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
-	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
-
-	struct timespec __rt =
-	{
-	  static_cast<std::time_t>(__s.time_since_epoch().count()),
-	  static_cast<long>(__ns.count())
-	};
+	struct timespec __rt = chrono::__to_timespec(__atime);
  
  	auto __e = syscall (SYS_futex, __addr,
  			    static_cast<int>(__futex_wait_flags::
@@ -150,14 +143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  	static_assert(std::__is_one_of<_Clock, chrono::steady_clock,
  					       chrono::system_clock>::value);
  
-	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
-	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
-
-	__gthread_time_t __ts =
-	  {
-	    static_cast<std::time_t>(__s.time_since_epoch().count()),
-	    static_cast<long>(__ns.count())
-	  };
+	__gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
  
  #ifdef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT
  	if constexpr (is_same_v<chrono::steady_clock, _Clock>)
diff --git a/libstdc++-v3/include/bits/chrono.h b/libstdc++-v3/include/bits/chrono.h
index fad216203d2f..55becc4d9aef 100644
--- a/libstdc++-v3/include/bits/chrono.h
+++ b/libstdc++-v3/include/bits/chrono.h
@@ -50,6 +50,9 @@
  
  #include <bits/version.h>
  
+// TODO move __to_gthread_time_t to a better place
+#include <bits/gthr.h> // for __gthread_time_t 
+
  namespace std _GLIBCXX_VISIBILITY(default)
  {
  _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -1513,6 +1516,98 @@ _GLIBCXX_END_INLINE_ABI_NAMESPACE(_V2)
    } // namespace filesystem
  #endif // C++17 && HOSTED
  
+#if defined _GLIBCXX_USE_NANOSLEEP || defined _GLIBCXX_USE_CLOCK_REALTIME \
+    || defined _GLIBCXX_HAS_GTHREADS
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wc++17-extensions"
+namespace chrono
+{
+/// @cond undocumented
+
+  // Convert a chrono::duration to a relative time represented as timespec
+  // (e.g. for use with nanosleep).
+  template<typename _Rep, typename _Period>
+    [[__nodiscard__]] _GLIBCXX14_CONSTEXPR inline
+    struct ::timespec
+    __to_timespec(const duration<_Rep, _Period>& __d)
+    {
+      struct ::timespec __ts{};
+
+      if (__d < __d.zero()) // Negative timeouts don't make sense.
+	return __ts;
+
+      if constexpr (__or_<ratio_greater<_Period, ratio<1>>,
+			  treat_as_floating_point<_Rep>>::value)
+	{
+	  // Converting from e.g. chrono::hours::max() to chrono::seconds
+	  // would evaluate LLONG_MAX * 3600 which would overflow.
+	  // Limit to chrono::seconds::max().
+	  chrono::duration<double> __fmax(chrono::seconds::max());
+	  if (__d > __fmax) [[__unlikely__]]
+	    return chrono::__to_timespec(chrono::seconds::max());
+	}
+
+      auto __s = chrono::duration_cast<chrono::seconds>(__d);
+
+      if constexpr (is_integral<time_t>::value) // POSIX.1-2001 allows floating
+	{
+	  // Also limit to time_t maximum (only relevant for 32-bit time_t).
+	  constexpr auto __tmax = numeric_limits<time_t>::max();
+	  if (__s.count() > __tmax) [[__unlikely__]]
+	    {
+	      __ts.tv_sec = __tmax;
+	      return __ts;
+	    }
+	}
+
+      auto __ns = chrono::duration_cast<chrono::nanoseconds>(__d - __s);
+
+      if constexpr (treat_as_floating_point<_Rep>::value)
+	if (__ns.count() > 999999999) [[__unlikely__]]
+	  __ns = chrono::nanoseconds(999999999);
+
+      __ts.tv_sec = static_cast<time_t>(__s.count());
+      __ts.tv_nsec = static_cast<long>(__ns.count());
+      return __ts;
+    }
+
+  // Convert a chrono::time_point to an absolute time represented as timespec.
+  // All times before the epoch get converted to the epoch, so this assumes
+  // that we only use it for clocks where that's true.
+  // It should be safe to use this for system_clock and steady_clock.
+  template<typename _Clock, typename _Dur>
+    [[__nodiscard__]] _GLIBCXX14_CONSTEXPR inline
+    struct ::timespec
+    __to_timespec(const time_point<_Clock, _Dur>& __t)
+    {
+      return chrono::__to_timespec(__t.time_since_epoch());
+    }
+
+#ifdef _GLIBCXX_HAS_GTHREADS
+  // Convert a time_point to an absolute time represented as __gthread_time_t
+  // (which is typically just a typedef for struct timespec).
+  template<typename _Clock, typename _Dur>
+    [[__nodiscard__]] _GLIBCXX14_CONSTEXPR inline
+    __gthread_time_t
+    __to_gthread_time_t(const time_point<_Clock, _Dur>& __t)
+    {
+      auto __ts = chrono::__to_timespec(__t.time_since_epoch());
+      if constexpr (is_same<::timespec, __gthread_time_t>::value)
+	return __ts;
+      else if constexpr (is_convertible<::timespec, __gthread_time_t>::value)
+	return __ts;
+      else if constexpr (is_scalar<__gthread_time_t>::value)
+	return static_cast<__gthread_time_t>(__ts.tv_sec);
+      else // Assume this works:
+	return __gthread_time_t{ __ts.tv_sec, __ts.tv_nsec };
+    }
+#endif // HAS_GTHREADS
+
+/// @endcond
+} // namespace chrono
+#pragma GCC diagnostic pop
+#endif // USE_NANOSLEEP || USE_CLOCK_REALTIME || HAS_GTHREADS
+
  _GLIBCXX_END_NAMESPACE_VERSION
  } // namespace std
  
diff --git a/libstdc++-v3/include/bits/this_thread_sleep.h b/libstdc++-v3/include/bits/this_thread_sleep.h
index 57f89f858952..bf52aa9cd1c5 100644
--- a/libstdc++-v3/include/bits/this_thread_sleep.h
+++ b/libstdc++-v3/include/bits/this_thread_sleep.h
@@ -36,6 +36,7 @@
  
  #if __cplusplus >= 201103L
  #include <bits/chrono.h> // std::chrono::*
+#include <ext/numeric_traits.h> // __int_traits
  
  #ifdef _GLIBCXX_USE_NANOSLEEP
  # include <cerrno>  // errno, EINTR
@@ -59,11 +60,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
    {
  #ifndef _GLIBCXX_NO_SLEEP
  
-#ifndef _GLIBCXX_USE_NANOSLEEP
-    void
-    __sleep_for(chrono::seconds, chrono::nanoseconds);
-#endif
-
      /// this_thread::sleep_for
      template<typename _Rep, typename _Period>
        inline void
@@ -71,18 +67,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        {
  	if (__rtime <= __rtime.zero())
  	  return;
-	auto __s = chrono::duration_cast<chrono::seconds>(__rtime);
-	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__rtime - __s);
+
+	struct timespec __ts = chrono::__to_timespec(__rtime);
  #ifdef _GLIBCXX_USE_NANOSLEEP
-	struct ::timespec __ts =
-	  {
-	    static_cast<std::time_t>(__s.count()),
-	    static_cast<long>(__ns.count())
-	  };
  	while (::nanosleep(&__ts, &__ts) == -1 && errno == EINTR)
  	  { }
  #else
-	__sleep_for(__s, __ns);
+	using chrono::seconds;
+	using chrono::nanoseconds;
+	void __sleep_for(seconds __s, nanoseconds __ns);
+	__sleep_for(seconds(__ts.tv_sec), nanoseconds(__ts.tv_nsec));
  #endif
        }
  
diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable
index 3525ff35ba31..cbc399493db3 100644
--- a/libstdc++-v3/include/std/condition_variable
+++ b/libstdc++-v3/include/std/condition_variable
@@ -193,15 +193,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        __wait_until_impl(unique_lock<mutex>& __lock,
  			const chrono::time_point<steady_clock, _Dur>& __atime)
        {
-	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
-	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
-
-	__gthread_time_t __ts =
-	  {
-	    static_cast<std::time_t>(__s.time_since_epoch().count()),
-	    static_cast<long>(__ns.count())
-	  };
-
+	__gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
  	_M_cond.wait_until(*__lock.mutex(), CLOCK_MONOTONIC, __ts);
  
  	return (steady_clock::now() < __atime
@@ -214,15 +206,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        __wait_until_impl(unique_lock<mutex>& __lock,
  			const chrono::time_point<system_clock, _Dur>& __atime)
        {
-	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
-	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
-
-	__gthread_time_t __ts =
-	  {
-	    static_cast<std::time_t>(__s.time_since_epoch().count()),
-	    static_cast<long>(__ns.count())
-	  };
-
+	__gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
  	_M_cond.wait_until(*__lock.mutex(), __ts);
  
  	return (system_clock::now() < __atime
diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index e575a81c138e..a4624b475411 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -179,14 +179,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  	_M_try_lock_until(const chrono::time_point<chrono::system_clock,
  						   _Duration>& __atime)
  	{
-	  auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
-	  auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
-
-	  __gthread_time_t __ts = {
-	    static_cast<std::time_t>(__s.time_since_epoch().count()),
-	    static_cast<long>(__ns.count())
-	  };
-
+	  __gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
  	  return static_cast<_Derived*>(this)->_M_timedlock(__ts);
  	}
  
@@ -196,14 +189,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  	_M_try_lock_until(const chrono::time_point<chrono::steady_clock,
  						   _Duration>& __atime)
  	{
-	  auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
-	  auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
-
-	  __gthread_time_t __ts = {
-	    static_cast<std::time_t>(__s.time_since_epoch().count()),
-	    static_cast<long>(__ns.count())
-	  };
-
+	  __gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
  	  return static_cast<_Derived*>(this)->_M_clocklock(CLOCK_MONOTONIC,
  							    __ts);
  	}
diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex
index 94c8532399d9..5936f9347899 100644
--- a/libstdc++-v3/include/std/shared_mutex
+++ b/libstdc++-v3/include/std/shared_mutex
@@ -520,15 +520,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        try_lock_until(const chrono::time_point<chrono::system_clock,
  		     _Duration>& __atime)
        {
-	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
-	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
-
-	__gthread_time_t __ts =
-	  {
-	    static_cast<std::time_t>(__s.time_since_epoch().count()),
-	    static_cast<long>(__ns.count())
-	  };
-
+	struct timespec __ts = chrono::__to_timespec(__atime);
  	int __ret = __glibcxx_rwlock_timedwrlock(&_M_rwlock, &__ts);
  	// On self-deadlock, we just fail to acquire the lock.  Technically,
  	// the program violated the precondition.
@@ -546,15 +538,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        try_lock_until(const chrono::time_point<chrono::steady_clock,
  		   _Duration>& __atime)
        {
-	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
-	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
-
-	__gthread_time_t __ts =
-	  {
-	    static_cast<std::time_t>(__s.time_since_epoch().count()),
-	    static_cast<long>(__ns.count())
-	  };
-
+	struct timespec __ts = chrono::__to_timespec(__atime);
  	int __ret = pthread_rwlock_clockwrlock(&_M_rwlock, CLOCK_MONOTONIC,
  					       &__ts);
  	// On self-deadlock, we just fail to acquire the lock.  Technically,
@@ -596,14 +580,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        try_lock_shared_until(const chrono::time_point<chrono::system_clock,
  			    _Duration>& __atime)
        {
-	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
-	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
-
-	__gthread_time_t __ts =
-	  {
-	    static_cast<std::time_t>(__s.time_since_epoch().count()),
-	    static_cast<long>(__ns.count())
-	  };
+	struct timespec __ts = chrono::__to_timespec(__atime);
  
  	int __ret;
  	// Unlike for lock(), we are not allowed to throw an exception so if
@@ -636,15 +613,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        try_lock_shared_until(const chrono::time_point<chrono::steady_clock,
  			    _Duration>& __atime)
        {
-	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
-	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
-
-	__gthread_time_t __ts =
-	  {
-	    static_cast<std::time_t>(__s.time_since_epoch().count()),
-	    static_cast<long>(__ns.count())
-	  };
-
+	struct timespec __ts = chrono::__to_timespec(__atime);
  	int __ret = pthread_rwlock_clockrdlock(&_M_rwlock, CLOCK_MONOTONIC,
  					       &__ts);
  	// On self-deadlock, we just fail to acquire the lock.  Technically,
diff --git a/libstdc++-v3/src/c++11/thread.cc b/libstdc++-v3/src/c++11/thread.cc
index 6c2ec2978f88..0768a99d6741 100644
--- a/libstdc++-v3/src/c++11/thread.cc
+++ b/libstdc++-v3/src/c++11/thread.cc
@@ -231,10 +231,30 @@ namespace std _GLIBCXX_VISIBILITY(default)
  _GLIBCXX_BEGIN_NAMESPACE_VERSION
  namespace this_thread
  {
+namespace
+{
+  // returns min(s, Dur::max())
+  template<typename Dur>
+    inline chrono::seconds
+    limit(chrono::seconds s)
+    {
+      static_assert(ratio_equal<typename Dur::period, ratio<1>>::value,
+		    "period must be seconds to avoid potential overflow");
+
+      if (s > Dur::max()) [[__unlikely__]]
+	s = chrono::duration_cast<chrono::seconds>(Dur::max());
+      return s;
+    }
+}
+
    void
    __sleep_for(chrono::seconds __s, chrono::nanoseconds __ns)
    {
  #ifdef _GLIBCXX_USE_NANOSLEEP
+#pragma GCC diagnostic ignored "-Wc++17-extensions"
+    if constexpr (is_integral<time_t>::value) // POSIX.1-2001 allows floating
+      __s = limit<chrono::duration<time_t>>(__s);
+
      struct ::timespec __ts =
        {
  	static_cast<std::time_t>(__s.count()),
@@ -246,6 +266,8 @@ namespace this_thread
      const auto target = chrono::steady_clock::now() + __s + __ns;
      while (true)
        {
+	__s = limit<chrono::duration<unsigned>>(__s);
+
  	unsigned secs = __s.count();
  	if (__ns.count() > 0)
  	  {
@@ -271,11 +293,19 @@ namespace this_thread
  	  break;
  	__s = chrono::duration_cast<chrono::seconds>(target - now);
  	__ns = chrono::duration_cast<chrono::nanoseconds>(target - (now + __s));
-    }
+      }
  #elif defined(_GLIBCXX_USE_WIN32_SLEEP)
+    // Can't use limit<chrono::milliseconds>(__s) here because it would
+    // multiply __s by 1000 which could overflow.
+    auto max_ms = chrono::milliseconds::max() / 1000;
+    auto max_ms_in_s = chrono::duration_cast<chrono::seconds>(max_ms);
+    if (__s > max_ms_in_s)
+      __s = max_ms_in_s;
+
      unsigned long ms = __ns.count() / 1000000;
      if (__ns.count() > 0 && ms == 0)
        ms = 1;
+
      ::Sleep(chrono::milliseconds(__s).count() + ms);
  #endif
    }
diff --git a/libstdc++-v3/testsuite/30_threads/this_thread/113327.cc b/libstdc++-v3/testsuite/30_threads/this_thread/113327.cc
new file mode 100644
index 000000000000..2daa2b0e46e2
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/this_thread/113327.cc
@@ -0,0 +1,29 @@
+// { dg-do run { target c++11 } }
+// { dg-additional-options "-pthread" { target pthread } }
+// { dg-require-gthreads "" }
+
+// PR libstdc++/113327
+// std::sleep_for(std::chrono::hours::max()) returns immediately
+
+#include <thread>
+#include <chrono>
+#include <cstdlib>
+#include <csignal>
+
+int main()
+{
+  std::thread sleepy([] {
+    // Rather than overflowing to a negative value, the timeout should be
+    // truncated to seconds::max() and so sleep for 292 billion years.
+    std::this_thread::sleep_for(std::chrono::minutes::max());
+    // This should not happen:
+    throw 1;
+  });
+  // Give the new thread a chance to start sleeping:
+  std::this_thread::yield();
+  std::this_thread::sleep_for(std::chrono::seconds(2));
+  // If we get here without the other thread throwing an exception
+  // then it should be sleeping peacefully, so the test passed.
+  // pthread_kill(sleepy.native_handle(), SIGINT);
+  std::_Exit(0);
+}
  

On Wednesday 08 October 2025 at 12:27:06 +0100, Jonathan Wakely wrote:
> A few months ago I was testing the patch below, which replaces all the
> repeated code that creates timespec objects with a call to a single
> function. That function avoids overflows, e.g. if a timeout of
> chrono::years::max() is used, and also turns negative timeouts into
> the epoch.
>
> This avoids solving the same problems in several places across the
> library, like the four places above in <shared_mutex>.
> 
> I got distracted by other fixes and never submitted this patch to the
> mailing list (we were in the final stages of gcc 15 when I last worked
> on it). I think it's a good approach but would appreciate your review.

In general I think that this is a great improvement. Consolidating this
kind of code that needs extreme care to get right makes a lot of sense.

> I think what I'll do is apply your patches locally on top of this one
> (resolving the conflicts) and test everything again.

Please let me know if you'd like me to have a go at doing that.

Do you know of any platforms that actually have a floating-point time_t? I
didn't realise that was possible until today and it looks like it was only
really permitted in the hope of being useful and never really materialised
according to https://www.austingroupbugs.net/view.php?id=327 .

I've made a few minor comments on the patch itself below.

> commit 27a12d66c96ebc107cc997faa63372392436de33
> Author:     Jonathan Wakely <jwakely@redhat.com>
> AuthorDate: Wed Apr 30 10:58:51 2025
> Commit:     Jonathan Wakely <redi@gcc.gnu.org>
> CommitDate: Mon May 19 14:25:49 2025
> 
>     libstdc++: Avoid overflow in timeout conversions [PR113327]
>     When converting from a coarse duration with a very large value, the
>     existing code scales that up to chrono::seconds which overflows the
>     chrono::seconds::rep type. For example, sleep_for(chrono::hours::max())
>     tries to calculate LLONG_MAX * 3600, which overflows to -3600 and so the
>     sleep returns immediately.
>     The solution in this commit is inspired by this_thread::sleep_for in
>     libc++ which compares the duration argument to
>     chrono::duration<long double>(nanoseconds::max()) and limits the
>     duration to nanoseconds::max(). Because we split the duration into
>     seconds and nanoseconds, we can use seconds::max() as our upper limit.
>     We might need to limit further if seconds::max() doesn't fit in the
>     type used for sleeping, which is one of std::time_t, unsigned int, or
>     chrono::milliseconds.
>     libstdc++-v3/ChangeLog:
>             PR libstdc++/113327
>             PR libstdc++/119258
>             PR libstdc++/58931
>             * include/bits/chrono.h (__to_timespec): New overloaded function
>             templates for converting chrono types to timespec.
>             (__to_gthread_time_t): New function template for converting
>             time_point to __gthread_time_t.
>             * include/bits/this_thread_sleep.h (sleep_for): Use
>             __to_timespec.
>             (__sleep_for): Remove namespace-scope declaration.
>             * include/bits/atomic_timed_wait.h: Use __to_timespec and
>             __to_gthread_time_t for timeouts.
>             * include/std/condition_variable: Likewise.
>             * include/std/mutex: Likewise.
>             * include/std/shared_mutex: Likewise.
>             * src/c++11/thread.cc (limit): New helper function.
>             (__sleep_for): Use limit to prevent overflow when converting
>             chrono::seconds to time_t, unsigned, or chrono::milliseconds.
>             * testsuite/30_threads/this_thread/113327.cc: New test.
> 
> diff --git a/libstdc++-v3/include/bits/atomic_timed_wait.h b/libstdc++-v3/include/bits/atomic_timed_wait.h
> index 9a6ac95b7d0e..9b915d11f672 100644
> --- a/libstdc++-v3/include/bits/atomic_timed_wait.h
> +++ b/libstdc++-v3/include/bits/atomic_timed_wait.h
> @@ -83,14 +83,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  				 const chrono::time_point<__wait_clock_t, _Dur>&
>  				      __atime) noexcept
>        {
> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
> -
> -	struct timespec __rt =
> -	{
> -	  static_cast<std::time_t>(__s.time_since_epoch().count()),
> -	  static_cast<long>(__ns.count())
> -	};
> +	struct timespec __rt = chrono::__to_timespec(__atime);
>  	auto __e = syscall (SYS_futex, __addr,
>  			    static_cast<int>(__futex_wait_flags::
> @@ -150,14 +143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	static_assert(std::__is_one_of<_Clock, chrono::steady_clock,
>  					       chrono::system_clock>::value);
> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
> -
> -	__gthread_time_t __ts =
> -	  {
> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
> -	    static_cast<long>(__ns.count())
> -	  };
> +	__gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
>  #ifdef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT
>  	if constexpr (is_same_v<chrono::steady_clock, _Clock>)
> diff --git a/libstdc++-v3/include/bits/chrono.h b/libstdc++-v3/include/bits/chrono.h
> index fad216203d2f..55becc4d9aef 100644
> --- a/libstdc++-v3/include/bits/chrono.h
> +++ b/libstdc++-v3/include/bits/chrono.h
> @@ -50,6 +50,9 @@
>  #include <bits/version.h>
> +// TODO move __to_gthread_time_t to a better place
> +#include <bits/gthr.h> // for __gthread_time_t +
>  namespace std _GLIBCXX_VISIBILITY(default)
>  {
>  _GLIBCXX_BEGIN_NAMESPACE_VERSION
> @@ -1513,6 +1516,98 @@ _GLIBCXX_END_INLINE_ABI_NAMESPACE(_V2)
>    } // namespace filesystem
>  #endif // C++17 && HOSTED
> +#if defined _GLIBCXX_USE_NANOSLEEP || defined _GLIBCXX_USE_CLOCK_REALTIME \
> +    || defined _GLIBCXX_HAS_GTHREADS
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wc++17-extensions"
> +namespace chrono
> +{
> +/// @cond undocumented
> +
> +  // Convert a chrono::duration to a relative time represented as timespec
> +  // (e.g. for use with nanosleep).
> +  template<typename _Rep, typename _Period>
> +    [[__nodiscard__]] _GLIBCXX14_CONSTEXPR inline
> +    struct ::timespec
> +    __to_timespec(const duration<_Rep, _Period>& __d)

IMO this function is really only useful for timeouts - the guaranteed
non-negative behaviour is required for that use case. Calling the function
__to_timeout_timespec() or __to_timespec_for_timeout() would make that
clearer and stop someone expecting the function to be more general.

> +    {
> +      struct ::timespec __ts{};
> +
> +      if (__d < __d.zero()) // Negative timeouts don't make sense.
> +	return __ts;
> +
> +      if constexpr (__or_<ratio_greater<_Period, ratio<1>>,
> +			  treat_as_floating_point<_Rep>>::value)
> +	{
> +	  // Converting from e.g. chrono::hours::max() to chrono::seconds
> +	  // would evaluate LLONG_MAX * 3600 which would overflow.
> +	  // Limit to chrono::seconds::max().
> +	  chrono::duration<double> __fmax(chrono::seconds::max());
> +	  if (__d > __fmax) [[__unlikely__]]
> +	    return chrono::__to_timespec(chrono::seconds::max());
> +	}
> +
> +      auto __s = chrono::duration_cast<chrono::seconds>(__d);
> +
> +      if constexpr (is_integral<time_t>::value) // POSIX.1-2001 allows floating
> +	{
> +	  // Also limit to time_t maximum (only relevant for 32-bit time_t).
> +	  constexpr auto __tmax = numeric_limits<time_t>::max();
> +	  if (__s.count() > __tmax) [[__unlikely__]]
> +	    {
> +	      __ts.tv_sec = __tmax;
> +	      return __ts;
> +	    }
> +	}
> +
> +      auto __ns = chrono::duration_cast<chrono::nanoseconds>(__d - __s);
> +
> +      if constexpr (treat_as_floating_point<_Rep>::value)
> +	if (__ns.count() > 999999999) [[__unlikely__]]
> +	  __ns = chrono::nanoseconds(999999999);

This probably doesn't make any real difference, but shouldn't this be:

 {
   ++__s;
   __ns = 0;
 }

to ensure that the timespec is at least as long as the provided duration?
(Though I suppose we then need to worry about __s potentially overflowing
too which is a pain for little gain.)

> +
> +      __ts.tv_sec = static_cast<time_t>(__s.count());
> +      __ts.tv_nsec = static_cast<long>(__ns.count());
> +      return __ts;
> +    }
> +
> +  // Convert a chrono::time_point to an absolute time represented as timespec.
> +  // All times before the epoch get converted to the epoch, so this assumes
> +  // that we only use it for clocks where that's true.
> +  // It should be safe to use this for system_clock and steady_clock.
> +  template<typename _Clock, typename _Dur>
> +    [[__nodiscard__]] _GLIBCXX14_CONSTEXPR inline
> +    struct ::timespec
> +    __to_timespec(const time_point<_Clock, _Dur>& __t)
> +    {
> +      return chrono::__to_timespec(__t.time_since_epoch());
> +    }
> +
> +#ifdef _GLIBCXX_HAS_GTHREADS
> +  // Convert a time_point to an absolute time represented as __gthread_time_t
> +  // (which is typically just a typedef for struct timespec).
> +  template<typename _Clock, typename _Dur>
> +    [[__nodiscard__]] _GLIBCXX14_CONSTEXPR inline
> +    __gthread_time_t
> +    __to_gthread_time_t(const time_point<_Clock, _Dur>& __t)

Similar comment to above for __to_timespec() about this only being useful
for timeouts and so naming it __to_gthread_timeout_time_t() or
__to_gthread_time_t_for_timeout(), though I admit that these names are
getting rather unwieldy.

> +    {
> +      auto __ts = chrono::__to_timespec(__t.time_since_epoch());
> +      if constexpr (is_same<::timespec, __gthread_time_t>::value)
> +	return __ts;
> +      else if constexpr (is_convertible<::timespec, __gthread_time_t>::value)
> +	return __ts;
> +      else if constexpr (is_scalar<__gthread_time_t>::value)
> +	return static_cast<__gthread_time_t>(__ts.tv_sec);
> +      else // Assume this works:
> +	return __gthread_time_t{ __ts.tv_sec, __ts.tv_nsec };
> +    }
> +#endif // HAS_GTHREADS
> +
> +/// @endcond
> +} // namespace chrono
> +#pragma GCC diagnostic pop
> +#endif // USE_NANOSLEEP || USE_CLOCK_REALTIME || HAS_GTHREADS
> +
>  _GLIBCXX_END_NAMESPACE_VERSION
>  } // namespace std
> diff --git a/libstdc++-v3/include/bits/this_thread_sleep.h b/libstdc++-v3/include/bits/this_thread_sleep.h
> index 57f89f858952..bf52aa9cd1c5 100644
> --- a/libstdc++-v3/include/bits/this_thread_sleep.h
> +++ b/libstdc++-v3/include/bits/this_thread_sleep.h
> @@ -36,6 +36,7 @@
>  #if __cplusplus >= 201103L
>  #include <bits/chrono.h> // std::chrono::*
> +#include <ext/numeric_traits.h> // __int_traits
>  #ifdef _GLIBCXX_USE_NANOSLEEP
>  # include <cerrno>  // errno, EINTR
> @@ -59,11 +60,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>    {
>  #ifndef _GLIBCXX_NO_SLEEP
> -#ifndef _GLIBCXX_USE_NANOSLEEP
> -    void
> -    __sleep_for(chrono::seconds, chrono::nanoseconds);
> -#endif
> -
>      /// this_thread::sleep_for
>      template<typename _Rep, typename _Period>
>        inline void
> @@ -71,18 +67,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        {
>  	if (__rtime <= __rtime.zero())
>  	  return;
> -	auto __s = chrono::duration_cast<chrono::seconds>(__rtime);
> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__rtime - __s);
> +
> +	struct timespec __ts = chrono::__to_timespec(__rtime);
>  #ifdef _GLIBCXX_USE_NANOSLEEP
> -	struct ::timespec __ts =
> -	  {
> -	    static_cast<std::time_t>(__s.count()),
> -	    static_cast<long>(__ns.count())
> -	  };
>  	while (::nanosleep(&__ts, &__ts) == -1 && errno == EINTR)
>  	  { }
>  #else
> -	__sleep_for(__s, __ns);
> +	using chrono::seconds;
> +	using chrono::nanoseconds;
> +	void __sleep_for(seconds __s, nanoseconds __ns);

Could we just make __sleep_for() take a single nanoseconds parameter so
that its implementation can just call __to_timespec()? Perhaps you'd need
to keep the old two-parameter version for ABI compatibility anyway so it's
not worth it?

> +	__sleep_for(seconds(__ts.tv_sec), nanoseconds(__ts.tv_nsec));
>  #endif
>        }
> diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable
> index 3525ff35ba31..cbc399493db3 100644
> --- a/libstdc++-v3/include/std/condition_variable
> +++ b/libstdc++-v3/include/std/condition_variable
> @@ -193,15 +193,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        __wait_until_impl(unique_lock<mutex>& __lock,
>  			const chrono::time_point<steady_clock, _Dur>& __atime)
>        {
> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
> -
> -	__gthread_time_t __ts =
> -	  {
> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
> -	    static_cast<long>(__ns.count())
> -	  };
> -
> +	__gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
>  	_M_cond.wait_until(*__lock.mutex(), CLOCK_MONOTONIC, __ts);
>  	return (steady_clock::now() < __atime
> @@ -214,15 +206,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        __wait_until_impl(unique_lock<mutex>& __lock,
>  			const chrono::time_point<system_clock, _Dur>& __atime)
>        {
> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
> -
> -	__gthread_time_t __ts =
> -	  {
> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
> -	    static_cast<long>(__ns.count())
> -	  };
> -
> +	__gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
>  	_M_cond.wait_until(*__lock.mutex(), __ts);
>  	return (system_clock::now() < __atime
> diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
> index e575a81c138e..a4624b475411 100644
> --- a/libstdc++-v3/include/std/mutex
> +++ b/libstdc++-v3/include/std/mutex
> @@ -179,14 +179,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	_M_try_lock_until(const chrono::time_point<chrono::system_clock,
>  						   _Duration>& __atime)
>  	{
> -	  auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
> -	  auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
> -
> -	  __gthread_time_t __ts = {
> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
> -	    static_cast<long>(__ns.count())
> -	  };
> -
> +	  __gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
>  	  return static_cast<_Derived*>(this)->_M_timedlock(__ts);
>  	}
> @@ -196,14 +189,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	_M_try_lock_until(const chrono::time_point<chrono::steady_clock,
>  						   _Duration>& __atime)
>  	{
> -	  auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
> -	  auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
> -
> -	  __gthread_time_t __ts = {
> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
> -	    static_cast<long>(__ns.count())
> -	  };
> -
> +	  __gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
>  	  return static_cast<_Derived*>(this)->_M_clocklock(CLOCK_MONOTONIC,
>  							    __ts);
>  	}
> diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex
> index 94c8532399d9..5936f9347899 100644
> --- a/libstdc++-v3/include/std/shared_mutex
> +++ b/libstdc++-v3/include/std/shared_mutex
> @@ -520,15 +520,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        try_lock_until(const chrono::time_point<chrono::system_clock,
>  		     _Duration>& __atime)
>        {
> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
> -
> -	__gthread_time_t __ts =
> -	  {
> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
> -	    static_cast<long>(__ns.count())
> -	  };
> -
> +	struct timespec __ts = chrono::__to_timespec(__atime);
>  	int __ret = __glibcxx_rwlock_timedwrlock(&_M_rwlock, &__ts);
>  	// On self-deadlock, we just fail to acquire the lock.  Technically,
>  	// the program violated the precondition.
> @@ -546,15 +538,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        try_lock_until(const chrono::time_point<chrono::steady_clock,
>  		   _Duration>& __atime)
>        {
> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
> -
> -	__gthread_time_t __ts =
> -	  {
> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
> -	    static_cast<long>(__ns.count())
> -	  };
> -
> +	struct timespec __ts = chrono::__to_timespec(__atime);
>  	int __ret = pthread_rwlock_clockwrlock(&_M_rwlock, CLOCK_MONOTONIC,
>  					       &__ts);
>  	// On self-deadlock, we just fail to acquire the lock.  Technically,
> @@ -596,14 +580,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        try_lock_shared_until(const chrono::time_point<chrono::system_clock,
>  			    _Duration>& __atime)
>        {
> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
> -
> -	__gthread_time_t __ts =
> -	  {
> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
> -	    static_cast<long>(__ns.count())
> -	  };
> +	struct timespec __ts = chrono::__to_timespec(__atime);
>  	int __ret;
>  	// Unlike for lock(), we are not allowed to throw an exception so if
> @@ -636,15 +613,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        try_lock_shared_until(const chrono::time_point<chrono::steady_clock,
>  			    _Duration>& __atime)
>        {
> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
> -
> -	__gthread_time_t __ts =
> -	  {
> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
> -	    static_cast<long>(__ns.count())
> -	  };
> -
> +	struct timespec __ts = chrono::__to_timespec(__atime);
>  	int __ret = pthread_rwlock_clockrdlock(&_M_rwlock, CLOCK_MONOTONIC,
>  					       &__ts);
>  	// On self-deadlock, we just fail to acquire the lock.  Technically,
> diff --git a/libstdc++-v3/src/c++11/thread.cc b/libstdc++-v3/src/c++11/thread.cc
> index 6c2ec2978f88..0768a99d6741 100644
> --- a/libstdc++-v3/src/c++11/thread.cc
> +++ b/libstdc++-v3/src/c++11/thread.cc
> @@ -231,10 +231,30 @@ namespace std _GLIBCXX_VISIBILITY(default)
>  _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  namespace this_thread
>  {
> +namespace
> +{
> +  // returns min(s, Dur::max())
> +  template<typename Dur>
> +    inline chrono::seconds
> +    limit(chrono::seconds s)
> +    {
> +      static_assert(ratio_equal<typename Dur::period, ratio<1>>::value,
> +		    "period must be seconds to avoid potential overflow");
> +
> +      if (s > Dur::max()) [[__unlikely__]]
> +	s = chrono::duration_cast<chrono::seconds>(Dur::max());
> +      return s;
> +    }
> +}
> +
>    void
>    __sleep_for(chrono::seconds __s, chrono::nanoseconds __ns)
>    {
>  #ifdef _GLIBCXX_USE_NANOSLEEP
> +#pragma GCC diagnostic ignored "-Wc++17-extensions"
> +    if constexpr (is_integral<time_t>::value) // POSIX.1-2001 allows floating
> +      __s = limit<chrono::duration<time_t>>(__s);
> +
>      struct ::timespec __ts =
>        {
>  	static_cast<std::time_t>(__s.count()),
> @@ -246,6 +266,8 @@ namespace this_thread
>      const auto target = chrono::steady_clock::now() + __s + __ns;
>      while (true)
>        {
> +	__s = limit<chrono::duration<unsigned>>(__s);
> +
>  	unsigned secs = __s.count();
>  	if (__ns.count() > 0)
>  	  {
> @@ -271,11 +293,19 @@ namespace this_thread
>  	  break;
>  	__s = chrono::duration_cast<chrono::seconds>(target - now);
>  	__ns = chrono::duration_cast<chrono::nanoseconds>(target - (now + __s));
> -    }
> +      }
>  #elif defined(_GLIBCXX_USE_WIN32_SLEEP)
> +    // Can't use limit<chrono::milliseconds>(__s) here because it would
> +    // multiply __s by 1000 which could overflow.

My initial reaction is that it ought to be possible to make limit() cope
with this but I haven't tried so I'm probably wrong. :)

> +    auto max_ms = chrono::milliseconds::max() / 1000;
> +    auto max_ms_in_s = chrono::duration_cast<chrono::seconds>(max_ms);
> +    if (__s > max_ms_in_s)
> +      __s = max_ms_in_s;
> +
>      unsigned long ms = __ns.count() / 1000000;
>      if (__ns.count() > 0 && ms == 0)
>        ms = 1;
> +
>      ::Sleep(chrono::milliseconds(__s).count() + ms);

Shouldn't there be a loop around this so that we're guaranteed to wait for
at least the requested duration even if __s is too large to be represented
as milliseconds (at the cost of waking up every ~25 days)?

>  #endif
>    }
> diff --git a/libstdc++-v3/testsuite/30_threads/this_thread/113327.cc b/libstdc++-v3/testsuite/30_threads/this_thread/113327.cc
> new file mode 100644
> index 000000000000..2daa2b0e46e2
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/30_threads/this_thread/113327.cc
> @@ -0,0 +1,29 @@
> +// { dg-do run { target c++11 } }
> +// { dg-additional-options "-pthread" { target pthread } }
> +// { dg-require-gthreads "" }
> +
> +// PR libstdc++/113327
> +// std::sleep_for(std::chrono::hours::max()) returns immediately
> +
> +#include <thread>
> +#include <chrono>
> +#include <cstdlib>
> +#include <csignal>
> +
> +int main()
> +{
> +  std::thread sleepy([] {
> +    // Rather than overflowing to a negative value, the timeout should be
> +    // truncated to seconds::max() and so sleep for 292 billion years.
> +    std::this_thread::sleep_for(std::chrono::minutes::max());
> +    // This should not happen:
> +    throw 1;
> +  });
> +  // Give the new thread a chance to start sleeping:
> +  std::this_thread::yield();
> +  std::this_thread::sleep_for(std::chrono::seconds(2));
> +  // If we get here without the other thread throwing an exception
> +  // then it should be sleeping peacefully, so the test passed.
> +  // pthread_kill(sleepy.native_handle(), SIGINT);
> +  std::_Exit(0);
> +}

Thanks again for looking at this.

Mike.
  

On Wed, 08 Oct 2025 at 21:21 +0100, Mike Crowe wrote:
>On Wednesday 08 October 2025 at 12:27:06 +0100, Jonathan Wakely wrote:
>> A few months ago I was testing the patch below, which replaces all the
>> repeated code that creates timespec objects with a call to a single
>> function. That function avoids overflows, e.g. if a timeout of
>> chrono::years::max() is used, and also turns negative timeouts into
>> the epoch.
>>
>> This avoids solving the same problems in several places across the
>> library, like the four places above in <shared_mutex>.
>>
>> I got distracted by other fixes and never submitted this patch to the
>> mailing list (we were in the final stages of gcc 15 when I last worked
>> on it). I think it's a good approach but would appreciate your review.
>
>In general I think that this is a great improvement. Consolidating this
>kind of code that needs extreme care to get right makes a lot of sense.
>
>> I think what I'll do is apply your patches locally on top of this one
>> (resolving the conflicts) and test everything again.
>
>Please let me know if you'd like me to have a go at doing that.

Don't worry, I can do that. I think only the patch for shared_mutex
will cause any conflicts, and they'll be trivial to resolve.

>Do you know of any platforms that actually have a floating-point time_t? I
>didn't realise that was possible until today and it looks like it was only
>really permitted in the hope of being useful and never really materialised
>according to https://www.austingroupbugs.net/view.php?id=327 .

Yeah, it's hypothetical as far as I know. But we're not just a POSIX
library, so it's possible that somebody will try to port libstdc++ to
an OS with a floating-point time_t one day. Unlikely though, so we
could just drop that part of the patch and maybe just leave a comment
saying "if your OS is weird, you need to make changes here".

>I've made a few minor comments on the patch itself below.

Thanks for the review, replies inline below.


>> commit 27a12d66c96ebc107cc997faa63372392436de33
>> Author:     Jonathan Wakely <jwakely@redhat.com>
>> AuthorDate: Wed Apr 30 10:58:51 2025
>> Commit:     Jonathan Wakely <redi@gcc.gnu.org>
>> CommitDate: Mon May 19 14:25:49 2025
>>
>>     libstdc++: Avoid overflow in timeout conversions [PR113327]
>>     When converting from a coarse duration with a very large value, the
>>     existing code scales that up to chrono::seconds which overflows the
>>     chrono::seconds::rep type. For example, sleep_for(chrono::hours::max())
>>     tries to calculate LLONG_MAX * 3600, which overflows to -3600 and so the
>>     sleep returns immediately.
>>     The solution in this commit is inspired by this_thread::sleep_for in
>>     libc++ which compares the duration argument to
>>     chrono::duration<long double>(nanoseconds::max()) and limits the
>>     duration to nanoseconds::max(). Because we split the duration into
>>     seconds and nanoseconds, we can use seconds::max() as our upper limit.
>>     We might need to limit further if seconds::max() doesn't fit in the
>>     type used for sleeping, which is one of std::time_t, unsigned int, or
>>     chrono::milliseconds.
>>     libstdc++-v3/ChangeLog:
>>             PR libstdc++/113327
>>             PR libstdc++/119258
>>             PR libstdc++/58931
>>             * include/bits/chrono.h (__to_timespec): New overloaded function
>>             templates for converting chrono types to timespec.
>>             (__to_gthread_time_t): New function template for converting
>>             time_point to __gthread_time_t.
>>             * include/bits/this_thread_sleep.h (sleep_for): Use
>>             __to_timespec.
>>             (__sleep_for): Remove namespace-scope declaration.
>>             * include/bits/atomic_timed_wait.h: Use __to_timespec and
>>             __to_gthread_time_t for timeouts.
>>             * include/std/condition_variable: Likewise.
>>             * include/std/mutex: Likewise.
>>             * include/std/shared_mutex: Likewise.
>>             * src/c++11/thread.cc (limit): New helper function.
>>             (__sleep_for): Use limit to prevent overflow when converting
>>             chrono::seconds to time_t, unsigned, or chrono::milliseconds.
>>             * testsuite/30_threads/this_thread/113327.cc: New test.
>>
>> diff --git a/libstdc++-v3/include/bits/atomic_timed_wait.h b/libstdc++-v3/include/bits/atomic_timed_wait.h
>> index 9a6ac95b7d0e..9b915d11f672 100644
>> --- a/libstdc++-v3/include/bits/atomic_timed_wait.h
>> +++ b/libstdc++-v3/include/bits/atomic_timed_wait.h
>> @@ -83,14 +83,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>  				 const chrono::time_point<__wait_clock_t, _Dur>&
>>  				      __atime) noexcept
>>        {
>> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
>> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
>> -
>> -	struct timespec __rt =
>> -	{
>> -	  static_cast<std::time_t>(__s.time_since_epoch().count()),
>> -	  static_cast<long>(__ns.count())
>> -	};
>> +	struct timespec __rt = chrono::__to_timespec(__atime);
>>  	auto __e = syscall (SYS_futex, __addr,
>>  			    static_cast<int>(__futex_wait_flags::
>> @@ -150,14 +143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>  	static_assert(std::__is_one_of<_Clock, chrono::steady_clock,
>>  					       chrono::system_clock>::value);
>> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
>> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
>> -
>> -	__gthread_time_t __ts =
>> -	  {
>> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
>> -	    static_cast<long>(__ns.count())
>> -	  };
>> +	__gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
>>  #ifdef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT
>>  	if constexpr (is_same_v<chrono::steady_clock, _Clock>)
>> diff --git a/libstdc++-v3/include/bits/chrono.h b/libstdc++-v3/include/bits/chrono.h
>> index fad216203d2f..55becc4d9aef 100644
>> --- a/libstdc++-v3/include/bits/chrono.h
>> +++ b/libstdc++-v3/include/bits/chrono.h
>> @@ -50,6 +50,9 @@
>>  #include <bits/version.h>
>> +// TODO move __to_gthread_time_t to a better place
>> +#include <bits/gthr.h> // for __gthread_time_t +
>>  namespace std _GLIBCXX_VISIBILITY(default)
>>  {
>>  _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> @@ -1513,6 +1516,98 @@ _GLIBCXX_END_INLINE_ABI_NAMESPACE(_V2)
>>    } // namespace filesystem
>>  #endif // C++17 && HOSTED
>> +#if defined _GLIBCXX_USE_NANOSLEEP || defined _GLIBCXX_USE_CLOCK_REALTIME \
>> +    || defined _GLIBCXX_HAS_GTHREADS
>> +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Wc++17-extensions"
>> +namespace chrono
>> +{
>> +/// @cond undocumented
>> +
>> +  // Convert a chrono::duration to a relative time represented as timespec
>> +  // (e.g. for use with nanosleep).
>> +  template<typename _Rep, typename _Period>
>> +    [[__nodiscard__]] _GLIBCXX14_CONSTEXPR inline
>> +    struct ::timespec
>> +    __to_timespec(const duration<_Rep, _Period>& __d)
>
>IMO this function is really only useful for timeouts - the guaranteed
>non-negative behaviour is required for that use case. Calling the function
>__to_timeout_timespec() or __to_timespec_for_timeout() would make that
>clearer and stop someone expecting the function to be more general.

Good idea.

>> +    {
>> +      struct ::timespec __ts{};
>> +
>> +      if (__d < __d.zero()) // Negative timeouts don't make sense.
>> +	return __ts;
>> +
>> +      if constexpr (__or_<ratio_greater<_Period, ratio<1>>,
>> +			  treat_as_floating_point<_Rep>>::value)
>> +	{
>> +	  // Converting from e.g. chrono::hours::max() to chrono::seconds
>> +	  // would evaluate LLONG_MAX * 3600 which would overflow.
>> +	  // Limit to chrono::seconds::max().
>> +	  chrono::duration<double> __fmax(chrono::seconds::max());
>> +	  if (__d > __fmax) [[__unlikely__]]
>> +	    return chrono::__to_timespec(chrono::seconds::max());
>> +	}
>> +
>> +      auto __s = chrono::duration_cast<chrono::seconds>(__d);
>> +
>> +      if constexpr (is_integral<time_t>::value) // POSIX.1-2001 allows floating
>> +	{
>> +	  // Also limit to time_t maximum (only relevant for 32-bit time_t).
>> +	  constexpr auto __tmax = numeric_limits<time_t>::max();
>> +	  if (__s.count() > __tmax) [[__unlikely__]]
>> +	    {
>> +	      __ts.tv_sec = __tmax;
>> +	      return __ts;
>> +	    }
>> +	}
>> +
>> +      auto __ns = chrono::duration_cast<chrono::nanoseconds>(__d - __s);
>> +
>> +      if constexpr (treat_as_floating_point<_Rep>::value)
>> +	if (__ns.count() > 999999999) [[__unlikely__]]
>> +	  __ns = chrono::nanoseconds(999999999);
>
>This probably doesn't make any real difference, but shouldn't this be:
>
> {
>   ++__s;
>   __ns = 0;
> }
>
>to ensure that the timespec is at least as long as the provided duration?
>(Though I suppose we then need to worry about __s potentially overflowing
>too which is a pain for little gain.)

It's been a few months now, but I think my thinking was that we could
only get a value larger than 999'999'999 from rounding, and that if it
was _really_ larger than that, then the duration_cast<seconds>(d)
should have already give us a larger value for s.

I don't think it's possible to get a value here that's significantly
larger than 999'999'999, only a few nanos larger, and so just
truncating to 999'999'999 should be fine (the kernel will return
EINVAL if you pass a tv_nsec larger than that. which is why I bothered
to clamp it here at all).

>> +
>> +      __ts.tv_sec = static_cast<time_t>(__s.count());
>> +      __ts.tv_nsec = static_cast<long>(__ns.count());
>> +      return __ts;
>> +    }
>> +
>> +  // Convert a chrono::time_point to an absolute time represented as timespec.
>> +  // All times before the epoch get converted to the epoch, so this assumes
>> +  // that we only use it for clocks where that's true.
>> +  // It should be safe to use this for system_clock and steady_clock.
>> +  template<typename _Clock, typename _Dur>
>> +    [[__nodiscard__]] _GLIBCXX14_CONSTEXPR inline
>> +    struct ::timespec
>> +    __to_timespec(const time_point<_Clock, _Dur>& __t)
>> +    {
>> +      return chrono::__to_timespec(__t.time_since_epoch());
>> +    }
>> +
>> +#ifdef _GLIBCXX_HAS_GTHREADS
>> +  // Convert a time_point to an absolute time represented as __gthread_time_t
>> +  // (which is typically just a typedef for struct timespec).
>> +  template<typename _Clock, typename _Dur>
>> +    [[__nodiscard__]] _GLIBCXX14_CONSTEXPR inline
>> +    __gthread_time_t
>> +    __to_gthread_time_t(const time_point<_Clock, _Dur>& __t)
>
>Similar comment to above for __to_timespec() about this only being useful
>for timeouts and so naming it __to_gthread_timeout_time_t() or
>__to_gthread_time_t_for_timeout(), though I admit that these names are
>getting rather unwieldy.

We don't need to use them too often, and they're not meant to be
public APIs that users would call. I think clear names are better.

>> +    {
>> +      auto __ts = chrono::__to_timespec(__t.time_since_epoch());
>> +      if constexpr (is_same<::timespec, __gthread_time_t>::value)
>> +	return __ts;
>> +      else if constexpr (is_convertible<::timespec, __gthread_time_t>::value)
>> +	return __ts;
>> +      else if constexpr (is_scalar<__gthread_time_t>::value)
>> +	return static_cast<__gthread_time_t>(__ts.tv_sec);
>> +      else // Assume this works:
>> +	return __gthread_time_t{ __ts.tv_sec, __ts.tv_nsec };
>> +    }
>> +#endif // HAS_GTHREADS
>> +
>> +/// @endcond
>> +} // namespace chrono
>> +#pragma GCC diagnostic pop
>> +#endif // USE_NANOSLEEP || USE_CLOCK_REALTIME || HAS_GTHREADS
>> +
>>  _GLIBCXX_END_NAMESPACE_VERSION
>>  } // namespace std
>> diff --git a/libstdc++-v3/include/bits/this_thread_sleep.h b/libstdc++-v3/include/bits/this_thread_sleep.h
>> index 57f89f858952..bf52aa9cd1c5 100644
>> --- a/libstdc++-v3/include/bits/this_thread_sleep.h
>> +++ b/libstdc++-v3/include/bits/this_thread_sleep.h
>> @@ -36,6 +36,7 @@
>>  #if __cplusplus >= 201103L
>>  #include <bits/chrono.h> // std::chrono::*
>> +#include <ext/numeric_traits.h> // __int_traits
>>  #ifdef _GLIBCXX_USE_NANOSLEEP
>>  # include <cerrno>  // errno, EINTR
>> @@ -59,11 +60,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>    {
>>  #ifndef _GLIBCXX_NO_SLEEP
>> -#ifndef _GLIBCXX_USE_NANOSLEEP
>> -    void
>> -    __sleep_for(chrono::seconds, chrono::nanoseconds);
>> -#endif
>> -
>>      /// this_thread::sleep_for
>>      template<typename _Rep, typename _Period>
>>        inline void
>> @@ -71,18 +67,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>        {
>>  	if (__rtime <= __rtime.zero())
>>  	  return;
>> -	auto __s = chrono::duration_cast<chrono::seconds>(__rtime);
>> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__rtime - __s);
>> +
>> +	struct timespec __ts = chrono::__to_timespec(__rtime);
>>  #ifdef _GLIBCXX_USE_NANOSLEEP
>> -	struct ::timespec __ts =
>> -	  {
>> -	    static_cast<std::time_t>(__s.count()),
>> -	    static_cast<long>(__ns.count())
>> -	  };
>>  	while (::nanosleep(&__ts, &__ts) == -1 && errno == EINTR)
>>  	  { }
>>  #else
>> -	__sleep_for(__s, __ns);
>> +	using chrono::seconds;
>> +	using chrono::nanoseconds;
>> +	void __sleep_for(seconds __s, nanoseconds __ns);
>
>Could we just make __sleep_for() take a single nanoseconds parameter so
>that its implementation can just call __to_timespec()? Perhaps you'd need
>to keep the old two-parameter version for ABI compatibility anyway so it's
>not worth it?

Yes, we'd need to keep it anyway. Since we can't remove it, I didn't
think about adding an alternative to use instead.

>> +	__sleep_for(seconds(__ts.tv_sec), nanoseconds(__ts.tv_nsec));
>>  #endif
>>        }
>> diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable
>> index 3525ff35ba31..cbc399493db3 100644
>> --- a/libstdc++-v3/include/std/condition_variable
>> +++ b/libstdc++-v3/include/std/condition_variable
>> @@ -193,15 +193,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>        __wait_until_impl(unique_lock<mutex>& __lock,
>>  			const chrono::time_point<steady_clock, _Dur>& __atime)
>>        {
>> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
>> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
>> -
>> -	__gthread_time_t __ts =
>> -	  {
>> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
>> -	    static_cast<long>(__ns.count())
>> -	  };
>> -
>> +	__gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
>>  	_M_cond.wait_until(*__lock.mutex(), CLOCK_MONOTONIC, __ts);
>>  	return (steady_clock::now() < __atime
>> @@ -214,15 +206,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>        __wait_until_impl(unique_lock<mutex>& __lock,
>>  			const chrono::time_point<system_clock, _Dur>& __atime)
>>        {
>> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
>> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
>> -
>> -	__gthread_time_t __ts =
>> -	  {
>> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
>> -	    static_cast<long>(__ns.count())
>> -	  };
>> -
>> +	__gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
>>  	_M_cond.wait_until(*__lock.mutex(), __ts);
>>  	return (system_clock::now() < __atime
>> diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
>> index e575a81c138e..a4624b475411 100644
>> --- a/libstdc++-v3/include/std/mutex
>> +++ b/libstdc++-v3/include/std/mutex
>> @@ -179,14 +179,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>  	_M_try_lock_until(const chrono::time_point<chrono::system_clock,
>>  						   _Duration>& __atime)
>>  	{
>> -	  auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
>> -	  auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
>> -
>> -	  __gthread_time_t __ts = {
>> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
>> -	    static_cast<long>(__ns.count())
>> -	  };
>> -
>> +	  __gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
>>  	  return static_cast<_Derived*>(this)->_M_timedlock(__ts);
>>  	}
>> @@ -196,14 +189,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>  	_M_try_lock_until(const chrono::time_point<chrono::steady_clock,
>>  						   _Duration>& __atime)
>>  	{
>> -	  auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
>> -	  auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
>> -
>> -	  __gthread_time_t __ts = {
>> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
>> -	    static_cast<long>(__ns.count())
>> -	  };
>> -
>> +	  __gthread_time_t __ts = chrono::__to_gthread_time_t(__atime);
>>  	  return static_cast<_Derived*>(this)->_M_clocklock(CLOCK_MONOTONIC,
>>  							    __ts);
>>  	}
>> diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex
>> index 94c8532399d9..5936f9347899 100644
>> --- a/libstdc++-v3/include/std/shared_mutex
>> +++ b/libstdc++-v3/include/std/shared_mutex
>> @@ -520,15 +520,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>        try_lock_until(const chrono::time_point<chrono::system_clock,
>>  		     _Duration>& __atime)
>>        {
>> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
>> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
>> -
>> -	__gthread_time_t __ts =
>> -	  {
>> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
>> -	    static_cast<long>(__ns.count())
>> -	  };
>> -
>> +	struct timespec __ts = chrono::__to_timespec(__atime);
>>  	int __ret = __glibcxx_rwlock_timedwrlock(&_M_rwlock, &__ts);
>>  	// On self-deadlock, we just fail to acquire the lock.  Technically,
>>  	// the program violated the precondition.
>> @@ -546,15 +538,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>        try_lock_until(const chrono::time_point<chrono::steady_clock,
>>  		   _Duration>& __atime)
>>        {
>> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
>> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
>> -
>> -	__gthread_time_t __ts =
>> -	  {
>> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
>> -	    static_cast<long>(__ns.count())
>> -	  };
>> -
>> +	struct timespec __ts = chrono::__to_timespec(__atime);
>>  	int __ret = pthread_rwlock_clockwrlock(&_M_rwlock, CLOCK_MONOTONIC,
>>  					       &__ts);
>>  	// On self-deadlock, we just fail to acquire the lock.  Technically,
>> @@ -596,14 +580,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>        try_lock_shared_until(const chrono::time_point<chrono::system_clock,
>>  			    _Duration>& __atime)
>>        {
>> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
>> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
>> -
>> -	__gthread_time_t __ts =
>> -	  {
>> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
>> -	    static_cast<long>(__ns.count())
>> -	  };
>> +	struct timespec __ts = chrono::__to_timespec(__atime);
>>  	int __ret;
>>  	// Unlike for lock(), we are not allowed to throw an exception so if
>> @@ -636,15 +613,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>        try_lock_shared_until(const chrono::time_point<chrono::steady_clock,
>>  			    _Duration>& __atime)
>>        {
>> -	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
>> -	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
>> -
>> -	__gthread_time_t __ts =
>> -	  {
>> -	    static_cast<std::time_t>(__s.time_since_epoch().count()),
>> -	    static_cast<long>(__ns.count())
>> -	  };
>> -
>> +	struct timespec __ts = chrono::__to_timespec(__atime);
>>  	int __ret = pthread_rwlock_clockrdlock(&_M_rwlock, CLOCK_MONOTONIC,
>>  					       &__ts);
>>  	// On self-deadlock, we just fail to acquire the lock.  Technically,
>> diff --git a/libstdc++-v3/src/c++11/thread.cc b/libstdc++-v3/src/c++11/thread.cc
>> index 6c2ec2978f88..0768a99d6741 100644
>> --- a/libstdc++-v3/src/c++11/thread.cc
>> +++ b/libstdc++-v3/src/c++11/thread.cc
>> @@ -231,10 +231,30 @@ namespace std _GLIBCXX_VISIBILITY(default)
>>  _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>  namespace this_thread
>>  {
>> +namespace
>> +{
>> +  // returns min(s, Dur::max())
>> +  template<typename Dur>
>> +    inline chrono::seconds
>> +    limit(chrono::seconds s)
>> +    {
>> +      static_assert(ratio_equal<typename Dur::period, ratio<1>>::value,
>> +		    "period must be seconds to avoid potential overflow");
>> +
>> +      if (s > Dur::max()) [[__unlikely__]]
>> +	s = chrono::duration_cast<chrono::seconds>(Dur::max());
>> +      return s;
>> +    }
>> +}
>> +
>>    void
>>    __sleep_for(chrono::seconds __s, chrono::nanoseconds __ns)
>>    {
>>  #ifdef _GLIBCXX_USE_NANOSLEEP
>> +#pragma GCC diagnostic ignored "-Wc++17-extensions"
>> +    if constexpr (is_integral<time_t>::value) // POSIX.1-2001 allows floating
>> +      __s = limit<chrono::duration<time_t>>(__s);
>> +
>>      struct ::timespec __ts =
>>        {
>>  	static_cast<std::time_t>(__s.count()),
>> @@ -246,6 +266,8 @@ namespace this_thread
>>      const auto target = chrono::steady_clock::now() + __s + __ns;
>>      while (true)
>>        {
>> +	__s = limit<chrono::duration<unsigned>>(__s);
>> +
>>  	unsigned secs = __s.count();
>>  	if (__ns.count() > 0)
>>  	  {
>> @@ -271,11 +293,19 @@ namespace this_thread
>>  	  break;
>>  	__s = chrono::duration_cast<chrono::seconds>(target - now);
>>  	__ns = chrono::duration_cast<chrono::nanoseconds>(target - (now + __s));
>> -    }
>> +      }
>>  #elif defined(_GLIBCXX_USE_WIN32_SLEEP)
>> +    // Can't use limit<chrono::milliseconds>(__s) here because it would
>> +    // multiply __s by 1000 which could overflow.
>
>My initial reaction is that it ought to be possible to make limit() cope
>with this but I haven't tried so I'm probably wrong. :)

I did consider that, then decided I didn't want to complicate the
limit function for the non-Windows systems.

>> +    auto max_ms = chrono::milliseconds::max() / 1000;
>> +    auto max_ms_in_s = chrono::duration_cast<chrono::seconds>(max_ms);
>> +    if (__s > max_ms_in_s)
>> +      __s = max_ms_in_s;
>> +
>>      unsigned long ms = __ns.count() / 1000000;
>>      if (__ns.count() > 0 && ms == 0)
>>        ms = 1;
>> +
>>      ::Sleep(chrono::milliseconds(__s).count() + ms);
>
>Shouldn't there be a loop around this so that we're guaranteed to wait for
>at least the requested duration even if __s is too large to be represented
>as milliseconds (at the cost of waking up every ~25 days)?

I think max_ms_in_s is 106751991 days, or 292277 years, so I felt we
didn't need to care about sleeping again if we'd woken up "early".

>>  #endif
>>    }
>> diff --git a/libstdc++-v3/testsuite/30_threads/this_thread/113327.cc b/libstdc++-v3/testsuite/30_threads/this_thread/113327.cc
>> new file mode 100644
>> index 000000000000..2daa2b0e46e2
>> --- /dev/null
>> +++ b/libstdc++-v3/testsuite/30_threads/this_thread/113327.cc
>> @@ -0,0 +1,29 @@
>> +// { dg-do run { target c++11 } }
>> +// { dg-additional-options "-pthread" { target pthread } }
>> +// { dg-require-gthreads "" }
>> +
>> +// PR libstdc++/113327
>> +// std::sleep_for(std::chrono::hours::max()) returns immediately
>> +
>> +#include <thread>
>> +#include <chrono>
>> +#include <cstdlib>
>> +#include <csignal>
>> +
>> +int main()
>> +{
>> +  std::thread sleepy([] {
>> +    // Rather than overflowing to a negative value, the timeout should be
>> +    // truncated to seconds::max() and so sleep for 292 billion years.
>> +    std::this_thread::sleep_for(std::chrono::minutes::max());
>> +    // This should not happen:
>> +    throw 1;
>> +  });
>> +  // Give the new thread a chance to start sleeping:
>> +  std::this_thread::yield();
>> +  std::this_thread::sleep_for(std::chrono::seconds(2));
>> +  // If we get here without the other thread throwing an exception
>> +  // then it should be sleeping peacefully, so the test passed.
>> +  // pthread_kill(sleepy.native_handle(), SIGINT);
>> +  std::_Exit(0);
>> +}
>
>Thanks again for looking at this.
>
>Mike.
>
  

On Wednesday 08 October 2025 at 21:49:16 +0100, Jonathan Wakely wrote:
> On Wed, 08 Oct 2025 at 21:21 +0100, Mike Crowe wrote:
> > On Wednesday 08 October 2025 at 12:27:06 +0100, Jonathan Wakely wrote:
> > > diff --git a/libstdc++-v3/src/c++11/thread.cc b/libstdc++-v3/src/c++11/thread.cc
> > > index 6c2ec2978f88..0768a99d6741 100644
> > > --- a/libstdc++-v3/src/c++11/thread.cc
> > > +++ b/libstdc++-v3/src/c++11/thread.cc
> > > @@ -231,10 +231,30 @@ namespace std _GLIBCXX_VISIBILITY(default)
> > >  _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > >  namespace this_thread
> > >  {
> > > +namespace
> > > +{
> > > +  // returns min(s, Dur::max())
> > > +  template<typename Dur>
> > > +    inline chrono::seconds
> > > +    limit(chrono::seconds s)
> > > +    {
> > > +      static_assert(ratio_equal<typename Dur::period, ratio<1>>::value,
> > > +		    "period must be seconds to avoid potential overflow");
> > > +
> > > +      if (s > Dur::max()) [[__unlikely__]]
> > > +	s = chrono::duration_cast<chrono::seconds>(Dur::max());
> > > +      return s;
> > > +    }
> > > +}
> > > +
> > >    void
> > >    __sleep_for(chrono::seconds __s, chrono::nanoseconds __ns)
> > >    {
> > >  #ifdef _GLIBCXX_USE_NANOSLEEP
> > > +#pragma GCC diagnostic ignored "-Wc++17-extensions"
> > > +    if constexpr (is_integral<time_t>::value) // POSIX.1-2001 allows floating
> > > +      __s = limit<chrono::duration<time_t>>(__s);
> > > +
> > >      struct ::timespec __ts =
> > >        {
> > >  	static_cast<std::time_t>(__s.count()),
> > > @@ -246,6 +266,8 @@ namespace this_thread
> > >      const auto target = chrono::steady_clock::now() + __s + __ns;
> > >      while (true)
> > >        {
> > > +	__s = limit<chrono::duration<unsigned>>(__s);
> > > +
> > >  	unsigned secs = __s.count();
> > >  	if (__ns.count() > 0)
> > >  	  {
> > > @@ -271,11 +293,19 @@ namespace this_thread
> > >  	  break;
> > >  	__s = chrono::duration_cast<chrono::seconds>(target - now);
> > >  	__ns = chrono::duration_cast<chrono::nanoseconds>(target - (now + __s));
> > > -    }
> > > +      }
> > >  #elif defined(_GLIBCXX_USE_WIN32_SLEEP)
> > > +    // Can't use limit<chrono::milliseconds>(__s) here because it would
> > > +    // multiply __s by 1000 which could overflow.
> > > +    auto max_ms = chrono::milliseconds::max() / 1000;
> > > +    auto max_ms_in_s = chrono::duration_cast<chrono::seconds>(max_ms);
> > > +    if (__s > max_ms_in_s)
> > > +      __s = max_ms_in_s;
> > > +
> > >      unsigned long ms = __ns.count() / 1000000;
> > >      if (__ns.count() > 0 && ms == 0)
> > >        ms = 1;
> > > +
> > >      ::Sleep(chrono::milliseconds(__s).count() + ms);
> > 
> > Shouldn't there be a loop around this so that we're guaranteed to wait for
> > at least the requested duration even if __s is too large to be represented
> > as milliseconds (at the cost of waking up every ~25 days)?
> 
> I think max_ms_in_s is 106751991 days, or 292277 years, so I felt we
> didn't need to care about sleeping again if we'd woken up "early".

It appears that Windows Sleep() takes a 32-bit unsigned DWORD, so I think
no matter what you pass you can't make it wait for more than just under 50
days. (The ~25 days in my original calculation was wrong because I hadn't
taken into account that the parameter is unsigned.)

I believe that the max_ms_in_s check is necessary to avoid overflow in the
milliseconds int64_t, but it's not sufficient to avoid truncation when that
int64_t is converted to a DWORD.

Some could argue that a wait of over fifty days is unreasonable, but I
think that's definitely more of a possibility in real code than a wait of
292277 years.

Nevertheless, I wonder whether this is now sufficiently complex that we
should factor it out to some sort of __duration_as_uint_ms() function or
similar so that the implementation can be tested (ideally not just on
Windows) with all the nasty corner cases?

Either that or invent a Sleep wrapper that takes the full resolution
timeout so we don't have to care about this part of the problem. Something
like:

void __win32_sleep(uint64_t ms)
{
  const auto max_sleep = numeric_limits<DWORD>::max();
  while (ms > max_sleep) {
    ::Sleep(max_sleep)
    ms -= max_sleep;
  }
  ::Sleep(ms);
}

or, an equivalent that uses steady_clock::now() and a timeout to account
for mounting errors from lazy wakeup but I doubt that callers care that
much about split-second accuracy when waiting for more than fifty days!

Thanks.

Mike.
  

On Fri, 10 Oct 2025 at 12:39, Mike Crowe <mac@mcrowe.com> wrote:

> On Wednesday 08 October 2025 at 21:49:16 +0100, Jonathan Wakely wrote:
> > On Wed, 08 Oct 2025 at 21:21 +0100, Mike Crowe wrote:
> > > On Wednesday 08 October 2025 at 12:27:06 +0100, Jonathan Wakely wrote:
> > > > diff --git a/libstdc++-v3/src/c++11/thread.cc
> b/libstdc++-v3/src/c++11/thread.cc
> > > > index 6c2ec2978f88..0768a99d6741 100644
> > > > --- a/libstdc++-v3/src/c++11/thread.cc
> > > > +++ b/libstdc++-v3/src/c++11/thread.cc
> > > > @@ -231,10 +231,30 @@ namespace std _GLIBCXX_VISIBILITY(default)
> > > >  _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > > >  namespace this_thread
> > > >  {
> > > > +namespace
> > > > +{
> > > > +  // returns min(s, Dur::max())
> > > > +  template<typename Dur>
> > > > +    inline chrono::seconds
> > > > +    limit(chrono::seconds s)
> > > > +    {
> > > > +      static_assert(ratio_equal<typename Dur::period,
> ratio<1>>::value,
> > > > +             "period must be seconds to avoid potential overflow");
> > > > +
> > > > +      if (s > Dur::max()) [[__unlikely__]]
> > > > + s = chrono::duration_cast<chrono::seconds>(Dur::max());
> > > > +      return s;
> > > > +    }
> > > > +}
> > > > +
> > > >    void
> > > >    __sleep_for(chrono::seconds __s, chrono::nanoseconds __ns)
> > > >    {
> > > >  #ifdef _GLIBCXX_USE_NANOSLEEP
> > > > +#pragma GCC diagnostic ignored "-Wc++17-extensions"
> > > > +    if constexpr (is_integral<time_t>::value) // POSIX.1-2001
> allows floating
> > > > +      __s = limit<chrono::duration<time_t>>(__s);
> > > > +
> > > >      struct ::timespec __ts =
> > > >        {
> > > >   static_cast<std::time_t>(__s.count()),
> > > > @@ -246,6 +266,8 @@ namespace this_thread
> > > >      const auto target = chrono::steady_clock::now() + __s + __ns;
> > > >      while (true)
> > > >        {
> > > > + __s = limit<chrono::duration<unsigned>>(__s);
> > > > +
> > > >   unsigned secs = __s.count();
> > > >   if (__ns.count() > 0)
> > > >     {
> > > > @@ -271,11 +293,19 @@ namespace this_thread
> > > >     break;
> > > >   __s = chrono::duration_cast<chrono::seconds>(target - now);
> > > >   __ns = chrono::duration_cast<chrono::nanoseconds>(target - (now +
> __s));
> > > > -    }
> > > > +      }
> > > >  #elif defined(_GLIBCXX_USE_WIN32_SLEEP)
> > > > +    // Can't use limit<chrono::milliseconds>(__s) here because it
> would
> > > > +    // multiply __s by 1000 which could overflow.
> > > > +    auto max_ms = chrono::milliseconds::max() / 1000;
> > > > +    auto max_ms_in_s =
> chrono::duration_cast<chrono::seconds>(max_ms);
> > > > +    if (__s > max_ms_in_s)
> > > > +      __s = max_ms_in_s;
> > > > +
> > > >      unsigned long ms = __ns.count() / 1000000;
> > > >      if (__ns.count() > 0 && ms == 0)
> > > >        ms = 1;
> > > > +
> > > >      ::Sleep(chrono::milliseconds(__s).count() + ms);
> > >
> > > Shouldn't there be a loop around this so that we're guaranteed to wait
> for
> > > at least the requested duration even if __s is too large to be
> represented
> > > as milliseconds (at the cost of waking up every ~25 days)?
> >
> > I think max_ms_in_s is 106751991 days, or 292277 years, so I felt we
> > didn't need to care about sleeping again if we'd woken up "early".
>
> It appears that Windows Sleep() takes a 32-bit unsigned DWORD, so I think
> no matter what you pass you can't make it wait for more than just under 50
> days. (The ~25 days in my original calculation was wrong because I hadn't
> taken into account that the parameter is unsigned.)
>
> I believe that the max_ms_in_s check is necessary to avoid overflow in the
> milliseconds int64_t, but it's not sufficient to avoid truncation when that
> int64_t is converted to a DWORD.
>

Gah, that's what I missed then, that DWORD is 32-bits. I think I assumed it
was a "double word", which is true, but it's Intel 16-bit words. Sigh.




>
> Some could argue that a wait of over fifty days is unreasonable, but I
> think that's definitely more of a possibility in real code than a wait of
> 292277 years.
>
> Nevertheless, I wonder whether this is now sufficiently complex that we
> should factor it out to some sort of __duration_as_uint_ms() function or
> similar so that the implementation can be tested (ideally not just on
> Windows) with all the nasty corner cases?
>
> Either that or invent a Sleep wrapper that takes the full resolution
> timeout so we don't have to care about this part of the problem. Something
> like:
>
> void __win32_sleep(uint64_t ms)
> {
>   const auto max_sleep = numeric_limits<DWORD>::max();
>   while (ms > max_sleep) {
>     ::Sleep(max_sleep)
>     ms -= max_sleep;
>   }
>   ::Sleep(ms);
> }
>
> or, an equivalent that uses steady_clock::now() and a timeout to account
> for mounting errors from lazy wakeup but I doubt that callers care that
> much about split-second accuracy when waiting for more than fifty days!
>
>
It's used in exactly one place, and it's Windows, so I find it hard to care
enough to factor it out.
  

On Sun, 14 Sep 2025 at 21:21 +0100, Mike Crowe wrote:
>Passing a timeout from before the epoch to
>std::shared_timed_mutex::try_lock_until or
>std::shared_timed_mutex::try_lock_shared_until causes the POSIX rwlock
>functions to be passed an invalid negative timeout which results in them
>returning EINVAL.
>
>thread.timedmutex.requirements.general in the C++ standard says:
>> If abs_time has already passed, the function attempts to obtain
>> ownership without blocking (as if by calling try_lock()).
>
>and a time before the epoch has clearly already passed (see description
>of libstdc++/PR116586). Let's treat such negative times as being at the
>epoch so that these methods work correctly. Add test cases to prove that
>this, and potential similar problems are no longer present.
>
>    PR libstdc++-v3/116586
>
>libstdc++-v3/ChangeLog:
>
>        * include/std/shared_mutex: (shared_timed_mutex::try_lock_until,
>          shared_timed_mutex::try_lock_shared_until): Ensure that
>          negative timeout is not passed to POSIX rwlock functions.
>        * testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc:
>          New test.

In r16-4427-gfb558b78108ff2 I pushed just the new test, without the
code changes. When fixing https://gcc.gnu.org/PR122401 I found that
the new test hangs on older Glibc. I observed it on 2.28 but
presumably other versions without your clocklock extensions are
affected.

Suggested fix below ...

>
>Signed-off-by: Mike Crowe <mac@mcrowe.com>
>---
> libstdc++-v3/include/std/shared_mutex         | 20 ++++
> .../try_lock_until/116586.cc                  | 94 +++++++++++++++++++
> 2 files changed, 114 insertions(+)
> create mode 100644 libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc
>
>diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex
>index 94c8532399d..ef0f1df5a2b 100644
>--- a/libstdc++-v3/include/std/shared_mutex
>+++ b/libstdc++-v3/include/std/shared_mutex
>@@ -529,6 +529,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret = __glibcxx_rwlock_timedwrlock(&_M_rwlock, &__ts);
> 	// On self-deadlock, we just fail to acquire the lock.  Technically,
> 	// the program violated the precondition.
>@@ -555,6 +560,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret = pthread_rwlock_clockwrlock(&_M_rwlock, CLOCK_MONOTONIC,
> 					       &__ts);
> 	// On self-deadlock, we just fail to acquire the lock.  Technically,
>@@ -605,6 +615,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret;
> 	// Unlike for lock(), we are not allowed to throw an exception so if
> 	// the maximum number of read locks has been exceeded, or we would
>@@ -645,6 +660,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret = pthread_rwlock_clockrdlock(&_M_rwlock, CLOCK_MONOTONIC,
> 					       &__ts);
> 	// On self-deadlock, we just fail to acquire the lock.  Technically,
>diff --git a/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc b/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc
>new file mode 100644
>index 00000000000..4ed650a78cf
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc
>@@ -0,0 +1,94 @@
>+// { dg-do run { target c++14 } }
>+
>+#include <chrono>
>+#include <shared_mutex>
>+#include <testsuite_hooks.h>
>+
>+namespace chrono = std::chrono;
>+
>+// thread.timedmutex.requirements.general:
>+//   If abs_time has already passed, the function attempts to obtain
>+//   ownership without blocking (as if by calling try_lock()).
>+
>+template <typename Clock>
>+void
>+test_exclusive_absolute(chrono::nanoseconds offset)
>+{
>+  std::shared_timed_mutex stm;
>+  chrono::time_point<Clock> tp(offset);
>+  VERIFY(stm.try_lock_until(tp));
>+  VERIFY(!stm.try_lock_until(tp));
>+}
>+
>+template <typename Clock>
>+void
>+test_shared_absolute(chrono::nanoseconds offset)
>+{
>+  std::shared_timed_mutex stm;
>+  chrono::time_point<Clock> tp(offset);
>+  VERIFY(stm.try_lock_shared_until(tp));
>+  stm.unlock_shared();
>+
>+  VERIFY(stm.try_lock_for(chrono::seconds{10}));
>+
>+  {
>+    // NPTL will give us EDEADLK if pthread_rwlock_timedrdlock() is called on
>+    // the same thread that already holds the exclusive (write) lock, so let's
>+    // arrange for a different thread to try to acquire the shared lock.
>+    auto t = std::async(std::launch::async, [&stm, tp]() {
>+	VERIFY(!stm.try_lock_shared_until(tp));
>+      });
>+  }
>+}
>+
>+// The type of clock used for the actual wait depends on whether
>+// _GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK is defined. We might as well just test
>+// both steady_clock and system_clock.
>+template <typename Clock>
>+void
>+test_exclusive_relative(chrono::nanoseconds offset)
>+{
>+  std::shared_timed_mutex stm;
>+  const auto d = -Clock::now().time_since_epoch() + offset;
>+  VERIFY(stm.try_lock_for(d));
>+  VERIFY(!stm.try_lock_for(d));
>+}
>+
>+template <typename Clock>
>+void
>+test_shared_relative(chrono::nanoseconds offset)
>+{
>+  std::shared_timed_mutex stm;
>+  const auto d = -Clock::now().time_since_epoch() + offset;
>+  VERIFY(stm.try_lock_shared_for(d));
>+  stm.unlock_shared();
>+  // Should complete immediately
>+  VERIFY(stm.try_lock_for(chrono::seconds{10}));
>+  VERIFY(!stm.try_lock_shared_for(d));

I think this function needs the same use of std::async as above,
because when _GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK is not defined we
still use pthread_rwlock_timedrdlock and so we still get EDEADLK (and
then loop forever) here. You won't see a problem with modern Glibc
versions because I think pthread_rwlock_clockrdlock doesn't return
EDEADLK here.


>+}
>+
>+int main()
>+{
>+  // Try once with an offset that ought to result in tv_sec == 0, tv_nsec < 0
>+  // and one with an offset that ought to result in tv_sec < 0, tv_nsec == 0
>+  // for the absolute calls at least.  It's not really possible to arrange for
>+  // the relative calls to have tv_nsec == 0 due to time advancing.
>+  for (const chrono::nanoseconds offset : {
>+      // tv_sec == 0, tv_nsec == 0
>+      chrono::nanoseconds{0},
>+      // tv_sec == 0, tv_nsec < 0
>+      chrono::duration_cast<chrono::nanoseconds>(chrono::milliseconds{-10}),
>+      // tv_sec < 0
>+      chrono::duration_cast<chrono::nanoseconds>(chrono::seconds{-10})
>+    }) {
>+    test_exclusive_absolute<chrono::system_clock>(offset);
>+    test_shared_absolute<chrono::system_clock>(offset);
>+    test_exclusive_relative<chrono::system_clock>(offset);
>+    test_shared_relative<chrono::system_clock>(offset);
>+
>+    test_exclusive_absolute<chrono::steady_clock>(offset);
>+    test_shared_absolute<chrono::steady_clock>(offset);
>+    test_exclusive_relative<chrono::steady_clock>(offset);
>+    test_shared_relative<chrono::steady_clock>(offset);
>+  }
>+}
>-- 
>2.39.5
>
>