<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Tue, Jun 30, 2026 at 7:46 PM Jonathan Wakely <<a href="mailto:jwakely@redhat.com">jwakely@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">My r17-471-ge79f0f818c0e42 change to optimize handling of leap seconds<br>
introduced a hard dependency on std::atomic<unsigned>, which causes<br>
problems for targets without atomic word operations, like Cortex-M0.:<br>
<a href="https://gcc.gnu.org/pipermail/gcc-patches/2026-June/719704.html" rel="noreferrer" target="_blank">https://gcc.gnu.org/pipermail/gcc-patches/2026-June/719704.html</a><br>
<br>
This patch replaces the num_leap_seconds variable with a struct which<br>
decides whether to use std::atomic_ref<unsigned> or perform all accesses<br>
while holding a lock on the pre-existing mutex used for the tzdb_list<br>
singleton.<br>
<br>
The workaround is a bit ugly, because it assumes that there is only one<br>
caller of num_leap_seconds.set and that the list_mutex() is locked by<br>
that caller iff the tzdb_list doesn't use atomic<shared_ptr<>>. To make<br>
the assumption explicit, there are two different functions used to<br>
update the value, depending on whether the mutex is used or not.<br>
<br>
libstdc++-v3/ChangeLog:<br>
<br>
* src/c++20/tzdb.cc (_Node::NumLeapSeconds): New class.<br>
(_Node::num_leap_seconds): New static variable.<br>
(num_leap_seconds): Remove.<br>
(__detail::__recent_leap_second_info): Replace uses of<br>
num_leap_seconds with _Node::num_leap_seconds.<br>
(_Node::_S_replace_head): Likewise.<br>
---<br>
<br>
v2: Replace NumLeapSeconds::set by two members, set_atomically and<br>
set_locked.<br></blockquote><div>LGTM. Thanks for using separate names. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Tested x86_64-linux.<br>
<br>
libstdc++-v3/src/c++20/tzdb.cc | 119 ++++++++++++++++++++++++++-------<br>
1 file changed, 94 insertions(+), 25 deletions(-)<br>
<br>
diff --git a/libstdc++-v3/src/c++20/tzdb.cc b/libstdc++-v3/src/c++20/tzdb.cc<br>
index 5793155b6d89..9e601fc176f3 100644<br>
--- a/libstdc++-v3/src/c++20/tzdb.cc<br>
+++ b/libstdc++-v3/src/c++20/tzdb.cc<br>
@@ -67,6 +67,7 @@<br>
#endif<br>
<br>
#if USE_ATOMIC_SHARED_PTR && ! USE_ATOMIC_LIST_HEAD<br>
+// Cannot use atomic<shared_ptr<T>> without lock-free atomic<T*>.<br>
# error Unsupported combination<br>
#endif<br>
<br>
@@ -200,6 +201,10 @@ namespace std::chrono<br>
<br>
// This is here because _Node is a friend so can call private constructor.<br>
static const leap_second fixed_leaps[];<br>
+<br>
+ // This is a member so that it can access fixed_leaps.<br>
+ struct NumLeapSeconds;<br>
+ static NumLeapSeconds num_leap_seconds;<br>
};<br>
<br>
// Implementation of the private constructor used for the singleton object.<br>
@@ -1301,12 +1306,85 @@ namespace<br>
// The expiry date corresponding to the list above.<br>
// tzdata 2026a leapseconds list expires at 2026-12-28 00:00:00 UTC<br>
constexpr seconds fixed_expiry{1798416000u};<br>
-<br>
- // This holds the most up-to-date number of leap seconds known at runtime.<br>
- // Initially zero, updated when _S_read_leap_seconds() is called.<br>
- constinit atomic<unsigned> num_leap_seconds{0};<br>
}<br>
<br>
+// This holds the most up-to-date number of leap seconds known at runtime.<br>
+// Initially zero, updated when _S_read_leap_seconds() is called.<br>
+struct tzdb_list::_Node::NumLeapSeconds<br>
+{<br>
+ // Called by __recent_leap_second_info to read num_leap_seconds.<br>
+ unsigned<br>
+ get()<br>
+ {<br>
+#if ATOMIC_INT_LOCK_FREE == 2<br>
+ atomic_ref<unsigned> ref(count);<br>
+ auto num = ref.load(memory_order::relaxed);<br>
+<br>
+ if (num == std::size(_Node::fixed_leaps))<br>
+ // A leapseconds file has been read and has no new leap seconds.<br>
+ return num;<br>
+<br>
+ if (num == 0)<br>
+ // No leapseconds file has been read yet.<br>
+ return 0;<br>
+<br>
+ // The tzdb_list has been initialized and contains a tzdb object with<br>
+ // new leap seconds, which the caller is going to use.<br>
+ // The relaxed load above does not synchronize with anything, so to<br>
+ // ensure that the get_tzdb_list() in the caller will see a tzdb object<br>
+ // set by _S_replace_head, we load num_leap_seconds again with acquire<br>
+ // ordering:<br>
+ return ref.load(memory_order::acquire);<br>
+#else<br>
+ lock_guard<mutex> l(list_mutex()); // This ensures acquire ordering.<br>
+ return count;<br>
+#endif<br>
+ }<br>
+<br>
+ // Called by __recent_leap_second_info to set num_leap_seconds when<br>
+ // we have determined there are no new leap seconds in a leapseconds file.<br>
+ void<br>
+ set_to_fixed_size()<br>
+ {<br>
+#if ATOMIC_INT_LOCK_FREE == 2<br>
+ atomic_ref<unsigned> ref(count);<br>
+ unsigned expected = 0;<br>
+ ref.compare_exchange_strong(expected, std::size(_Node::fixed_leaps),<br>
+ memory_order::relaxed);<br>
+#else<br>
+ lock_guard<mutex> l(list_mutex());<br>
+ if (count == 0)<br>
+ count = std::size(_Node::fixed_leaps);<br>
+#endif<br>
+ }<br>
+<br>
+ // Called by _Node::_S_replace_head<br>
+ // The two versions are named differently so that caller has to be explicit<br>
+ // about which version it calls, based on whether the mutex is held.<br>
+#if ATOMIC_INT_LOCK_FREE == 2<br>
+ void<br>
+ set_atomically(unsigned val)<br>
+ {<br>
+ atomic_ref<unsigned> ref(count);<br>
+ // The release op here synchronizes with the acquire op in get().<br>
+ ref.store(val, memory_order::release);<br>
+ }<br>
+#else<br>
+ void<br>
+ set_locked(unsigned val, const lock_guard<mutex>&)<br>
+ {<br>
+ // XXX The only caller of this function locks list_mutex() so we would<br>
+ // deadlock if we locked it again here.<br>
+ count = val;<br>
+ }<br>
+#endif<br>
+<br>
+private:<br>
+ unsigned count = 0;<br>
+};<br>
+<br>
+constinit tzdb_list::_Node::NumLeapSeconds tzdb_list::_Node::num_leap_seconds;<br>
+<br>
namespace __detail<br>
{<br>
// Called by chrono::__detail::__get_leap_second_info in <chrono><br>
@@ -1368,19 +1446,11 @@ namespace<br>
<br>
constexpr auto num_fixed_leaps = std::size(_Node::fixed_leaps);<br>
<br>
- auto num_leaps = num_leap_seconds.load(memory_order::relaxed);<br>
+ auto num_leaps = _Node::num_leap_seconds.get();<br>
if (num_leaps == num_fixed_leaps)<br>
// A leapseconds file has been read and has no new leap seconds:<br>
return update_info(_Node::fixed_leaps);<br>
- else if (num_leaps != 0)<br>
- // The tzdb_list has been initialized and contains a tzdb object<br>
- // with new leap seconds, which we want to use here.<br>
- // The relaxed load above does not synchronize with anything, so to<br>
- // ensure that the get_tzdb_list() below will see a tzdb object set<br>
- // by _S_replace_head, we load num_leap_seconds again with acquire<br>
- // ordering:<br>
- (void) num_leap_seconds.load(memory_order::acquire);<br>
- else<br>
+ else if (num_leaps == 0)<br>
{<br>
// The tzdb_list has not been initialized yet, so we don't know<br>
// the correct number of leap seconds.<br>
@@ -1389,11 +1459,9 @@ namespace<br>
// to parse all of tzdata.zi and initialize a whole tzdb object.<br>
if (_Node::_S_read_leap_seconds().first.size() == num_fixed_leaps)<br>
{<br>
- // There are no new leap seconds. remember that so that the next<br>
+ // There are no new leap seconds. Remember that so that the next<br>
// call to this function can just use fixed_leaps.<br>
- num_leap_seconds.compare_exchange_strong(num_leaps,<br>
- num_fixed_leaps,<br>
- memory_order::relaxed);<br>
+ _Node::num_leap_seconds.set_to_fixed_size();<br>
return update_info(_Node::fixed_leaps);<br>
}<br>
// else there are new leap seconds. We init tzdb_list so that the<br>
@@ -1525,8 +1593,13 @@ namespace<br>
new_head_ptr->next = curr;<br>
}<br>
// XXX small window here where _S_head_cache still points to previous tzdb.<br>
+ _S_cache_list_head(new_head_ptr);<br>
+<br>
+ // This allows __recent_leap_second_info() to know that it can use<br>
+ // get_tzdb_list()->begin()->leap_seconds to get new leap seconds.<br>
+ num_leap_seconds.set_atomically(new_head_ptr->db.leap_seconds.size());<br>
#else<br>
- lock_guard<mutex> l(list_mutex());<br>
+ lock_guard<mutex> lock(list_mutex());<br>
if (const _Node* h = _S_head_owner.get())<br>
{<br>
if (h->db.version == new_head_ptr->db.version)<br>
@@ -1534,14 +1607,10 @@ namespace<br>
new_head_ptr->next = _S_head_owner;<br>
}<br>
_S_head_owner = std::move(new_head);<br>
-#endif<br>
_S_cache_list_head(new_head_ptr);<br>
<br>
- // This allows __recent_leap_second_info() to know that it can use<br>
- // get_tzdb_list()->begin()->leap_seconds to get new leap seconds.<br>
- // The release op here synchronizes with the acquire op there.<br>
- num_leap_seconds.store(new_head_ptr->db.leap_seconds.size(),<br>
- memory_order::release);<br>
+ num_leap_seconds.set_locked(new_head_ptr->db.leap_seconds.size(), lock);<br>
+#endif<br>
<br>
return new_head_ptr->db;<br>
}<br>
-- <br>
2.54.0<br>
<br>
</blockquote></div></div>