On Thu, 25 Jun 2026 at 13:01, Tomasz Kaminski <[email protected]> wrote:
>
>
>
> On Thu, Jun 25, 2026 at 1:42 PM Jonathan Wakely <[email protected]> wrote:
>>
>> My r17-471-ge79f0f818c0e42 change to optimize handling of leap seconds
>> introduced a hard dependency on std::atomic<unsigned>, which causes
>> problems for targets without atomic word operations, like Cortex-M0.:
>> https://gcc.gnu.org/pipermail/gcc-patches/2026-June/719704.html
>>
>> This patch replaces the num_leap_seconds variable with a struct which
>> decides whether to use std::atomic_ref<unsigned> or perform all accesses
>> while holding a lock on the pre-existing mutex used for the tzdb_list
>> singleton.
>>
>> The workaround is a bit ugly and fragile, because it assumes that there
>> is only one caller of num_leap_seconds.set and that the list_mutex() is
>> locked by that caller iff the tzdb_list doesn't use atomic<shared_ptr<>>
>> (which is enforced via preprocessor checks).
>
> We could have two different methods, set_under_lock and set_atomic
> and call them in branches:
I considered that, but it means duplicating the _S_cache_list_head
call, because ...
> #if USE_ATOMIC_SHARED_PTR
> new_head_ptr->next = curr;
> while (!_S_head_owner.compare_exchange_strong(curr, new_head))
> {
> if (curr->db.version == new_head_ptr->db.version)
> return curr->db;
> new_head_ptr->next = curr;
> }
> // XXX small window here where _S_head_cache still points to previous
> tzdb.
> _S_cache_list_head(new_head_ptr); <-- This is empty if
> USE_ATOMIC_SHARED_PTR is false,
> so do
> not define it.
Strictly speaking, it's empty for ! USE_ATOMIC_LIST_HEAD and in theory
the code is designed to support the case where we have
USE_ATOMIC_LIST_HEAD and ! USE_ATOMIC_SHARED_PTR. For example, if
benchmarking shows that the mutex performs better than the
atomic<shared_ptr<>>, maybe only on particular platforms.
So it's true that _S_cache_list_head is empty for !
USE_ATOMIC_SHARED_PTR *today* but the design doesn't require that.
> set_lockfree(....)
> #else
> lock_guard<mutex> l(list_mutex());
> if (const _Node* h = _S_head_owner.get())
> {
> if (h->db.version == new_head_ptr->db.version)
> return h->db;
> new_head_ptr->next = _S_head_owner;
> }
> _S_head_owner = std::move(new_head);
> set_atomic(....)
> #endif
> I think I would preffer that.
>
> return new_head_ptr->db;
>
>
>>
>>
>> libstdc++-v3/ChangeLog:
>>
>> * src/c++20/tzdb.cc (_Node::NumLeapSeconds): New class.
>> (_Node::num_leap_seconds): New static variable.
>> (num_leap_seconds): Remove.
>> (__detail::__recent_leap_second_info): Replace uses of
>> num_leap_seconds with _Node::num_leap_seconds.
>> (_Node::_S_replace_head): Likewise.
>> ---
>>
>> Tested x86_64-linux.
>>
>> libstdc++-v3/src/c++20/tzdb.cc | 106 ++++++++++++++++++++++++++-------
>> 1 file changed, 85 insertions(+), 21 deletions(-)
>>
>> diff --git a/libstdc++-v3/src/c++20/tzdb.cc b/libstdc++-v3/src/c++20/tzdb.cc
>> index 5793155b6d89..17980b6e92ce 100644
>> --- a/libstdc++-v3/src/c++20/tzdb.cc
>> +++ b/libstdc++-v3/src/c++20/tzdb.cc
>> @@ -67,6 +67,7 @@
>> #endif
>>
>> #if USE_ATOMIC_SHARED_PTR && ! USE_ATOMIC_LIST_HEAD
>> +// Cannot use atomic<shared_ptr<T>> without lock-free atomic<T*>.
>> # error Unsupported combination
>> #endif
>>
>> @@ -200,6 +201,10 @@ namespace std::chrono
>>
>> // This is here because _Node is a friend so can call private
>> constructor.
>> static const leap_second fixed_leaps[];
>> +
>> + // This is a member so that it can access fixed_leaps.
>> + struct NumLeapSeconds;
>> + static NumLeapSeconds num_leap_seconds;
>> };
>>
>> // Implementation of the private constructor used for the singleton
>> object.
>> @@ -1301,12 +1306,83 @@ namespace
>> // The expiry date corresponding to the list above.
>> // tzdata 2026a leapseconds list expires at 2026-12-28 00:00:00 UTC
>> constexpr seconds fixed_expiry{1798416000u};
>> -
>> - // This holds the most up-to-date number of leap seconds known at runtime.
>> - // Initially zero, updated when _S_read_leap_seconds() is called.
>> - constinit atomic<unsigned> num_leap_seconds{0};
>> }
>>
>> +// This holds the most up-to-date number of leap seconds known at runtime.
>> +// Initially zero, updated when _S_read_leap_seconds() is called.
>> +struct tzdb_list::_Node::NumLeapSeconds
>> +{
>> + // Called by __recent_leap_second_info to read num_leap_seconds.
>> + unsigned
>> + get()
>> + {
>> +#if ATOMIC_INT_LOCK_FREE == 2
>> + atomic_ref<unsigned> ref(count);
>> + auto num = ref.load(memory_order::relaxed);
>> +
>> + if (num == std::size(_Node::fixed_leaps))
>> + // A leapseconds file has been read and has no new leap seconds.
>> + return num;
>> +
>> + if (num == 0)
>> + // No leapseconds file has been read yet.
>> + return 0;
>> +
>> + // The tzdb_list has been initialized and contains a tzdb object with
>> + // new leap seconds, which the caller is going to use.
>> + // The relaxed load above does not synchronize with anything, so to
>> + // ensure that the get_tzdb_list() in the caller will see a tzdb object
>> + // set by _S_replace_head, we load num_leap_seconds again with acquire
>> + // ordering:
>> + return ref.load(memory_order::acquire);
>
> This leads to improvement in readability, so I think we benefited from the
> change.
>>
>> +#else
>> + lock_guard<mutex> l(list_mutex()); // This ensures acquire ordering.
>> + return count;
>> +#endif
>> + }
>> +
>> + // Called by __recent_leap_second_info to set num_leap_seconds when
>> + // we have determined there are no new leap seconds in a leapseconds file.
>> + void
>> + set_to_fixed_size()
>
> Would split this method instead, as decribed in above.
>>
>> + {
>> +#if ATOMIC_INT_LOCK_FREE == 2
>> + atomic_ref<unsigned> ref(count);
>> + unsigned expected = 0;
>> + ref.compare_exchange_strong(expected, std::size(_Node::fixed_leaps),
>> + memory_order::relaxed);
>> +#else
>>
>> + lock_guard<mutex> l(list_mutex());
>> + if (count == 0)
>> + count = std::size(_Node::fixed_leaps);
>> +#endif
>> + }
>> +
>> + // Called by _Node::_S_replace_head
>> + void
>> + 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);
>> +#elif USE_ATOMIC_SHARED_PTR
>> + // The caller doesn't lock list_mutex() in this case, but it doesn't
>> seem
>> + // realistic to have lock-free atomic pointers and needs a lock for int.
>> +# error Unsupported combination
>> +#else
>> + // XXX The only caller of this function locks list_mutex() so we would
>> + // deadlock if we locked it again here.
>> + count = val;
>> +#endif
>> + }
>> +
>> +private:
>> + unsigned count = 0;
>> +};
>> +
>> +constinit tzdb_list::_Node::NumLeapSeconds
>> tzdb_list::_Node::num_leap_seconds;
>> +
>> namespace __detail
>> {
>> // Called by chrono::__detail::__get_leap_second_info in <chrono>
>> @@ -1368,19 +1444,11 @@ namespace
>>
>> constexpr auto num_fixed_leaps = std::size(_Node::fixed_leaps);
>>
>> - auto num_leaps = num_leap_seconds.load(memory_order::relaxed);
>> + auto num_leaps = _Node::num_leap_seconds.get();
>> if (num_leaps == num_fixed_leaps)
>> // A leapseconds file has been read and has no new leap seconds:
>> return update_info(_Node::fixed_leaps);
>> - else if (num_leaps != 0)
>> - // The tzdb_list has been initialized and contains a tzdb object
>> - // with new leap seconds, which we want to use here.
>> - // The relaxed load above does not synchronize with anything, so to
>> - // ensure that the get_tzdb_list() below will see a tzdb object set
>> - // by _S_replace_head, we load num_leap_seconds again with acquire
>> - // ordering:
>> - (void) num_leap_seconds.load(memory_order::acquire);
>> - else
>> + else if (num_leaps == 0)
>> {
>> // The tzdb_list has not been initialized yet, so we don't know
>> // the correct number of leap seconds.
>> @@ -1389,11 +1457,9 @@ namespace
>> // to parse all of tzdata.zi and initialize a whole tzdb object.
>> if (_Node::_S_read_leap_seconds().first.size() == num_fixed_leaps)
>> {
>> - // There are no new leap seconds. remember that so that the
>> next
>> + // There are no new leap seconds. Remember that so that the
>> next
>> // call to this function can just use fixed_leaps.
>> - num_leap_seconds.compare_exchange_strong(num_leaps,
>> - num_fixed_leaps,
>> -
>> memory_order::relaxed);
>> + _Node::num_leap_seconds.set_to_fixed_size();
>> return update_info(_Node::fixed_leaps);
>> }
>> // else there are new leap seconds. We init tzdb_list so that the
>> @@ -1539,9 +1605,7 @@ namespace
>>
>> // This allows __recent_leap_second_info() to know that it can use
>> // get_tzdb_list()->begin()->leap_seconds to get new leap seconds.
>> - // The release op here synchronizes with the acquire op there.
>> - num_leap_seconds.store(new_head_ptr->db.leap_seconds.size(),
>> - memory_order::release);
>> + num_leap_seconds.set(new_head_ptr->db.leap_seconds.size());
>>
>> return new_head_ptr->db;
>> }
>> --
>> 2.54.0
>>
