Paul Eggert wrote:
3. Please construct a third patch containing your mktime test case for glibc, and we then apply that patch to glibc.
I looked at that test case and found some issues with it, e.g., it assumed a particular time_t width in some cases and assumed a particular error number in others. Attached is a revised test case that should fix the issues. For convenience I'm also attaching the same glibc code patch again.
>From 11823e3d7b7aa6126607dbdd9c495f344682ea04 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Fri, 2 Nov 2018 18:49:23 -0700 Subject: [PATCH 1/2] mktime: fix EOVERFLOW bug [BZ#23789] * time/mktime.c [!_LIBC && !DEBUG_MKTIME]: Include libc-config.h, not config.h, for __set_errno. (guess_time_tm, __mktime_internal): Set errno to EOVERFLOW on overflow. --- ChangeLog | 8 ++++++++ time/mktime.c | 21 +++++++++++++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index ef268e8337..231a69b65a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2018-11-04 Paul Eggert <[email protected]> + + mktime: fix EOVERFLOW bug + [BZ#23789] + * time/mktime.c [!_LIBC && !DEBUG_MKTIME]: + Include libc-config.h, not config.h, for __set_errno. + (guess_time_tm, __mktime_internal): Set errno to EOVERFLOW on overflow. + 2018-11-03 Samuel Thibault <[email protected]> * sysdeps/mach/hurd/msync.c: New file. diff --git a/time/mktime.c b/time/mktime.c index 00f0dec6b4..26f7a27516 100644 --- a/time/mktime.c +++ b/time/mktime.c @@ -39,7 +39,7 @@ */ #if !defined _LIBC && !DEBUG_MKTIME -# include <config.h> +# include <libc-config.h> #endif /* Assume that leap seconds are possible, unless told otherwise. @@ -51,6 +51,7 @@ #include <time.h> +#include <errno.h> #include <limits.h> #include <stdbool.h> #include <stdlib.h> @@ -255,8 +256,9 @@ long_int_avg (long_int a, long_int b) If TP is null, return a value not equal to T; this avoids false matches. YEAR and YDAY must not be so large that multiplying them by three times the number of seconds in a year (or day, respectively) would overflow long_int. - If the returned value would be out of range, yield the minimal or - maximal in-range value, except do not yield a value equal to T. */ + If TP is non-null and the returned value would be out of range, set + errno to EOVERFLOW and yield a minimal or maximal in-range value + that is not equal to T. */ static long_int guess_time_tm (long_int year, long_int yday, int hour, int min, int sec, long_int t, const struct tm *tp) @@ -269,9 +271,10 @@ guess_time_tm (long_int year, long_int yday, int hour, int min, int sec, tp->tm_hour, tp->tm_min, tp->tm_sec); if (! INT_ADD_WRAPV (t, d, &result)) return result; + __set_errno (EOVERFLOW); } - /* Overflow occurred one way or another. Return the nearest result + /* An error occurred, probably overflow. Return the nearest result that is actually in range, except don't report a zero difference if the actual difference is nonzero, as that would cause a false match; and don't oscillate between two values, as that would @@ -344,6 +347,8 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *), Use *OFFSET to keep track of a guess at the offset of the result, compared to what the result would be for UTC without leap seconds. If *OFFSET's guess is correct, only one CONVERT call is needed. + If successful, set *TP to the canonicalized struct tm; + otherwise leave *TP alone, return ((time_t) -1) and set errno. This function is external because it is used also by timegm.c. */ time_t __mktime_internal (struct tm *tp, @@ -505,8 +510,12 @@ __mktime_internal (struct tm *tp, sec_adjustment -= sec; sec_adjustment += sec_requested; if (INT_ADD_WRAPV (t, sec_adjustment, &t) - || ! (mktime_min <= t && t <= mktime_max) - || ! convert_time (convert, t, &tm)) + || ! (mktime_min <= t && t <= mktime_max)) + { + __set_errno (EOVERFLOW); + return -1; + } + if (! convert_time (convert, t, &tm)) return -1; } -- 2.19.1
>From b634d4e4025768787e14604e08284e5a3ac0da6e Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Sun, 4 Nov 2018 23:49:03 -0800 Subject: [PATCH 2/2] mktime: new test for mktime failure Based on a test suggested by Albert Aribaud in: https://www.sourceware.org/ml/libc-alpha/2018-10/msg00662.html * time/Makefile (tests): Add bug-mktime4. * time/bug-mktime4.c: New file. --- ChangeLog | 6 +++ time/Makefile | 2 +- time/bug-mktime4.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 time/bug-mktime4.c diff --git a/ChangeLog b/ChangeLog index 231a69b65a..c5b806a54f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2018-11-04 Paul Eggert <[email protected]> + mktime: new test for mktime failure + Based on a test suggested by Albert Aribaud in: + https://www.sourceware.org/ml/libc-alpha/2018-10/msg00662.html + * time/Makefile (tests): Add bug-mktime4. + * time/bug-mktime4.c: New file. + mktime: fix EOVERFLOW bug [BZ#23789] * time/mktime.c [!_LIBC && !DEBUG_MKTIME]: diff --git a/time/Makefile b/time/Makefile index ec3e39dcea..743bd99f18 100644 --- a/time/Makefile +++ b/time/Makefile @@ -43,7 +43,7 @@ tests := test_time clocktest tst-posixtz tst-strptime tst_wcsftime \ tst-getdate tst-mktime tst-mktime2 tst-ftime_l tst-strftime \ tst-mktime3 tst-strptime2 bug-asctime bug-asctime_r bug-mktime1 \ tst-strptime3 bug-getdate1 tst-strptime-whitespace tst-ftime \ - tst-tzname tst-y2039 + tst-tzname tst-y2039 bug-mktime4 include ../Rules diff --git a/time/bug-mktime4.c b/time/bug-mktime4.c new file mode 100644 index 0000000000..9c076eb623 --- /dev/null +++ b/time/bug-mktime4.c @@ -0,0 +1,92 @@ +#include <time.h> +#include <errno.h> +#include <limits.h> +#include <stdbool.h> +#include <stdio.h> +#include <string.h> + +static bool +equal_tm (struct tm const *t, struct tm const *u) +{ + return (t->tm_sec == u->tm_sec && t->tm_min == u->tm_min + && t->tm_hour == u->tm_hour && t->tm_mday == u->tm_mday + && t->tm_mon == u->tm_mon && t->tm_year == u->tm_year + && t->tm_wday == u->tm_wday && t->tm_yday == u->tm_yday + && t->tm_isdst == u->tm_isdst && t->tm_gmtoff == u->tm_gmtoff + && t->tm_zone == u->tm_zone); +} + +static int +do_test (void) +{ + /* Calculate minimum time_t value. This would be simpler with C11, + which has _Generic, but we cannot assume C11. It would also + be simpler with intprops.h, which has TYPE_MINIMUM, but it's + better not to use glibc internals. */ + time_t time_t_min = -1; + time_t_min = (0 < time_t_min ? 0 + : sizeof time_t_min == sizeof (long int) ? LONG_MIN + : sizeof time_t_min == sizeof (long long int) ? LLONG_MIN + : 1); + if (time_t_min == 1) + { + printf ("unknown time type\n"); + return 1; + } + time_t ymin = time_t_min / 60 / 60 / 24 / 366; + bool mktime_should_fail = ymin == 0 || INT_MIN + 1900 < ymin + 1970; + + struct tm tm0 = { .tm_year = INT_MIN, .tm_mday = 1, .tm_wday = -1 }; + struct tm tm = tm0; + errno = 0; + time_t t = mktime (&tm); + long long int llt = t; + bool mktime_failed = tm.tm_wday == tm0.tm_wday; + + if (mktime_failed) + { + if (! mktime_should_fail) + { + printf ("mktime failed but should have succeeded\n"); + return 1; + } + if (errno == 0) + { + printf ("mktime failed without setting errno"); + return 1; + } + if (t != (time_t) -1) + { + printf ("mktime returned %lld but did not set tm_wday\n", llt); + return 1; + } + if (! equal_tm (&tm, &tm0)) + { + printf ("mktime (P) failed but modified *P\n"); + return 1; + } + } + else + { + if (mktime_should_fail) + { + printf ("mktime succeeded but should have failed\n"); + return 1; + } + struct tm *lt = localtime (&t); + if (lt == NULL) + { + printf ("mktime returned a value rejected by localtime\n"); + return 1; + } + if (! equal_tm (lt, &tm)) + { + printf ("mktime result does not match localtime result\n"); + return 1; + } + } + return 0; +} + +#define TIMEOUT 1000 +#include "support/test-driver.c" -- 2.19.1
