On Thu, Jun 5, 2025 at 12:10 AM Jonathan Wakely <jwak...@redhat.com> wrote:
> These overloads should never be used for proxy waits, so add assertions
> to ensure that they aren't used accidentally.
>
> The reason they can't be used is that they don't call
> __args._M_setup_wait to obtain a __wait_state pointer. Even if that was
> changed, they would wait on a proxy wait which is potentially used by
> many other threads waiting on other addresses, meaning spurious wake ups
> are likely. In order to make the functions correct they would need to
> perform additional loads and comparisons of the atomic variable before
> calling __wait_impl or __wait_until_impl, which would make these
> functions no faster than the general purpose overloads that take an
> accessor function and predicate. That would be possible, and I think
> they would then work for proxy waits, but doesn't seem necessary at this
> time.
>
> In order to preseve the property that these functions are more
> lightweight and efficient than the general ones, they should not be used
> for proxy waits.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/atomic_timed_wait.h (__atomic_wait_address_until_v):
> Add assertion to prevent use with proxy waits.
> (__atomic_wait_address_for_v): Likewise.
> * include/bits/atomic_wait.h (__atomic_wait_address_v):
> Likewise.
> ---
>
> Tested x86_64-linux and sparc-solaris.
>
I would change all the static_assert into runtime asserts. This functions
are dependent
only on the duration parameters, and I writing something like"
__atomic_wait_address_for_v(add, old, ACQUIRE, 0s);
would lead to hard error even in the , even in dependent context like
platform_semphore_impl:
I think it is surprising that __atomic_wait_address_v works in such cases,
but duration eversion sometimes
does not.
>
> libstdc++-v3/include/bits/atomic_timed_wait.h | 8 ++++++++
> libstdc++-v3/include/bits/atomic_wait.h | 4 +++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/include/bits/atomic_timed_wait.h
> b/libstdc++-v3/include/bits/atomic_timed_wait.h
> index bd2e6bf61ecf..9cd79e8829c7 100644
> --- a/libstdc++-v3/include/bits/atomic_timed_wait.h
> +++ b/libstdc++-v3/include/bits/atomic_timed_wait.h
> @@ -156,6 +156,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> const chrono::time_point<_Clock, _Dur>&
> __atime,
> bool __bare_wait = false) noexcept
> {
> +#ifndef _GLIBCXX_HAVE_PLATFORM_TIMED_WAIT
> + using namespace __detail;
> + static_assert(__platform_wait_uses_type<__platform_wait_t>);
> +#endif
> __detail::__wait_args __args{ __addr, __old, __order, __bare_wait };
> auto __res = __detail::__wait_until(__addr, __args, __atime);
> return !__res._M_timeout; // C++26 will also return last observed
> __val
> @@ -205,6 +209,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> const chrono::duration<_Rep, _Period>&
> __rtime,
> bool __bare_wait = false) noexcept
> {
> +#ifndef _GLIBCXX_HAVE_PLATFORM_TIMED_WAIT
> + using namespace __detail;
> + static_assert(__platform_wait_uses_type<__platform_wait_t>);
> +#endif
> __detail::__wait_args __args{ __addr, __old, __order, __bare_wait };
> auto __res = __detail::__wait_for(__addr, __args, __rtime);
> return !__res._M_timeout; // C++26 will also return last observed
> __val
> diff --git a/libstdc++-v3/include/bits/atomic_wait.h
> b/libstdc++-v3/include/bits/atomic_wait.h
> index d32fda1c2435..95151479c120 100644
> --- a/libstdc++-v3/include/bits/atomic_wait.h
> +++ b/libstdc++-v3/include/bits/atomic_wait.h
> @@ -250,12 +250,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> }
>
> // Wait on __addr while *__addr == __old is true.
> - // Cannot be used for proxy waits.
> inline void
> __atomic_wait_address_v(const __detail::__platform_wait_t* __addr,
> __detail::__platform_wait_t __old,
> int __order, bool __bare_wait = false)
> {
> +#ifndef _GLIBCXX_HAVE_PLATFORM_WAIT
> + __glibcxx_assert(false); // This function can't be used for proxy
> wait.
> +#endif
> __detail::__wait_args __args{ __addr, __old, __order, __bare_wait };
> // C++26 will not ignore the return value here
> __detail::__wait_impl(__addr, __args);
> --
> 2.49.0
>
>
