mirror of
https://forge.sourceware.org/marek/gcc.git
synced 2026-02-22 03:47:02 -05:00
A failed assertion was observed with std::atomic<bool>::wait when the loop in __atomic_wait_address is entered and calls _M_setup_wait a second time, after waking from __wait_impl. When the first call to _M_setup_wait makes a call to _M_setup_proxy_wait that function decides that a proxy wait is needed for an object of type bool, and it updates the _M_obj and _M_obj_size members to refer to the futex in the proxy state, instead of referring to the bool object itself. The next time _M_setup_wait is called it calls _M_setup_proxy_wait again but now it sees _M_obj_size == sizeof(futex) and so this time decides a proxy wait is *not* needed, and then fails the __glibcxx_assert(_M_obj == addr) check. The problem is that _M_setup_proxy_wait wasn't correctly handling the case where it's called a second time, after the decision to use a proxy wait has already been made. That can be fixed in _M_setup_proxy_wait by checking if _M_obj != addr, which implies that a proxy wait has already been set up by a previous call. In that case, _M_setup_proxy_wait should only update _M_old to the latest value of the proxy _M_ver. This change means that _M_setup_proxy_wait is safe to call repeatedly for a proxy wait, and will only update _M_wait_state, _M_obj, and _M_obj_size on the first call. On the second and subsequent calls, those variables are already correctly set for the proxy wait so don't need to be set again. For non-proxy waits, calling _M_setup_proxy_wait more than once is safe, but pessimizes performance. The caller shouldn't make a second call to _M_setup_proxy_wait because we don't need to check again if a proxy wait should be used (the answer won't change) and we don't need to load a value from the proxy _M_ver. However, it was difficult to detect the case of a non-proxy wait, because _M_setup_wait doesn't know if it's being called the first time (when _M_setup_proxy_wait is called to make the initial decision) or a subsequent time (in which case _M_obj == addr implies a non-proxy wait was already decided on). As a result, _M_setup_proxy_wait was being used every time to see if it's a proxy wait. We can resolve this by splitting the _M_setup_wait function into _M_setup_wait and _M_on_wake, where the former is only called once to do the initial setup and the latter is called after __wait_impl returns, to prepare to check the predicate and possibly wait again. The new _M_on_wake function can avoid unnecessary calls to _M_setup_proxy_wait by checking _M_obj == addr to identify a non-proxy wait. The three callers of _M_setup_wait are updated to use _M_on_wake instead of _M_setup_wait after waking from a waiting function. This change revealed a latent performance bug in __atomic_wait_address_for which was not passing __res to _M_setup_wait, so a new value was always loaded even when __res._M_has_val was true. By splitting _M_on_wake out of _M_setup_wait this problem became more obvious, because we no longer have _M_setup_wait doing two different jobs, depending on whether it was passed the optional third argument or not. libstdc++-v3/ChangeLog: * include/bits/atomic_timed_wait.h (__atomic_wait_address_until): Use _M_on_wake instead of _M_setup_wait after waking. (__atomic_wait_address_for): Likewise. * include/bits/atomic_wait.h (__atomic_wait_address): Likewise. (__wait_args::_M_setup_wait): Remove third parameter and move code to update _M_old to ... (__wait_args::_M_on_wake): New member function to update _M_old after waking, only calling _M_setup_proxy_wait if needed. (__wait_args::_M_store): New member function to update _M_old from a value, for non-proxy waits. * src/c++20/atomic.cc (__wait_args::_M_setup_proxy_wait): If _M_obj is not addr, only load a new value and return true. Reviewed-by: Tomasz Kamiński <tkaminsk@redhat.com>
file: libstdc++-v3/README New users may wish to point their web browsers to the file index.html in the 'doc/html' subdirectory. It contains brief building instructions and notes on how to configure the library in interesting ways.