Incorrect transition times in std::chrono::time_zone::get_info()

[Jonathan Wakely]

On Fri, 26 Jul 2024 at 11:56, Edward Welbourne <edward.welbourne@qt.io> wrote:
>
> On Fri, 26 Jul 2024 at 11:04, Edward Welbourne <edward.welbourne@qt.io> wrote:
> >> One issue is that the supported range of times is rather limited; Qt
> >> uses a 64-bit signed count of milliseconds from the UTC start of
> >> 1970, which does admittedly have an insanely wide range, but (I am
> >> happy to report) glibc's time_t functions seem to cope with that
> >> whole range just fine.
>
> Jonathan Wakely (26 July 2024 12:41) replied:
> > For a 64-bit time_t they'll have an even larger range, since they'll
> > work with a 64-bit signed number of seconds, not milliseconds.
> >
> > For 32-bit time_t, you have bigger problems anyway.
>
> Indeed, but the Qt code knows about that and kludges round it.
>
> >> However, the std::chrono::time_zone::get_info() range is
> >> considerably narrower (I haven't yet pinned down its limits).  That's
> >> bearable, since the out-of-range times are of course only ever seen in
> >> artificial test-cases; and get_info() raises a std::runtime_error for
> >> out-of-range values, so we can catch that and cope by other kludges
> >> (that we need on other platforms anyway).
>
> > What exactly is the problem you see? Do you have a testcase for this?
>
> static void lateInfo(void)
> {
>     using namespace std::chrono;
>     constexpr long long huge = 0x7fffffffffffffff;
>     constexpr auto late = sys_time<milliseconds>(milliseconds(huge));
>     const time_zone *zone = idToZone("Europe/Oslo");
>     const sys_info info = zone->get_info(late); // std::runtime_error
>     std::cout << '\n' << info.abbrev << '\n';
> }
>
> Slap a call to this into the start of my previous main(), for example,
> to see the exception thrown.  It's using the same idToZone() as before.

I replaced idToZone with chrono::locate_zone just because.

Your sys_time is greater than sys_days(year::max()/January/1), so I
think libstdc++ gives up calculating transitions past that point and
can't give you the info.

I find it hard to be motivated to support get_info past the 33rd
millennium, I don't expect this code to still be in use by the time of
the Horus Heresy.

>
> > time_zone::get_info converts its argument to sys_seconds and calls
> > _M_get_sys_info. That should have exactly the same range as time_t
> > (64-bit signed number of seconds, with 1970-01-01 as the epoch).
>
> I'm on a 64-bit system, where the time_t functions cope with times such
> as the one above, and this is a 64-bit number of milliseconds out, so
> should be well within a 64-bit second range.

But not within the chrono::year::max() upper limit.

I could add an explicit check for times past that limit and give a
better what() in the exception (and not bother calculating 64 thousand
transitions before realising the requested time point is still too
high).


>
> > There's a known bug (well, known to me) where libstdc++ should
> > inerpret the transition time relative to the current wall clock time,
> > not UTC. There are a couple of relevant comments in the code:
> >
> >     // XXX The ri.until() time point should be
> >     // "interpreted using the rules in effect just before the transition"
> >     info.end = ri.until();
> >
> >       // XXX UNTIL field should be interpreted
> >       // "using the rules in effect just before the transition"
> >       // so might need to store as year_month_day and hh_mm_ss and only
> >       // convert to a sys_time once we know the offset in effect.
> >       inf.m_until = sys_days(year(y)/m.m/day(d)) + seconds(t.time);
>
> That matches my suspicions.
>
> > So if a transition should happen at (for example) 2am local time,
> > libstdc++ might be applying it at 2am UTC in some cases.
>
> That fits some of the cases in the list I gave before.
> Others seem to be using UTC 00:00.

For example, this one:
Asia/Bishkek        : 2005-08-12 06:00:00 +06

The transition in that case is:

5 KG +05/+06 2005 Au 12
6 - +06

Since the UNTIL field is just Au 12 without a TIME, it occurred at
midnight, but it should be midnight standard time, not midnight UTC.
So it's the same bug.

>
> >> So I first want to check whether I'm using the API right and, if so, to
>
> > I'll take a look at your testcase and check if it matches my
> > understanding.
>
> Thanks - I'm relatively new to the C++20 tzdb stuff, so won't be
> entirely surprised if I've got some crossed wires.
>
> >> know where to report the bug.
>
> > All libstdc++ bugs should be reported in GCC's bugzilla.
>
> Duly noted, thanks - and thanks for the prompt reply,
>
>         Eddy.
>
>