[1/6] libstdc++: Fix __platform_wait_until for negative timeouts [PR116586]

  Passing a timeout from before the epoch to __platform_wait_until()
currently results in an exception being thrown because futex(2) doesn't
support negative timeouts.  Let's just immediately return indicating a
timeout has happened.

Add test cases to prove that this bug is fixed for std::binary_semaphore
(which is just an alias for std::counting_semaphore<1>).  The tests
exercise cases that aren't problematic with the current code since
system_clock is converted to steady_clock before calling
__platform_wait_until() is called but they will protect against changes
in the implementation reintroducing this bug.

        PR libstdc++/116586

    libstdc++-v3/ChangeLog:
        * src/c++20/atomic.cc (__platform_wait_until): Return false on
          timeout before epoch.
        * testsuite/30_threads/semaphore/try_acquire_for.cc: Add tests.
        * testsuite/30_threads/semaphore/try_acquire_until.cc: Add tests.

Signed-off-by: Mike Crowe <mac@mcrowe.com>
---
 libstdc++-v3/src/c++20/atomic.cc              |  4 ++++
 .../30_threads/semaphore/try_acquire_for.cc   | 20 +++++++++++++++++
 .../30_threads/semaphore/try_acquire_until.cc | 22 +++++++++++++++++++
 3 files changed, 46 insertions(+)
  

Thanks for the patches, Mike, I've finally managed to do the first set
of reviews ...

On Sun, 14 Sept 2025 at 21:30, Mike Crowe wrote:
>
> Passing a timeout from before the epoch to __platform_wait_until()
> currently results in an exception being thrown because futex(2) doesn't
> support negative timeouts.  Let's just immediately return indicating a
> timeout has happened.
>
> Add test cases to prove that this bug is fixed for std::binary_semaphore
> (which is just an alias for std::counting_semaphore<1>).  The tests
> exercise cases that aren't problematic with the current code since
> system_clock is converted to steady_clock before calling
> __platform_wait_until() is called but they will protect against changes
> in the implementation reintroducing this bug.
>
>         PR libstdc++/116586
>
>     libstdc++-v3/ChangeLog:
>         * src/c++20/atomic.cc (__platform_wait_until): Return false on
>           timeout before epoch.
>         * testsuite/30_threads/semaphore/try_acquire_for.cc: Add tests.
>         * testsuite/30_threads/semaphore/try_acquire_until.cc: Add tests.
>
> Signed-off-by: Mike Crowe <mac@mcrowe.com>
> ---
>  libstdc++-v3/src/c++20/atomic.cc              |  4 ++++
>  .../30_threads/semaphore/try_acquire_for.cc   | 20 +++++++++++++++++
>  .../30_threads/semaphore/try_acquire_until.cc | 22 +++++++++++++++++++
>  3 files changed, 46 insertions(+)
>
> diff --git a/libstdc++-v3/src/c++20/atomic.cc b/libstdc++-v3/src/c++20/atomic.cc
> index 4120e1a0817..1e987213d80 100644
> --- a/libstdc++-v3/src/c++20/atomic.cc
> +++ b/libstdc++-v3/src/c++20/atomic.cc
> @@ -359,6 +359,10 @@ __platform_wait_until(const __platform_wait_t* __addr,
>      static_cast<long>(__ns.count())
>    };
>
> +  // SYS_futex returns EINVAL if timeout is negative, act as if we timed out immediately
> +  if (__rt.tv_sec < 0 || __rt.tv_nsec < 0)

We could put [[unlikely]] here. I'm not sure how much difference it
will make, but I don't think it can hurt.

> +    return false;
> +
>    if (syscall (SYS_futex, __addr,
>                static_cast<int>(__futex_wait_flags::__wait_bitset_private),
>                __old, &__rt, nullptr,
> diff --git a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc
> index 39681c7ee56..fe0d704baf8 100644
> --- a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc
> +++ b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc
> @@ -90,9 +90,29 @@ test03()
>    s.try_acquire_for(timeout);
>  }
>
> +// Prove semaphore doesn't suffer from PR116586
> +template <typename Clock>
> +void
> +test_relative(std::chrono::nanoseconds offset)
> +{
> +  std::binary_semaphore sem(1);
> +  VERIFY(sem.try_acquire_for(offset));
> +  VERIFY(!sem.try_acquire_for(offset));
> +}
> +
>  int main()
>  {
>    test01();
>    test02();
>    test03();
> +  for (const std::chrono::nanoseconds offset : {
> +      std::chrono::nanoseconds{0},
> +      std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::milliseconds{-10}),

With a 'using namespace std::chrono;' above, this could be just
nanoseconds{-10ms}

> +      std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::seconds{-10})

And this could be just nanoseconds{-10s}

Which seems more readable to me.

I can push this if you're happy with those changes.

> +    }) {
> +    test_relative<std::chrono::system_clock>(offset);
> +    test_relative<std::chrono::system_clock>(offset - std::chrono::system_clock::now().time_since_epoch());
> +    test_relative<std::chrono::steady_clock>(offset);
> +    test_relative<std::chrono::steady_clock>(offset - std::chrono::steady_clock::now().time_since_epoch());
> +  }
>  }
> diff --git a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_until.cc b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_until.cc
> index de0068d670a..a00f124b147 100644
> --- a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_until.cc
> +++ b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_until.cc
> @@ -87,8 +87,30 @@ void test02()
>    b.wait(1);
>  }
>
> +// Prove semaphore doesn't suffer from PR116586
> +template <typename Clock>
> +void
> +test_absolute(std::chrono::nanoseconds offset)
> +{
> +  std::binary_semaphore sem(1);
> +  std::chrono::time_point<Clock> tp(offset);
> +  VERIFY(sem.try_acquire_until(tp));
> +  VERIFY(!sem.try_acquire_until(tp));
> +}
> +
>  int main()
>  {
>    test01();
>    test02();
> +  for (const std::chrono::nanoseconds offset : {
> +      // tv_sec == 0, tv_nsec == 0
> +      std::chrono::nanoseconds{0},
> +      // tv_sec == 0, tv_nsec < 0
> +      std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::milliseconds{-10}),
> +      // tv_sec < 0
> +      std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::seconds{-10})
> +    }) {
> +    test_absolute<std::chrono::system_clock>(offset);
> +    test_absolute<std::chrono::steady_clock>(offset);
> +  }
>  }
> --
> 2.39.5
>
  

On Wednesday 08 October 2025 at 12:05:24 +0100, Jonathan Wakely wrote:
> Thanks for the patches, Mike, I've finally managed to do the first set
> of reviews ...

Thanks for the review.

[snip suggested changes]

> I can push this if you're happy with those changes.

I'm happy with those changes and any other changes you want to make. I'm
also happy to make the types of changes you requested across the whole
series and re-post patches if that means less work for you.

Thanks.

Mike.