Created attachment 249371 [details] Code that triggers the mktime bug The following works on FreeBSD 13.0-RELEASE amd64 but fails on 14.0-RELEASE amd64 (code included as an attachment): # # freebsd-version 13.0-RELEASE # # uname -m amd64 # # cat mktime_bug.c #include <assert.h> #include <err.h> #include <errno.h> #include <stdio.h> #include <stdlib.h> #include <sysexits.h> #include <time.h> int main(void) { struct tm time_buf = { .tm_sec = 0, .tm_min = 0, .tm_hour = 0, .tm_mday = 1, .tm_mon = 0, .tm_year = 69, .tm_wday = 3, .tm_yday = 0, .tm_isdst = 0, .tm_zone = "CET", }; errno = 0; time_t const unix_time = mktime(&time_buf); if (errno) err(EX_DATAERR, "mktime failed"); printf("Unix time is: %ld\n", unix_time); return 0; } # # cc -g -O0 -o mktime_bug mktime_bug.c # ./mktime_bug Unix time is: -31539600 # Whereas on 14.0-RELEASE: $ freebsd-version 14.0-RELEASE-p5 $ $ uname -m amd64 $ $ cc -g -O0 -o mktime_bug mktime_bug.c $ ./mktime_bug mktime_bug: mktime failed: Value too large to be stored in data type $ Note that the requested year is 1969 - the bug is possibly related to this fact. This might be a regression due to a version bump of tzcode. This bug was discovered due to a test failure in schilytools at https://codeberg.org/schilytools/schilytools/issues/66.
I can confirm this is a bug even in 13.3-RELEASE : callisto$ callisto$ ./foo foo: mktime failed: Value too large to be stored in data type callisto$ uname -apKU FreeBSD callisto 13.3-RELEASE FreeBSD 13.3-RELEASE releng/13.3-n257428-80d2b634ddf0 GENERIC amd64 amd64 1303001 1303001 callisto$ Dennis Clarke
We verified that it seems to work jusrt fine in a 13.0 and 13.1 jail : root@:~ # freebsd-version -ru 14.0-RELEASE-p5 13.1-RELEASE root@:~ # ./foo Unix time is: -31536000 root@:~ #
Seems the breakage is somewhere between 13.1 and 13.2 : root@:~ # cc -g -O0 -o foo foo.c root@:~ # root@:~ # ./foo foo: mktime failed: Value too large to be stored in data type root@:~ # freebsd-version -ru 14.0-RELEASE-p5 13.2-RELEASE root@:~ #
Stepping through mktime, I see a binary search in the range INT_MIN to INT_MAX seconds that fails because time -4611686018427387904 seconds does not convert cleanly to a 32 bit year number when using 32 bit intermediate variables. In this code y = -109624180. if (ckd_add(&tmp->tm_year, y, -TM_YEAR_BASE)) { errno = EOVERFLOW; return NULL; }
I meant LONG_MIN and LONG_MAX. 64 bits signed, not 32 bits signed.
(QEMU FreeBSD 14.0-RELEASE Test not actual hardware) Big Endian root@freebsd14_ppc64:~ # ./foo Unix time is: -31518000 root@freebsd14_ppc64:~ # Little Endian root@freebsd14_ppc64le:~ # ./foo foo: mktime failed: Value too large to be stored in data type root@freebsd14_ppc64le:~ #
CC des@, who did the last tzcode merge. This may need to be bisected (sigh...)
Your code assumes that errno will be unchanged if mktime() succeeds. This is incorrect. Here is a better test program: #include <errno.h> #include <stdio.h> #include <time.h> int main(void) { struct tm tm = { .tm_mday = 1, .tm_year = 69, .tm_yday = -1 }; time_t t = mktime(&tm); if (tm.tm_yday == -1) { perror("mktime failed"); return 1; } printf("%04d-%02d-%02d %02d:%02d:%02d %s = %ld\n", tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec, tzname[0], (long)t); return 0; } If you want to argue that errno _should_ be zero if mktime() succeeds, I invite you to make that argument to the tzcode maintainers (https://www.iana.org/time-zones), the Austin Group (https://www.austingroupbugs.net/), and the C standardization committee (https://www.open-std.org/jtc1/sc22/wg14/), but please take the time to familiarize yourself with prior work in this area (notably WG14 N3147) first.
There is no mention of errno anywhere in the manpage CTIME(3).
Dennis, see https://pubs.opengroup.org/onlinepubs/9699919799/functions/mktime.html
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=7534109d13a6cdb22e78d9d4c0a0cd5efd323c45 commit 7534109d13a6cdb22e78d9d4c0a0cd5efd323c45 Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2024-03-27 10:03:37 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-03-27 10:03:37 +0000 libc: Improve description of mktime() / timegm(). * Mention that mktime() and timegm() set errno on failure. * Correctly determining whether mktime() / timegm() succeeded with arbitrary input (where -1 can be a valid result) is non-trivial. Document the recommended procedure. PR: 277863 MFC after: 1 week Reviewed by: pauamma_gundo.com, gbe Differential Revision: https://reviews.freebsd.org/D44503 lib/libc/stdtime/ctime.3 | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=fdde01c5b44904e34ff3b003f446644de865fa21 commit fdde01c5b44904e34ff3b003f446644de865fa21 Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2024-03-27 10:03:37 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-04-04 09:41:41 +0000 libc: Improve description of mktime() / timegm(). * Mention that mktime() and timegm() set errno on failure. * Correctly determining whether mktime() / timegm() succeeded with arbitrary input (where -1 can be a valid result) is non-trivial. Document the recommended procedure. PR: 277863 MFC after: 1 week Reviewed by: pauamma_gundo.com, gbe Differential Revision: https://reviews.freebsd.org/D44503 (cherry picked from commit 7534109d13a6cdb22e78d9d4c0a0cd5efd323c45) lib/libc/stdtime/ctime.3 | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=662dc74155ada993513a08190032710cf224e47f commit 662dc74155ada993513a08190032710cf224e47f Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2023-04-26 09:46:41 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-04-04 09:51:02 +0000 tzcode: Clean up the ctime(3) manual page. MFC after: 3 weeks Sponsored by: Klara, Inc. Reviewed by: pauamma_gundo.com Differential Revision: https://reviews.freebsd.org/D39714 (cherry picked from commit 6f3c2f41b18b9a7a8feb3c50254a21db2d9908fd) libc: Improve description of mktime() / timegm(). * Mention that mktime() and timegm() set errno on failure. * Correctly determining whether mktime() / timegm() succeeded with arbitrary input (where -1 can be a valid result) is non-trivial. Document the recommended procedure. PR: 277863 MFC after: 1 week Reviewed by: pauamma_gundo.com, gbe Differential Revision: https://reviews.freebsd.org/D44503 (cherry picked from commit 7534109d13a6cdb22e78d9d4c0a0cd5efd323c45) lib/libc/stdtime/ctime.3 | 207 ++++++++++++++++++++++++----------------------- 1 file changed, 104 insertions(+), 103 deletions(-)