[PATCH] libstdc++: Remove dependency on std::atomic<unsigned> in tzdb.cc
Tomasz Kaminski
tkaminsk@redhat.com
Thu Jun 25 12:00:56 GMT 2026
On Thu, Jun 25, 2026 at 1:42 PM Jonathan Wakely <jwakely@redhat.com> 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:
#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.
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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20260625/5671ede6/attachment.htm>
More information about the Libstdc++
mailing list