[gcc r11-3157] libstdc++: Avoid rounding errors in std::future::wait_* [PR 91486]

[gcc r11-3157] libstdc++: Avoid rounding errors in std::future::wait_* [PR 91486]

[Jonathan Wakely]

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

commit r11-3157-gf9ddb696a289cc48d24d3d23c0b324cb88de9573
Author: Mike Crowe <mac@mcrowe.com>
Date:   Fri Sep 11 14:25:00 2020 +0100

    libstdc++: Avoid rounding errors in std::future::wait_* [PR 91486]
    
    Convert the specified duration to the target clock's duration type
    before adding it to the current time in
    __atomic_futex_unsigned::_M_load_when_equal_for and
    _M_load_when_equal_until.  This removes the risk of the timeout being
    rounded down to the current time resulting in there being no wait at all
    when the duration type lacks sufficient precision to hold the
    steady_clock current time.
    
    Rather than using the style of fix from PR68519, let's expose the C++17
    std::chrono::ceil function as std::chrono::__detail::ceil so that it can
    be used in code compiled with earlier standards versions and simplify
    the fix. This was suggested by John Salmon in
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91486#c5 .
    
    This problem has become considerably less likely to trigger since I
    switched the __atomic__futex_unsigned::__clock_t reference clock from
    system_clock to steady_clock and added the loop, but the consequences of
    triggering it have changed too.
    
    By my calculations it takes just over 194 days from the epoch for the
    current time not to be representable in a float. This means that
    system_clock is always subject to the problem (with the standard 1970
    epoch) whereas steady_clock with float duration only runs out of
    resolution machine has been running for that long (assuming the Linux
    implementation of CLOCK_MONOTONIC.)
    
    The recently-added loop in
    __atomic_futex_unsigned::_M_load_when_equal_until turns this scenario
    into a busy wait.
    
    Unfortunately the combination of both of these things means that it's
    not possible to write a test case for this occurring in
    _M_load_when_equal_until as it stands.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/91486
            * include/bits/atomic_futex.h
            (__atomic_futex_unsigned::_M_load_when_equal_for)
            (__atomic_futex_unsigned::_M_load_when_equal_until): Use
            __detail::ceil to convert delta to the reference clock
            duration type to avoid resolution problems.
            * include/std/chrono (__detail::ceil): Move implementation
            of std::chrono::ceil into private namespace so that it's
            available to pre-C++17 code.
            * testsuite/30_threads/async/async.cc (test_pr91486):
            Test __atomic_futex_unsigned::_M_load_when_equal_for.

Diff:
---
 libstdc++-v3/include/bits/atomic_futex.h         |  6 ++++--
 libstdc++-v3/include/std/chrono                  | 19 +++++++++++++++----
 libstdc++-v3/testsuite/30_threads/async/async.cc | 15 +++++++++++++++
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/libstdc++-v3/include/bits/atomic_futex.h b/libstdc++-v3/include/bits/atomic_futex.h
index 5f95adeb361..aa137a7b64e 100644
--- a/libstdc++-v3/include/bits/atomic_futex.h
+++ b/libstdc++-v3/include/bits/atomic_futex.h
@@ -219,8 +219,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _M_load_when_equal_for(unsigned __val, memory_order __mo,
 	  const chrono::duration<_Rep, _Period>& __rtime)
       {
+	using __dur = typename __clock_t::duration;
 	return _M_load_when_equal_until(__val, __mo,
-					__clock_t::now() + __rtime);
+		    __clock_t::now() + chrono::__detail::ceil<__dur>(__rtime));
       }
 
     // Returns false iff a timeout occurred.
@@ -233,7 +234,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	do {
 	  const __clock_t::time_point __s_entry = __clock_t::now();
 	  const auto __delta = __atime - __c_entry;
-	  const auto __s_atime = __s_entry + __delta;
+	  const auto __s_atime = __s_entry +
+	      chrono::__detail::ceil<_Duration>(__delta);
 	  if (_M_load_when_equal_until(__val, __mo, __s_atime))
 	    return true;
 	  __c_entry = _Clock::now();
diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 9b9fd2bd42d..893d1f6b2c9 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -329,6 +329,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 #endif // C++20
 
+    // We want to use ceil even when compiling for earlier standards versions
+    namespace __detail
+    {
+      template<typename _ToDur, typename _Rep, typename _Period>
+	constexpr __enable_if_is_duration<_ToDur>
+	ceil(const duration<_Rep, _Period>& __d)
+	{
+	  auto __to = chrono::duration_cast<_ToDur>(__d);
+	  if (__to < __d)
+	    return __to + _ToDur{1};
+	  return __to;
+	}
+    }
+
 #if __cplusplus >= 201703L
 # define __cpp_lib_chrono 201611
 
@@ -346,10 +360,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       constexpr __enable_if_is_duration<_ToDur>
       ceil(const duration<_Rep, _Period>& __d)
       {
-	auto __to = chrono::duration_cast<_ToDur>(__d);
-	if (__to < __d)
-	  return __to + _ToDur{1};
-	return __to;
+	return __detail::ceil<_ToDur>(__d);
       }
 
     template <typename _ToDur, typename _Rep, typename _Period>
diff --git a/libstdc++-v3/testsuite/30_threads/async/async.cc b/libstdc++-v3/testsuite/30_threads/async/async.cc
index 31e9686fab2..46f8d2f327d 100644
--- a/libstdc++-v3/testsuite/30_threads/async/async.cc
+++ b/libstdc++-v3/testsuite/30_threads/async/async.cc
@@ -157,6 +157,20 @@ void test04()
   }
 }
 
+void test_pr91486()
+{
+  future<void> f1 = async(launch::async, []() {
+      std::this_thread::sleep_for(std::chrono::seconds(1));
+    });
+
+  std::chrono::duration<float> const wait_time = std::chrono::seconds(1);
+  auto const start_steady = chrono::steady_clock::now();
+  auto status = f1.wait_for(wait_time);
+  auto const elapsed_steady = chrono::steady_clock::now() - start_steady;
+
+  VERIFY( elapsed_steady >= std::chrono::seconds(1) );
+}
+
 int main()
 {
   test01();
@@ -165,5 +179,6 @@ int main()
   test03<std::chrono::steady_clock>();
   test03<steady_clock_copy>();
   test04();
+  test_pr91486();
   return 0;
 }