On Wed, Jul 1, 2026 at 5:13 PM Jonathan Wakely <[email protected]> wrote:
> On Wed, 1 Jul 2026 at 15:53, Tomasz Kaminski <[email protected]> wrote: > > > > > > > > On Wed, Jul 1, 2026 at 4:36 PM Jonathan Wakely <[email protected]> > wrote: > >> > >> The changes in r17-2047-gb12fdd95251178 cause a bootstrap failure if > >> ATOMIC_POINTER_LOCK_FREE != 2 && ATOMIC_INT_LOCK_FREE == 2 is true for > >> the target: > >> > >> .../tzdb.cc: In static member function ‘static const std::chrono::tzdb& > std::chrono::tzdb_list::_Node::_S_replace_head(std::shared_ptr<std::chrono::tzdb_list::_Node>, > std::shared_ptr<std::chrono::tzdb_list::_Node>)’: > >> .../tzdb.cc:1617:22: error: ‘struct > std::chrono::tzdb_list::_Node::NumLeapSeconds’ has no member named > ‘set_locked’ > >> 1617 | > num_leap_seconds.set_locked(new_head_ptr->db.leap_seconds.size(), lock); > >> | ^~~~~~~~~~ > >> > >> This fix defines the 'set_atomically' and 'set_locked' functions > >> unconditionally, and renames the former to just 'set'. This fixes the > >> mismatch between the atomic pointer and atomic int conditions, as both > >> functions are available for both branches of the #if/#else in > >> _Node::_S_replace_head. > >> > >> libstdc++-v3/ChangeLog: > >> > >> * src/c++20/tzdb.cc (_Node::NumLeapSeconds::set_atomically): > >> Rename to set and define unconditionally. > >> (_Node::NumLeapSeconds::set_locked): Define unconditionally. > >> --- > >> > >> Pushed to trunk. > >> > >> libstdc++-v3/src/c++20/tzdb.cc | 15 +++++++++------ > >> 1 file changed, 9 insertions(+), 6 deletions(-) > >> > >> diff --git a/libstdc++-v3/src/c++20/tzdb.cc > b/libstdc++-v3/src/c++20/tzdb.cc > >> index 751370325d76..1aae07033f73 100644 > >> --- a/libstdc++-v3/src/c++20/tzdb.cc > >> +++ b/libstdc++-v3/src/c++20/tzdb.cc > >> @@ -1454,23 +1454,26 @@ struct tzdb_list::_Node::NumLeapSeconds > >> // Called by _Node::_S_replace_head > >> // The two versions are named differently so that caller has to be > explicit > >> // about which version it calls, based on whether the mutex is held. > >> -#if ATOMIC_INT_LOCK_FREE == 2 > >> void > >> - set_atomically(unsigned val) > >> + set(unsigned val) > >> { > >> +#if ATOMIC_INT_LOCK_FREE == 2 > >> atomic_ref<unsigned> ref(count); > >> // The release op here synchronizes with the acquire op in get(). > >> ref.store(val, memory_order::release); > >> - } > >> #else > >> + lock_guard<mutex> l(list_mutex()); > >> + set_locked(val, l); > >> +#endif > >> + } > >> + > >> void > >> set_locked(unsigned val, const lock_guard<mutex>&) > >> { > >> - // XXX The only caller of this function locks list_mutex() so we > would > >> + // The only caller of this function locks list_mutex() so we would > >> // deadlock if we locked it again here. > >> count = val; > > > > If ATOMIC_INT_LOCK_FREE is true, but USE_ATOMIC_SHARED_PTR is false, > > we are having a data race here, as __recent_leap_seconds will not lock > the mutex > > before reading count in that case. We should simply call set in this > case. > > Ah yes, so we want: > > --- a/libstdc++-v3/src/c++20/tzdb.cc > +++ b/libstdc++-v3/src/c++20/tzdb.cc > @@ -1470,9 +1470,15 @@ struct tzdb_list::_Node::NumLeapSeconds > void > set_locked(unsigned val, const lock_guard<mutex>&) > { > +#if ATOMIC_INT_LOCK_FREE == 2 > + // Even though the caller locked the mutex, we still need to use an > + // atomic store in this case, because there could be concurrent loads. > + set(val); > Yes, something like this. > +#else // The only caller of this function locks list_mutex() so we would > // deadlock if we locked it again here. > count = val; > +#endif > } > > private: > > > > > > > > > >> } > >> -#endif > >> > >> private: > >> unsigned count = 0; > >> @@ -1690,7 +1693,7 @@ constinit tzdb_list::_Node::NumLeapSeconds > tzdb_list::_Node::num_leap_seconds; > >> > >> // This allows __recent_leap_second_info() to know that it can use > >> // get_tzdb_list()->begin()->leap_seconds to get new leap seconds. > >> - > num_leap_seconds.set_atomically(new_head_ptr->db.leap_seconds.size()); > >> + num_leap_seconds.set(new_head_ptr->db.leap_seconds.size()); > >> #else > >> lock_guard<mutex> lock(list_mutex()); > >> if (const _Node* h = _S_head_owner.get()) > >> -- > >> 2.54.0 > >> > >
