On Mon, May 11, 2026 at 4:55 PM Tomasz Kaminski <[email protected]> wrote:
> > > On Mon, Apr 27, 2026 at 1:55 AM Álvaro Begué <[email protected]> > wrote: > >> The previous patches for PR 116110 left one case unresolved: a Zone >> line with a wall-time UNTIL whose RULES field is a named rule set. >> The save value used to convert the wall UNTIL to UTC depends on which >> rule of the set was active at the UNTIL instant, but at parse time >> the rule records have not all been loaded, so the active-rule lookup >> can't yet be performed. >> >> The remaining FIXME in operator>>(istream&, ZoneInfo&) caused zones >> like Africa/Algiers (around 1977-10-21) to place their zone-line >> boundary one save-period off from the canonical zic interpretation, >> producing brief incorrect sys_info windows during DST transitions. >> >> This commit defers the save adjustment to a fixup pass run after >> ranges::stable_sort(node->rules) at the end of reload_tzdb. >> >> A new ZoneInfo::m_until_save_pending bit (stolen from m_pos:15, which >> becomes m_pos:14 -- still 16384 max, far above any realistic offset >> into m_buf) marks pending entries. The parser sets this bit when >> it sees a wall-time UNTIL on a named-rule line and skips the save >> subtraction. The fixup pass walks every zone's ZoneInfos, looks up >> the active rule, and applies the deferred adjustment. >> >> The active-rule lookup uses a new helper find_pre_until_rule() that >> walks all (rule, year) pairs in chronological order and maintains a >> running save value, so wall-time rules' TIME fields are interpreted >> relative to the cascaded save state. The boundary it compares >> against shrinks as the running save grows, which gives zic.c's >> interpretation: a rule firing at exactly the wall UNTIL belongs to >> the next zone line, not the current one. >> >> The lazy-expansion seeding code that finds the active rule at >> info.begin is also updated to use a half-open `rule_start < t` >> window with `t = info.begin + 1s`, so a rule firing at exactly >> info.begin (the new line's first instant) is correctly identified >> as in force. Without this, the new line would seed with the wrong >> save and the first sys_info would have the wrong total offset and >> abbreviation. >> >> The test_apia case in 116110.cc had a hardcoded `+11h` workaround >> for the unfixed bug; with this fix in place the workaround is removed >> and the value becomes the canonical `+10h`. >> >> libstdc++-v3/ChangeLog: >> >> PR libstdc++/116110 >> * src/c++20/tzdb.cc (ZoneInfo): Add m_until_save_pending bit >> (stolen from m_pos:15) and accessors until_save_pending(), >> set_until_save_pending(), clear_until_save_pending(), and >> adjust_until(). >> (find_pre_until_rule): New function. Chronological cascade >> walker with iterative-boundary semantics, used by the post- >> parse fixup pass. >> (operator>>(istream&, ZoneInfo&)): Set m_until_save_pending >> when the wall UNTIL on a named-rule line cannot have its save >> subtracted at parse time. Replaces the FIXME. >> (time_zone::_M_get_sys_info): Change the seeding active- >> rule lookup to use t = info.begin + 1s, so a rule firing at >> exactly info.begin is included. >> (reload_tzdb): After sorting node->rules, run a fixup pass over >> every ZoneInfo with until_save_pending set, calling >> find_pre_until_rule and adjust_until. >> * testsuite/std/time/time_zone/116110.cc (test_apia): Remove >> the +11h workaround for the unfixed named-rule UNTIL bug; the >> canonical +10h boundary is now produced. >> * testsuite/std/time/time_zone/pr116110_named.cc: New test. >> >> Signed-off-by: Álvaro Begué <[email protected]> >> --- >> libstdc++-v3/src/c++20/tzdb.cc | 122 +++++++++++++++++- >> .../testsuite/std/time/time_zone/116110.cc | 5 +- >> .../std/time/time_zone/pr116110_named.cc | 74 +++++++++++ >> 3 files changed, 196 insertions(+), 5 deletions(-) >> create mode 100644 >> libstdc++-v3/testsuite/std/time/time_zone/pr116110_named.cc >> >> diff --git a/libstdc++-v3/src/c++20/tzdb.cc >> b/libstdc++-v3/src/c++20/tzdb.cc >> index c0d62bc35..648b9f85a 100644 >> --- a/libstdc++-v3/src/c++20/tzdb.cc >> +++ b/libstdc++-v3/src/c++20/tzdb.cc >> @@ -518,6 +518,20 @@ namespace std::chrono >> sys_seconds >> until() const noexcept { return m_until; } >> >> + // True if this is a named-rule zone line whose wall-time UNTIL >> still >> + // needs its SAVE adjustment applied. See reload_tzdb for the >> fixup. >> + bool >> + until_save_pending() const noexcept { return m_until_save_pending; >> } >> + >> + void >> + set_until_save_pending() noexcept { m_until_save_pending = 1; } >> > + >> + void >> + clear_until_save_pending() noexcept { m_until_save_pending = 0; } >> + >> + void >> + adjust_until(seconds s) noexcept { m_until -= s; } >> > I would set m_until_save_pending here; this is only function that sets > this bit. > > >> + >> friend istream& operator>>(istream&, ZoneInfo&); >> >> bool >> @@ -571,8 +585,9 @@ namespace std::chrono >> } >> >> string m_buf; // rules() + ' ' + format() OR letters + ' ' + >> format() >> - uint_least16_t m_pos : 15 = 0; // offset of format() in m_buf >> + uint_least16_t m_pos : 14 = 0; // offset of format() in m_buf >> uint_least16_t m_expanded : 1 = 0; >> + uint_least16_t m_until_save_pending : 1 = 0; >> > Instead of using a separate bit, I would put some int_least32_t min value > as the save, to indicate that the value is not usable. I doubt any > reasonable > rule would use such save value. This way the flag will be automatically > cleared > when save is set. We could also use it to differentiate between unexpanded > zone > for which save is know (was computed), and maybe allow us to simplify find > zone. > >> duration<int_least16_t, ratio<60>> m_save{}; >> sec32_t m_offset{}; >> sys_seconds m_until{}; >> @@ -679,6 +694,81 @@ namespace std::chrono >> } >> #endif >> }; >> + >> + // Find the Rule whose save value is in force at the wall-time UNTIL >> + // of a Zone line, given that `wall_minus_stdoff` is the line's UNTIL >> + // with STDOFF subtracted and `stdoff` is the line's standard offset. >> + // >> + // Walks (rule, year) pairs chronologically, maintaining a running >> + // save value used to interpret subsequent Wall-indicator rules. >> + // The boundary `wall_minus_stdoff - running_save` shrinks as save >> + // accumulates, so a rule firing AT the boundary is treated as >> + // belonging to the next zone line. >> + // >> + // The calendar window extends by one year on each side to catch >> + // rules whose wall at_time crosses a year boundary in UT due to a >> + // large stdoff or save. >> + template<typename _RuleRange> >> + const Rule* >> + find_pre_until_rule(const _RuleRange& rules, >> + sys_seconds wall_minus_stdoff, seconds stdoff) >> + { >> + if (rules.empty()) >> + return nullptr; >> + >> + const year last_year >> + = year_month_day{chrono::floor<days>(wall_minus_stdoff)}.year() >> + + years(1); >> + year first_year = year::max(); >> + for (const auto& r : rules) >> + if (r.from < first_year) >> + first_year = r.from; >> + if (first_year > last_year) >> + return nullptr; >> + >> + struct Pending >> + { >> + const Rule* rule; >> + year y; >> + sys_seconds approx_when; >> + }; >> + vector<Pending> pending; >> > This function is called many times in the loop, and we allocate memory > each time. > I think declaring the vector and passing it by reference outside the loop > would be better. > I think the best way to achieve that would be to replace this function with a functor. i.e. class with operator() and a vector of Pending as a member. This way ithe allocation could get reused between calls easily. > + pending.reserve(64); >> + for (year y = first_year; y <= last_year; ++y) >> + for (const auto& r : rules) >> + { >> + if (y < r.from || y > r.to) >> + continue; >> + seconds approx_off{}; >> + if (r.when.indicator == at_time::Wall >> + || r.when.indicator == at_time::Standard) >> + approx_off = stdoff; >> + pending.push_back({&r, y, r.start_time(y, approx_off)}); >> + } >> + std::sort(pending.begin(), pending.end(), >> + [](const Pending& a, const Pending& b) { >> + return a.approx_when < b.approx_when; >> + }); >> + >> + seconds running_save{}; >> + sys_seconds boundary = wall_minus_stdoff; >> + const Rule* last_fired = nullptr; >> + for (const auto& p : pending) >> + { >> + seconds offset{}; >> + if (p.rule->when.indicator == at_time::Wall) >> + offset = stdoff + running_save; >> + else if (p.rule->when.indicator == at_time::Standard) >> + offset = stdoff; >> + sys_seconds fire = p.rule->start_time(p.y, offset); >> + if (fire >= boundary) >> + continue; >> + last_fired = p.rule; >> + running_save = p.rule->save; >> + boundary = wall_minus_stdoff - running_save; >> + } >> + return last_fired; >> + } >> } // namespace >> #endif // TZDB_DISABLED >> >> @@ -867,7 +957,9 @@ namespace std::chrono >> >> if (letters.empty()) >> { >> - sys_seconds t = info.begin - seconds(1); >> + // info.begin + 1s makes the strict `rule_start < t` search >> + // inclusive of a rule that fires at exactly info.begin. >> + sys_seconds t = info.begin + seconds(1); >> const year_month_day date(chrono::floor<days>(t)); >> >> // Try to find a Rule active before this time, to get initial >> @@ -1625,6 +1717,27 @@ namespace std::chrono >> ranges::sort(node->db.links, {}, &time_zone_link::name); >> ranges::stable_sort(node->rules, {}, &Rule::name); >> >> + // For every Zone line whose UNTIL was a wall-time expression on a >> + // named-rule line, the parser deferred the SAVE adjustment because >> + // the active rule was not yet identifiable. Now that all Rule >> + // records are loaded and indexed, find the rule active just before >> + // the wall UNTIL and subtract its save from m_until. >> + for (const auto& tz : node->db.zones) >> > The libstc++ on purpose avoids doing any computation on the time zones, > that are not queried, so you do not pay for what you do not use. But here > we > are looking at all possible zone values. > > I think we should consider doing that fixup in _M_get_sys_info. To do it > only > once we can use the highest bit on rules_counter (and have a bool > indicating if a mutex is used) > to indicate that a rule needs updating. The the lock method could > be changed to rename that bit, so instead of: > // Prevent concurrent access to _M_impl->infos if it might need to > change. > lock_guard lock(_M_impl->rules_counter); > We would have: > // Prevent concurrent access to _M_impl->infos if it might need to > change. > bool needs_save_update = _M_impl->rules_counter; > lock_guard lock(adopt_lock, _M_impl->rules_counter); > Then update the rules if needs_save_update. > > >> + { >> + auto& infos = tz._M_impl->infos; >> + for (auto& info : infos) >> + { >> + if (!info.until_save_pending()) >> + continue; >> + auto rules = ranges::equal_range(node->rules, info.rules(), >> + ranges::less{}, &Rule::name); >> + if (const Rule* r >> + = find_pre_until_rule(rules, info.until(), info.offset())) >> + info.adjust_until(seconds(r->save)); >> + info.clear_until_save_pending(); >> + } >> + } >> + >> return Node::_S_replace_head(std::move(head), std::move(node)); >> #else >> __throw_disabled(); >> @@ -2386,7 +2499,10 @@ namespace std::chrono >> { >> if (inf.m_expanded) // Not a named Rule, SAVE is known now. >> inf.m_until -= inf.m_save; >> - // else Named Rule, SAVE is unknown. FIXME: PR 116110 >> + else >> + // Named Rule: defer SAVE adjustment until reload_tzdb >> + // has loaded all Rule records. >> + inf.set_until_save_pending(); >> } >> } >> } >> diff --git a/libstdc++-v3/testsuite/std/time/time_zone/116110.cc >> b/libstdc++-v3/testsuite/std/time/time_zone/116110.cc >> index 26b9ba33c..7827387b5 100644 >> --- a/libstdc++-v3/testsuite/std/time/time_zone/116110.cc >> +++ b/libstdc++-v3/testsuite/std/time/time_zone/116110.cc >> @@ -65,8 +65,9 @@ test_apia() >> auto* tz = locate_zone("Pacific/Apia"); >> local_seconds t = local_days(2011y/December/29) + 24h; >> >> - // FIXME: this should be + 10h but we do not account for DST yet, so + >> 11h. >> - sys_seconds ut(t.time_since_epoch() + 11h ); >> + // The wall UNTIL is interpreted in the prior offset (-11h + save 1h >> + // = -10h), so the boundary is at local_days + 24h + 10h. >> + sys_seconds ut(t.time_since_epoch() + 10h ); >> sys_info info; >> info = tz->get_info(ut - 1s); >> VERIFY( info.offset == (-11h + info.save) ); >> diff --git a/libstdc++-v3/testsuite/std/time/time_zone/pr116110_named.cc >> b/libstdc++-v3/testsuite/std/time/time_zone/pr116110_named.cc >> new file mode 100644 >> index 000000000..b3bf4eb1a >> --- /dev/null >> +++ b/libstdc++-v3/testsuite/std/time/time_zone/pr116110_named.cc >> @@ -0,0 +1,74 @@ >> +// { dg-do run { target c++20 } } >> +// { dg-require-effective-target tzdb } >> +// { dg-require-effective-target cxx11_abi } >> +// { dg-xfail-run-if "no weak override on AIX" { powerpc-ibm-aix* } } >> + >> +// Africa/Algiers 1977-10-21: a Zone line whose RULES references a >> +// named Rule and whose UNTIL is a wall-time expression. The wall >> +// UNTIL is interpreted using the SAVE value in force just before the >> +// boundary (the May-6 rule's save=1, not the Oct-21 rule's save=0 >> +// even though the Oct-21 rule fires at the same wall instant). >> +// >> +// Rule d 1977 May 6 0:00 wall save=1 >> +// Rule d 1977 Oct 21 0:00 wall save=0 >> +// Z A 0 d WE%sT 1977 O 21 >> +// 1 d CE%sT >> + >> +#include <chrono> >> +#include <fstream> >> +#include <testsuite_hooks.h> >> + >> +static bool override_used = false; >> + >> +namespace __gnu_cxx >> +{ >> + const char* zoneinfo_dir_override() { >> + override_used = true; >> + return "./"; >> + } >> +} >> + >> +int >> +main() >> +{ >> + using namespace std::chrono; >> + >> + std::ofstream("tzdata.zi") << R"(# version test_pr116110_named >> +R d 1977 o - May 6 0 1 S >> +R d 1977 o - O 21 0 0 - >> +Z Test/Algiers 0 d WE%sT 1977 O 21 >> + 1 d CE%sT >> +)"; >> + >> + const auto& db = reload_tzdb(); >> + VERIFY( override_used ); >> + VERIFY( db.version == "test_pr116110_named" ); >> + >> + auto* tz = locate_zone("Test/Algiers"); >> + >> + // Just before the boundary: still in the first Zone line under >> + // the May-6 rule (save=1, WEST, total +1). >> + auto pre = tz->get_info(sys_days{1977y/October/20} + 22h); >> + VERIFY( pre.offset == 1h ); >> + VERIFY( pre.save == 1h ); >> + VERIFY( pre.abbrev == "WEST" ); >> + >> + // The boundary is Oct 20 23:00 UTC (= wall 00:00 - stdoff(0) - >> save(1)). >> + // At and after the boundary we are in the second line (CET). >> + auto at = tz->get_info(sys_days{1977y/October/20} + 23h); >> + VERIFY( at.offset == 1h ); // stdoff 1 + save 0 (CET, second line) >> + VERIFY( at.save == 0min ); >> + VERIFY( at.abbrev == "CET" ); >> + >> + // A second query inside the second line, well clear of the boundary. >> + auto after = tz->get_info(sys_days{1977y/October/21} + 12h); >> + VERIFY( after.offset == 1h ); >> + VERIFY( after.save == 0min ); >> + VERIFY( after.abbrev == "CET" ); >> + >> + // A query inside the [Oct 20 23:00, Oct 21 00:00] UTC window must be >> + // in the second line, not in a leftover stretch from the first line. >> + auto window = tz->get_info(sys_days{1977y/October/20} + 23h + 30min); >> + VERIFY( window.offset == 1h ); >> + VERIFY( window.abbrev == "CET" ); >> +} >> -- >> 2.34.1 >> >>
