On Tue, Mar 17, 2026 at 6:16 PM Jonathan Wakely <[email protected]> wrote:
> On Tue, 17 Mar 2026 at 13:18, Tomasz Kaminski <[email protected]> wrote: > > > > > > > > On Mon, Mar 16, 2026 at 11:55 PM Jonathan Wakely <[email protected]> > wrote: > >> > >> The Australia/Broken_Hill example in PR libstdc++/116110 demonstrates a > >> bug in the time zone code. The current code gives incorrect results when > >> a zone changes from one named Rule to another during DST, e.g. the > >> Broken_Hill time zone uses: > >> > >> Zone Australia/Broken_Hill 9:25:48 - LMT 1895 Feb > >> 10 - AEST 1896 Aug 23 > >> 9 - ACST 1899 May > >> 9:30 AU AC%sT 1971 > >> 9:30 AN AC%sT 2000 > >> 9:30 AS AC%sT > >> > >> So the AN Rules take effect on 2000 Jan 1 which is during DST (which > >> runs from October to March). > > > > Now, I understand the problem: the end of AS 2000 Jan 1, which according > > to the application is: > > R AS 1987 2007 - O lastSu 2s 1 D > > However, we seem to be searching the wrong rule list because we are using > > the "AN" rules to find previous value of DST, not the AS. This still > gives us > > expected save value, as I assume the rule switch is not meant to cause > abrupt > > changes to the actual wall time, and the changes in wall clock happen > accordingly > > to DST. > > Right. > > > > > I.e. we make assumption that at the time of the rule change, the save > value > > is same according to new and old rules. > > > > So I think the code is technically incorrect, but practically correct. > To be exact, > > we would need look use the rule set from previous iterator in that case. > > I think we're handling this case wrong, but with the patch we give the > right result for this time zone. > > The zic(8) man page says: > > A zone or continuation line L with a named rule set starts with > standard > time by default: that is, any of L's timestamps preceding L's > earliest > rule use the rule in effect after L's first transition into > standard > time. > I think that describes how the rule is interpreted for dates before its first entry, i.e. 1971 for both AN and AS: R AN 1971 1985 - O lastSu 2s 1 D R AN 1972 o - F 27 2s 0 S R AN 1973 1981 - Mar Su>=1 2s 0 S R AS 1971 1985 - O lastSu 2s 1 D R AS 1986 o - O 19 2s 1 D R AS 1987 2007 - O lastSu 2s 1 D R AS 1972 o - F 27 2s 0 S R AS 1973 1985 - Mar Su>=1 2s 0 S > I think that means that we should not actually change from the AN rule > to the AS rule on Jan 1st 2001, we should continue using the AN rule > until the first transition to standard time of the AS rule. > Given that the entries may also switch the offset, I think they meant to apply until the end date, for example, the Kirimiti zone that switched between sides o International Day Line: Z Pacific/Kiritimati -10:29:20 - LMT 1901 -10:40 - %z 1979 O -10 - %z 1994 D 31 14 - %z Switch happened on 23:59:59 21th December of 1994. > I'll see if that's reasonable to implement, because there could be > cases where what this patch does is wrong e.g. if the AN and AS rules > used different formats, say AC%sT and AC%sT2, then we should not > change from ACDT to ACDT2 on Jan 1, but should continue using ACDT > until the end of DST, then change to ACT, then change to ACDT2 in > October 2001. It doesn't matter in this case because both rules use > the same offset and the same format, so it's not detectable that we > change to a new rule on Jan 1. > > > > >> > >> > >> The fix for this is to update info.offset and info.save when we find an > >> active rule, instead of only updating the 'letters' variable. > >> > >> libstdc++-v3/ChangeLog: > >> > >> PR libstdc++/116110 > >> * src/c++20/tzdb.cc (time_zone::_M_get_sys_info): Update > >> info.offset and info.save to values from the active rule. > >> * testsuite/std/time/time_zone/116110.cc: New test. > >> --- > >> > >> Tested x86_64-linux. > > > > Add the explanation of the assumption that save value at rule change is > same > > as with previous rules, and we are OK. The comment before ri.offset() > would > > benefit from update. > >> > >> > >> libstdc++-v3/src/c++20/tzdb.cc | 6 ++++- > >> .../testsuite/std/time/time_zone/116110.cc | 22 +++++++++++++++++++ > >> 2 files changed, 27 insertions(+), 1 deletion(-) > >> create mode 100644 libstdc++-v3/testsuite/std/time/time_zone/116110.cc > >> > >> diff --git a/libstdc++-v3/src/c++20/tzdb.cc > b/libstdc++-v3/src/c++20/tzdb.cc > >> index 53441880ae6e..e26a9ee38072 100644 > >> --- a/libstdc++-v3/src/c++20/tzdb.cc > >> +++ b/libstdc++-v3/src/c++20/tzdb.cc > >> @@ -917,7 +917,11 @@ namespace std::chrono > > > > > > > > > > This change is inside if and can be completely omitted: > > string_view letters; > > if (i != infos.begin() && i[-1].expanded()) > > letters = i[-1].next_letters(); > > > > if (letters.empty()) > > { > > And I wondered if caching the next_letters is causing any trouble > > here (i.e. we should have also handle saving in this situation), however > > The code below ensures that list of expanded zones always ends with the > > save == 0, so it is correct. > > > > > > > >> > >> } > >> > >> if (active_rule) > >> - letters = active_rule->letters; > >> + { > >> + info.offset = ri.offset() + active_rule->save; > >> + info.save = > chrono::duration_cast<minutes>(active_rule->save); > >> + letters = active_rule->letters; > >> + } > >> else if (first_std) > >> letters = first_std->letters; > >> } > >> diff --git a/libstdc++-v3/testsuite/std/time/time_zone/116110.cc > b/libstdc++-v3/testsuite/std/time/time_zone/116110.cc > >> new file mode 100644 > >> index 000000000000..b52e83b293cc > >> --- /dev/null > >> +++ b/libstdc++-v3/testsuite/std/time/time_zone/116110.cc > >> @@ -0,0 +1,22 @@ > >> +// { dg-do run { target c++20 } } > >> +// { dg-require-effective-target tzdb } > >> +// { dg-require-effective-target cxx11_abi } > >> + > >> +#include <chrono> > >> +#include <testsuite_hooks.h> > >> + > >> +void > >> +test_broken_hill() > >> +{ > >> + using namespace std::chrono; > >> + auto* tz = locate_zone("Australia/Broken_Hill"); > >> > >> + auto info = tz->get_info(sys_days(2000y/February/29d) + 23h + 23min > + 23s); > >> + VERIFY( info.offset == 630min ); > >> + VERIFY( info.save == 60min ); > >> + VERIFY( info.abbrev == "ACDT" ); > >> +} > >> + > >> +int main() > >> +{ > >> + test_broken_hill(); > >> +} > >> -- > >> 2.53.0 > >> > >
