Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 25 Mar 2024 12:55:28 +0000
From: Alexander Weps <exander77@...me>
To: Rich Felker <dalias@...c.org>
Cc: musl@...ts.openwall.com
Subject: Re: Broken mktime calculations when crossing DST boundary

See below.

AW

On Monday, March 25th, 2024 at 13:21, Rich Felker <dalias@...c.org> wrote:

> On Mon, Mar 25, 2024 at 11:52:00AM +0000, Alexander Weps wrote:
>
> > This is the simplest and most obvious example how broken the
> > calculation in musl is:
> >
> > void test10()
> > {
> > time_t t = 0;
> > struct tm tm = {0};
> > char buf[64];
> >
> > tm.tm_year = 2011 - 1900;
> > tm.tm_mon = 12 - 1;
> > tm.tm_mday = 29;
> > tm.tm_hour = 0;
> > tm.tm_min = 0;
> > tm.tm_sec = 0;
> > tm.tm_isdst = 0;
> >
> > strftime(buf, sizeof buf, "%F %T %Z", &tm);
> > printf("before: %s %ld %ld\n", buf, t, calc(&tm));
> >
> > t = mktime(&tm);
> >
> > strftime(buf, sizeof buf, "%F %T %Z", &tm);
> > printf("after1: %s %ld %ld\n", buf, t, calc(&tm));
> >
> > tm.tm_mday += 1;
> > t = mktime(&tm);
> >
> > strftime(buf, sizeof buf, "%F %T %Z", &tm);
> > printf("after2: %s %ld %ld\n", buf, t, calc(&tm));
> > }
> >
> > TZ=Pacific/Apia
> > Year is greater than 1970.
> >
> > Input:
> > 2011-12-29 01:00:00 -10
> >
> > Add a day:
> > tm.tm_mday += 1;
> > t = mktime(&tm);
> >
> > Output:
> > 2011-12-29 01:00:00 -10
> >
> > Musl cannot reliably increment date by a day. Incrementing struct tm
> > representing 2011-12-29 01:00:00 -10 by one day leads to the same
> > date.
> >
> > Causing a program to loop or stack overflow.
>
>
> I thought you had found a real bug here, and spent some time working
> out the math by hand on paper because local time is so headbangingly
> awful and confusing. In the end, the conclusion I'm left with is that
> it's working just as expected.

It isn't.

Output from musl:

2011-12-29 01:00:00 -10

    tm.tm_mday += 1;
    t = mktime(&tm);

2011-12-29 01:00:00 -10 <-- date is the same after incrementing

    tm.tm_mday -= 1;
    t = mktime(&tm);

2011-12-28 01:00:00 -10 <-- going below the original date while decrementing

Output from glibc:

2011-12-29 01:00:00 -10

    tm.tm_mday += 1;
    t = mktime(&tm);

2011-12-30 01:00:00 -10 <-- ok

    tm.tm_mday -= 1;
    t = mktime(&tm);

2011-12-29 01:00:00 -10 <-- ok

Hour earlier (same calculations):

Output from musl:

2011-12-29 00:00:00 -10
2011-12-29 00:00:00 -10 <-- date is the same after incrementing
2011-12-28 00:00:00 -10 <-- going below the original date while decrementing

Output from glibc:
2011-12-29 00:00:00 -10
2011-12-30 00:00:00 -10 <-- ok
2011-12-29 00:00:00 -10 <-- ok

Hour after (same calculations).

Output from musl:

2011-12-29 02:00:00 -10
2011-12-29 02:00:00 -10 <-- date is the same after incrementing
2011-12-28 02:00:00 -10 <-- going below the original date while decrementing

Output from glibc:
2011-12-29 02:00:00 -10
2011-12-30 02:00:00 -10 <-- ok
2011-12-29 02:00:00 -10 <-- ok

What are you talking about?


>
> A "spring forward" like this is just like the start of DST, except
> that you can't disambiguate the does-not-exist time with an explicit
> tm_isdst. So all reasoning about what happens is equivalent to the
> much more familar case of start-of-DST with tm_isdst=-1.
>

As I said proclaimed, the issue never was about tm_isdst=-1. Issue is with musl.

> If you take your test program and switch it to initialize with
> tm_mday=31, then do -=1 instead of +=1, you'll find that it gives
> 2011-12-29 01:00:00 -10 as well, only now it seems like the correct,
> expected thing to happen. Any change to "fix" the case you're
> complaining about would necessarily break this case.

So (- day, +day):

Musl:
2011-12-31 01:00:00 +14
2011-12-29 01:00:00 -10
2011-12-29 01:00:00 -10

Glibc:
2012-01-01 01:00:00 +14
2011-12-31 01:00:00 +14
2012-01-01 01:00:00 +14

Seems like musl doesn't even interpret the initial struct tm correctly in that case. It is off by day.

Because December only had 30 days, 31s day after normalization is January 1st.

So no, musl is not correct, it is even more incorrect.

Jesus Christ.

>
> You cannot iterate days by making relative changes to struct tm and
> calling mktime. This just does not work. You could instead iterate
> calendar day inputs yourself, throwing away duplicate outputs
> (resulting from nonexistent days like this one) but that would miss
> days that exist in duplicate on the calendar, where the change happens
> in the opposite direction. What's probably a better approach is
> iterating time_t values (or a struct tm in UTC, using timegm) then,
> for each day, converting to localtime and picking a "start of day"
> time in localtime.
>
> In any case, the core issue you're hitting here is that time zones are
> HARD to work with and that there is inherent complexity that libc
> cannot save you from. You only got lucky that what you were trying to
> do "worked" with glibc because you were iterating days forward; if you
> were doing reverse, it would break exactly the same way.

I am not really commenting on this, until you sort out the above inconsistencies.

>
> Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.