Re: [musl] Broken mktime calculations when crossing DST boundary

[Alexander Weps <exander77@pm.me>]

I don't think time can go backwards by incrementing field under any conditions.

Going from:
tm_sec: 2
tm_min: 60
tm_hour: 1
tm_mday: 31
tm_mon: 2
tm_year: 124
tm_wday: 0
tm_yday: 90
tm_isdst: -1

To:
tm_sec: 2
tm_min: 0
tm_hour: 1
tm_mday: 31
tm_mon: 2
tm_year: 124
tm_wday: 0
tm_yday: 90
tm_isdst: 0

Seems to be plain wrong. I cannot come up with any argument for this being correct under any conditions.

mktime was given a struct tm with uncertain STD/DST, it deduced it is STD and then thrown away 60 minute information. The minutes got reset from 60 to 0 and no other change was done.

It claims that 01:00:00 + 60 minutes is 01:00:00

It should be:
02:00:00 (wrong, but at least makes sense)
Or it should be:
03:00:00 (correct)

How it could be?
01:00:00

This leads to:
01:59:00 + 1 minute to be 01:00:00

Time is going backwards by incrementing fields.

I looked when the tm_isdst = -1 was introduced in the library I am working on and it comments with this:
---
 CRON_USE_LOCAL_TIME: use DST offsets

mktime(3) will automatically account for daylight savings time offsets
if the tm_isdst field of the struct tm is set to -1:

    http://pubs.opengroup.org/onlinepubs/009695399/functions/strptime.html

~~~
tm.tm_isdst = -1;      /* Not set by strptime(); tells mktime()
                          to determine whether daylight saving time
                          is in effect */
~~~

ctime(3) also mentions:

    tm_isdst A flag that indicates whether daylight saving time is in
             effect at the time described. The value is positive if daylight
             saving time is in effect, zero if it is not, and negative if the
             information is not available.
---

Looking at this, I think musl completely ignores the detection part.

If minutes are set to 60 (or above), the struct tm was created by incrementing (going forward in time).
If minutes are set to -1 (or below), the struct tm was created by decrementing (going backward in time).

This applies to all fields. So there is enough information to know how was DST boundary crossed.

I am just looking into glibc code and there is a pretty significant part that handles what I assume is this detection.

I don't see any code in musl that handles this, except for some comparison in __secs_to_zone.

It has enough information to correctly deduce STD/DST, but it does not do it at all.

AW


On Saturday, March 23rd, 2024 at 13:31, Rich Felker <dalias@libc.org> wrote:

> On Sat, Mar 23, 2024 at 12:00:52PM +0000, Alexander Weps wrote:
>
> > This is how it should work as far as I am concerned. After
> > manipulating the date, the tm_isdst is set to -1.
> >
> > > From https://cplusplus.com/reference/ctime/tm/:
>
>
> This site is notoriously sloppy if not outright wrong about pretty
> much everything.
>
> > The Daylight Saving Time flag (tm_isdst) is greater than zero if
> > Daylight Saving Time is in effect, zero if Daylight Saving Time is
> > not in effect, and less than zero if the information is not
> > available.
>
>
> The actual specification is at
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/mktime.html
> (POSIX version; the C version is not independently linkable but says
> basically the same thing anyway). The relevant text, which differs
> from the above, is:
>
> "A negative value for tm_isdst shall cause mktime() to attempt to
> determine whether Daylight Savings Time is in effect for the
> specified time."
>
> No details are specified for how this determination is made, so it may
> differ between implementations. At DST transition boundaries, it's
> impossible to define unambiguously. There are either times that don't
> exist, or that could mean two different things.
>
> If there are cases where our current behavior isn't consistent (the
> input time does not match the output in either DST or non-DST), it
> seems like those should be treated as bugs and fixed. But if you're
> just expecting the implementation to guess whether your nonexistant
> time came from moving forward from non-DST or backward from DST, it
> can't; it just had to make an arbitrary choice, and that choice is not
> going to agree across different systems.
>
> > And the date should be correctly set as DST or STD in mktime.
> >
> > I have a date. I make change in fields, I set tm_isdst = -1, I call
> > mktime.
>
>
> If you already have a date and want a new date offset by some fixed
> amount of time from it, setting tm_isdst is wrong. You leave tm_isdst
> alone and adjust whichever field(s) (e.g. tm_min) you want to offset
> by, then call mktime. The resulting tm_isdst (and correspondingly
> other fields) may have changed if you crossed a DST boundary by making
> the offset.
>
> When you set tm_isdst=-1, you're throwing away information and
> explicitly asking the implementation to guess what an ambiguous
> timestamp meant.
>
> Does this help? Do you think there's an actual bug to be investigated
> on our side here?
>
> Rich