This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v4] Ensure mktime sets errno on error (bug 23789)
- From: Paul Eggert <eggert at cs dot ucla dot edu>
- To: "Albert ARIBAUD (3ADEV)" <albert dot aribaud at 3adev dot fr>
- Cc: libc-alpha at sourceware dot org, Gnulib bugs <bug-gnulib at gnu dot org>
- Date: Sun, 4 Nov 2018 23:54:59 -0800
- Subject: Re: [PATCH v4] Ensure mktime sets errno on error (bug 23789)
- References: <20181030111850.5322-1-albert.aribaud@3adev.fr> <9947f682-fc46-f2d6-4b07-536c03b9e657@cs.ucla.edu>
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 <eggert@cs.ucla.edu>
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 <eggert@cs.ucla.edu>
+
+ 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 <samuel.thibault@ens-lyon.org>
* 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 <eggert@cs.ucla.edu>
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 <eggert@cs.ucla.edu>
+ 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