Bug 277863
| Summary: | Possible regression in mktime(3) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Nico Sonack <nsonack> | ||||
| Component: | misc | Assignee: | freebsd-bugs (Nobody) <bugs> | ||||
| Status: | Closed Works As Intended | ||||||
| Severity: | Affects Many People | CC: | brennenmurph, dclarke, des, fuz, jfc, nsonack | ||||
| Priority: | --- | ||||||
| Version: | 14.0-RELEASE | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| URL: | https://codeberg.org/schilytools/schilytools/issues/66 | ||||||
| Attachments: |
|
||||||
|
Description
Nico Sonack
2024-03-21 13:32:32 UTC
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). 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(-) |
