The following update to preatty-printers seem to be also required: diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py index 5f5963cb595..51defbce78b 100644 --- a/libstdc++-v3/python/libstdcxx/v6/printers.py +++ b/libstdc++-v3/python/libstdcxx/v6/printers.py @@ -287,7 +287,7 @@ class SharedPointerPrinter(printer_base): def _get_refcounts(self): if self._typename == 'std::atomic': # A tagged pointer is stored as uintptr_t. - ptr_val = self._val['_M_refcount']['_M_val']['_M_i'] + ptr_val = self._val['_M_refcount']['_M_val'] ptr_val = ptr_val - (ptr_val % 2) # clear lock bit ptr_type = find_type(self._val['_M_refcount'].type, 'pointer') return ptr_val.cast(ptr_type)
On Wed, Aug 20, 2025 at 5:00 PM Tomasz Kamiński <tkami...@redhat.com> wrote: > Previously, atomic<shared_ptr<T>>::wait (and the weak_ptr version) was > equivalent to waiting directly on _M_val, which corresponds to the pointer > to > the control block (_M_pi). Consequently, wakeups were not triggered if the > stored > pointer value was changed to a pointer that uses the same control block but > stores pointer to a different object. Such a pointer can be constructed > using an > aliasing constructor. > > To address this, wait now uses a generic proxy wait > std::__atomic_wait_address > function, which supports waiting until any predicate is satisfied. The > provided > predicate now compares both the control block (_M_pi) and the stored > pointer > (_M_ptr). Comparing the latter requires locking the pointer. > > Since this function operates on raw pointers, the type of _M_val was > changed > from __atomic_base<uintptr_t> to uninptr. Invocations of the corresponding > member functions are now replaced with direct use of __atomic builtins. > > PR libstdc++/118757 > > libstdc++-v3/ChangeLog: > > * include/bits/shared_ptr_atomic.h (_Atomic_count::_M_wait_unlock): > Add parameter capturing reference to _M_ptr. Reimplement in terms > of __atomic_wait_address. > (_Atomic_count::~_Atomic_count, _Atomic_count::lock) > (_Atomic_count::unlock, _Atomic_count::_M_swap_unlock): Replace > invocation of atomic member funcitons with __atomic builtins. > (_Atomic_count::notify_one, _Atomic_count::notify_all): > Use __atomic_notify_address. > (_Sp_atomic::element_type): Define. > (_Sp_atomic::_M_val): Change type to uintptr_t. > (_Sp_atomic::wait): Pass _M_ptr to _M_wait_unlock. > * testsuite/20_util/shared_ptr/atomic/pr118757.cc: New test. > * testsuite/20_util/weak_ptr/pr118757.cc: New test. > > Signed-off-by: Tomasz Kamiński <tkami...@redhat.com> > --- > Testing on x86_64-linux locally. The 20_util/*ptr test alrady passed. > OK for trunk? > > Given large atomic refactoring, I do not think we want to backport it. > > libstdc++-v3/include/bits/shared_ptr_atomic.h | 61 +++++++++++++------ > .../20_util/shared_ptr/atomic/pr118757.cc | 46 ++++++++++++++ > .../testsuite/20_util/weak_ptr/pr118757.cc | 47 ++++++++++++++ > 3 files changed, 137 insertions(+), 17 deletions(-) > create mode 100644 > libstdc++-v3/testsuite/20_util/shared_ptr/atomic/pr118757.cc > create mode 100644 libstdc++-v3/testsuite/20_util/weak_ptr/pr118757.cc > > diff --git a/libstdc++-v3/include/bits/shared_ptr_atomic.h > b/libstdc++-v3/include/bits/shared_ptr_atomic.h > index cc7841a8775..7a44f61f5c7 100644 > --- a/libstdc++-v3/include/bits/shared_ptr_atomic.h > +++ b/libstdc++-v3/include/bits/shared_ptr_atomic.h > @@ -392,6 +392,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > class _Sp_atomic > { > using value_type = _Tp; > + using element_type = typename _Tp::element_type; > > friend struct atomic<_Tp>; > > @@ -420,7 +421,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > ~_Atomic_count() > { > - auto __val = _M_val.load(memory_order_relaxed); > + auto __val = __atomic_load_n(&_M_val, int(memory_order_relaxed)); > _GLIBCXX_TSAN_MUTEX_DESTROY(&_M_val); > __glibcxx_assert(!(__val & _S_lock_bit)); > if (auto __pi = reinterpret_cast<pointer>(__val)) > @@ -442,21 +443,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > { > // To acquire the lock we flip the LSB from 0 to 1. > > - auto __current = _M_val.load(memory_order_relaxed); > + auto __current = __atomic_load_n(&_M_val, > int(memory_order_relaxed)); > while (__current & _S_lock_bit) > { > #if __glibcxx_atomic_wait > __detail::__thread_relax(); > #endif > - __current = _M_val.load(memory_order_relaxed); > + __current = __atomic_load_n(&_M_val, > int(memory_order_relaxed)); > } > > _GLIBCXX_TSAN_MUTEX_TRY_LOCK(&_M_val); > > - while (!_M_val.compare_exchange_strong(__current, > - __current | _S_lock_bit, > - __o, > - memory_order_relaxed)) > + while (!__atomic_compare_exchange_n( > + &_M_val, &__current, __current | _S_lock_bit, 0, > + int(__o), int(memory_order_relaxed))) > { > _GLIBCXX_TSAN_MUTEX_TRY_LOCK_FAILED(&_M_val); > #if __glibcxx_atomic_wait > @@ -474,7 +474,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > unlock(memory_order __o) const noexcept > { > _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(&_M_val); > - _M_val.fetch_sub(1, __o); > + __atomic_fetch_sub(&_M_val, 1, int(__o)); > _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(&_M_val); > } > > @@ -487,7 +487,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > __o = memory_order_release; > auto __x = reinterpret_cast<uintptr_t>(__c._M_pi); > _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(&_M_val); > - __x = _M_val.exchange(__x, __o); > + __x = __atomic_exchange_n(&_M_val, __x, int(__o)); > _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(&_M_val); > __c._M_pi = reinterpret_cast<pointer>(__x & ~_S_lock_bit); > } > @@ -495,19 +495,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > #if __glibcxx_atomic_wait > // Precondition: caller holds lock! > void > - _M_wait_unlock(memory_order __o) const noexcept > + _M_wait_unlock(const element_type* const& __ptr, memory_order __o) > const noexcept > { > + auto __old_ptr = __ptr; > _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(&_M_val); > - auto __v = _M_val.fetch_sub(1, memory_order_relaxed); > + auto __old_pi > + = __atomic_fetch_sub(&_M_val, 1, int(memory_order_relaxed)) > + & ~_S_lock_bit; > _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(&_M_val); > - _M_val.wait(__v & ~_S_lock_bit, __o); > + > + // Ensure that the correct value of _M_ptr is visible after > locking, > + // by upgrading relaxed or consume to acquire. > + auto __lo = __o; > + if (__o != memory_order_seq_cst) > + __lo = memory_order_acquire; > + > + std::__atomic_wait_address( > + &_M_val, > + [=, &__ptr, this](uintptr_t __new_pi) > + { > + if (__old_pi != (__new_pi & ~_S_lock_bit)) > + // control block changed, we can wake up > + return true; > + > + // control block is same, we need to check if ptr changed, > + // we need to take the lock first, the value of pi may > have > + // also been updated in meantime, so reload it > + __new_pi = reinterpret_cast<uintptr_t>(this->lock(__lo)); > + auto __new_ptr = __ptr; > + this->unlock(memory_order_relaxed); > + // wake up if either of the values changed > + return __new_pi != __old_pi || __new_ptr != __old_ptr; > + }, > + [__o, this] { return __atomic_load_n(&_M_val, int(__o)); }); > } > > void > notify_one() noexcept > { > _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(&_M_val); > - _M_val.notify_one(); > + std::__atomic_notify_address(&_M_val, false); > _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(&_M_val); > } > > @@ -515,17 +542,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > notify_all() noexcept > { > _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(&_M_val); > - _M_val.notify_all(); > + std::__atomic_notify_address(&_M_val, true); > _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(&_M_val); > } > #endif > > private: > - mutable __atomic_base<uintptr_t> _M_val{0}; > + mutable uintptr_t _M_val{0}; > static constexpr uintptr_t _S_lock_bit{1}; > }; > > - typename _Tp::element_type* _M_ptr = nullptr; > + element_type* _M_ptr = nullptr; > _Atomic_count _M_refcount; > > static typename _Atomic_count::pointer > @@ -608,7 +635,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > { > auto __pi = _M_refcount.lock(memory_order_acquire); > if (_M_ptr == __old._M_ptr && __pi == __old._M_refcount._M_pi) > - _M_refcount._M_wait_unlock(__o); > + _M_refcount._M_wait_unlock(_M_ptr, __o); > else > _M_refcount.unlock(memory_order_relaxed); > } > diff --git a/libstdc++-v3/testsuite/20_util/shared_ptr/atomic/pr118757.cc > b/libstdc++-v3/testsuite/20_util/shared_ptr/atomic/pr118757.cc > new file mode 100644 > index 00000000000..9c3dc01255c > --- /dev/null > +++ b/libstdc++-v3/testsuite/20_util/shared_ptr/atomic/pr118757.cc > @@ -0,0 +1,46 @@ > +// Copyright (C) 2014-2025 Free Software Foundation, Inc. > +// > +// This file is part of the GNU ISO C++ Library. This library is free > +// software; you can redistribute it and/or modify it under the > +// terms of the GNU General Public License as published by the > +// Free Software Foundation; either version 3, or (at your option) > +// any later version. > + > +// This library is distributed in the hope that it will be useful, > +// but WITHOUT ANY WARRANTY; without even the implied warranty of > +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +// GNU General Public License for more details. > + > +// You should have received a copy of the GNU General Public License along > +// with this library; see the file COPYING3. If not see > +// <http://www.gnu.org/licenses/>. > + > +// { dg-do run { target c++20 } } > +// { dg-require-gthreads "" } > +// { dg-require-effective-target hosted } > + > +#include <memory> > +#include <chrono> > +#include <thread> > +#include <barrier> > + > +std::shared_ptr<int> q = std::make_shared<int>(42); > +std::atomic<std::shared_ptr<int>> p = q; > + > +std::barrier bar(2); > + > +void signaler() > +{ > + std::this_thread::sleep_for(std::chrono::seconds(1)); > + p.store(std::shared_ptr<int>(q, nullptr)); > + p.notify_one(); > + bar.arrive_and_wait(); > +} > + > +int main(int, char**) > +{ > + std::thread thr(signaler); > + p.wait(q); > + bar.arrive_and_wait(); > + thr.join(); > +} > diff --git a/libstdc++-v3/testsuite/20_util/weak_ptr/pr118757.cc > b/libstdc++-v3/testsuite/20_util/weak_ptr/pr118757.cc > new file mode 100644 > index 00000000000..c1110241f09 > --- /dev/null > +++ b/libstdc++-v3/testsuite/20_util/weak_ptr/pr118757.cc > @@ -0,0 +1,47 @@ > +// Copyright (C) 2014-2025 Free Software Foundation, Inc. > +// > +// This file is part of the GNU ISO C++ Library. This library is free > +// software; you can redistribute it and/or modify it under the > +// terms of the GNU General Public License as published by the > +// Free Software Foundation; either version 3, or (at your option) > +// any later version. > + > +// This library is distributed in the hope that it will be useful, > +// but WITHOUT ANY WARRANTY; without even the implied warranty of > +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +// GNU General Public License for more details. > + > +// You should have received a copy of the GNU General Public License along > +// with this library; see the file COPYING3. If not see > +// <http://www.gnu.org/licenses/>. > + > +// { dg-do run { target c++20 } } > +// { dg-require-gthreads "" } > +// { dg-require-effective-target hosted } > + > +#include <memory> > +#include <chrono> > +#include <thread> > +#include <barrier> > + > +std::shared_ptr<int> s = std::make_shared<int>(42); > +std::weak_ptr<int> q = s; > +std::atomic<std::weak_ptr<int>> p = q; > + > +std::barrier bar(2); > + > +void signaler() > +{ > + std::this_thread::sleep_for(std::chrono::seconds(1)); > + p.store(std::shared_ptr<int>(s, nullptr)); > + p.notify_one(); > + bar.arrive_and_wait(); > +} > + > +int main(int, char**) > +{ > + std::thread thr(signaler); > + p.wait(q); > + bar.arrive_and_wait(); > + thr.join(); > +} > -- > 2.50.1 > >