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. > I realized, that this suggestion causes us to override the saved value if the default from rule is ever used, so we should keep your separate bit solution. > 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. > >> + 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(); > lock_guard lock(adopt_lock, _M_impl->rules_counter); > Then update the rules if needs_save_update. > As we need to know previous rule offset (for example for the purposes of computing merge window for patch 5), we could consider computing the end save for are infos that is rule based. And it case when find non zero_save we then could insert expanded zone at the end (or replace it if begin is at the start, or in the merge window). This way we will always know the save of previous zone: * if it is unexpanded, then it is zero (from above) * if its expanded, the value is correct. > > >> + { >> + 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 >> >>
