On Tue, Mar 17, 2026 at 2:17 PM 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. > Sorry, I switched "AN" and "AS" in above, but the point apply, for time before 2000 Jan 1 and afterward, we will use the same rules. > 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. > > 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. > >> >> 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 >> >>
