Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 920 bytes --]

Lukasz Majewski wrote:
> Paul, will you find any time soon to provide next version of mktime
> compatibility patch?

As far as mktime compatibility goes, I think I'd rather first cause mktime to be 
compatible with __time64_t, and worry about _TIME_BITS later. Proposed mktime 
patch attached; it's similar to my previous proposal except it leaves the 
__time64_t definition in posix/bits/types.h rather than moving it to 
time/mktime-internal.h.

This patch exposes __time64_t to user code if _LIBC is defined (because glibc 
needs __time64_t internally), or if __TIMESIZE != 64 (because of the compromise 
to expose __time64_t only on 32-bit time_t platforms). Once we add _TIME_BITS I 
expect that we should change the visibility rule to something that also involves 
_TIME_BITS, because __time64_t should not be exposed to user code if _TIME_BITS 
== 64. However, that can wait for the patches involving _TIME_BITS.

[-- Attachment #2: 0001-Make-mktime-etc.-compatible-with-__time64_t.patch --]
[-- Type: text/x-patch, Size: 18033 bytes --]

From 1a7cfd679d96b48335731c65f3e4e4d2e57fd086 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 18 Mar 2019 14:14:15 -0700
Subject: [PATCH] Make mktime etc. compatible with __time64_t

Keep these functions compatible with Gnulib while adding
__time64_t support.  The basic idea is to move private API
declarations from include/time.h to time/mktime-internal.h, since
the former file cannot easily be shared with Gnulib whereas the
latter can.
Also, do some other minor cleanup while in the neighborhood.
* include/time.h: Include stdbool.h, time/mktime-internal.h.
(__mktime_internal): Move this prototype to time/mktime-internal.h,
since Gnulib needs it.
(__localtime64_r, __gmtime64_r) [__TIMESIZE == 64]:
Move these macros to time/mktime-internal.h, since Gnulib needs them.
(__mktime64, __timegm64) [__TIMESIZE != 64]: New prototypes.
(in_time_t_range): New static function.
* posix/bits/types.h (__time64_t) [__TIMESIZE == 64 && !defined __LIBC]:
Do not define as a macro in this case, so that portable code is
less tempted to use __time64_t.
* time/mktime-internal.h: Rewrite so that it does both glibc
and Gnulib work.  Include time.h if not _LIBC.
(mktime_offset_t) [!_LIBC]: Define for gnulib.
(__time64_t, __gmtime64_r, __localtime64_r, __mktime64, __timegm64)
[!_LIBC || __TIMESIZE == 64]: New macros, mostly moved here
from include/time.h.
(__gmtime_r, __localtime_r, __mktime_internal) [!_LIBC]:
New macros, taken from GNulib.
(__mktime_internal): New prototype, moved here from include/time.h.
* time/mktime.c (mktime_min, mktime_max, convert_time)
(ranged_convert, __mktime_internal, __mktime64):
* time/timegm.c (__timegm64):
Use __time64_t, not time_t.
* time/mktime.c: Stop worrying about whether time_t is floating-point.
(__mktime64) [! (_LIBC && __TIMESIZE != 64)]:
Rename from mktime.
(mktime) [_LIBC && __TIMESIZE != 64]: New function.
* time/timegm.c [!_LIBC]: Include libc-config.h, not config.h,
for libc_hidden_def.
Include errno.h.
(__timegm64) [! (_LIBC && __TIMESIZE != 64)]:
Rename from timegm.
(timegm) [_LIBC && __TIMESIZE != 64]: New function.

First cut at publicizing __time64_t
---
 ChangeLog              | 44 +++++++++++++++++++++++
 include/time.h         | 39 +++++++++++----------
 posix/bits/types.h     |  9 +++--
 time/mktime-internal.h | 79 +++++++++++++++++++++++++++++++++++++++++-
 time/mktime.c          | 71 ++++++++++++++++++++++---------------
 time/timegm.c          | 32 ++++++++++++++---
 6 files changed, 220 insertions(+), 54 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index aac356bb69..e513c0ea06 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,47 @@
+2019-04-28  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Make mktime etc. compatible with __time64_t
+	Keep these functions compatible with Gnulib while adding
+	__time64_t support.  The basic idea is to move private API
+	declarations from include/time.h to time/mktime-internal.h, since
+	the former file cannot easily be shared with Gnulib whereas the
+	latter can.
+	Also, do some other minor cleanup while in the neighborhood.
+	* include/time.h: Include stdbool.h, time/mktime-internal.h.
+	(__mktime_internal): Move this prototype to time/mktime-internal.h,
+	since Gnulib needs it.
+	(__localtime64_r, __gmtime64_r) [__TIMESIZE == 64]:
+	Move these macros to time/mktime-internal.h, since Gnulib needs them.
+	(__mktime64, __timegm64) [__TIMESIZE != 64]: New prototypes.
+	(in_time_t_range): New static function.
+	* posix/bits/types.h (__time64_t): Move to time/mktime-internal.h,
+	so that glibc users are not tempted to use __time64_t.
+	* time/mktime-internal.h: Rewrite so that it does both glibc
+	and Gnulib work.  Include time.h if not _LIBC.
+	(mktime_offset_t) [!_LIBC]: Define for gnulib.
+	(__time64_t): New type or macro, moved here from
+	posix/bits/types.h.
+	(__gmtime64_r, __localtime64_r, __mktime64, __timegm64)
+	[!_LIBC || __TIMESIZE == 64): New macros, mostly moved here
+	from include/time.h.
+	(__gmtime_r, __localtime_r, __mktime_internal) [!_LIBC]:
+	New macros, taken from GNulib.
+	(__mktime_internal): New prototype, moved here from include/time.h.
+	* time/mktime.c (mktime_min, mktime_max, convert_time)
+	(ranged_convert, __mktime_internal, __mktime64):
+	* time/timegm.c (__timegm64):
+	Use __time64_t, not time_t.
+	* time/mktime.c: Stop worrying about whether time_t is floating-point.
+	(__mktime64) [! (_LIBC && __TIMESIZE != 64)]:
+	Rename from mktime.
+	(mktime) [_LIBC && __TIMESIZE != 64]: New function.
+	* time/timegm.c [!_LIBC]: Include libc-config.h, not config.h,
+	for libc_hidden_def.
+	Include errno.h.
+	(__timegm64) [! (_LIBC && __TIMESIZE != 64)]:
+	Rename from timegm.
+	(timegm) [_LIBC && __TIMESIZE != 64]: New function.
+
 2019-04-26  Florian Weimer  <fweimer@redhat.com>
 
 	elf: Link sotruss-lib.so with BIND_NOW for --enable-bind-now.
diff --git a/include/time.h b/include/time.h
index 61dd9e180b..ac3163c2a5 100644
--- a/include/time.h
+++ b/include/time.h
@@ -3,6 +3,8 @@
 
 #ifndef _ISOMAC
 # include <bits/types/locale_t.h>
+# include <stdbool.h>
+# include <time/mktime-internal.h>
 
 extern __typeof (strftime_l) __strftime_l;
 libc_hidden_proto (__strftime_l)
@@ -49,19 +51,11 @@ extern void __tzset_parse_tz (const char *tz) attribute_hidden;
 extern void __tz_compute (__time64_t timer, struct tm *tm, int use_localtime)
   __THROW attribute_hidden;
 
-/* Subroutine of `mktime'.  Return the `time_t' representation of TP and
-   normalize TP, given that a `struct tm *' maps to a `time_t' as performed
-   by FUNC.  Record next guess for localtime-gmtime offset in *OFFSET.  */
-extern time_t __mktime_internal (struct tm *__tp,
-				 struct tm *(*__func) (const time_t *,
-						       struct tm *),
-				 long int *__offset) attribute_hidden;
-
 #if __TIMESIZE == 64
 # define __ctime64 ctime
 #else
 extern char *__ctime64 (const __time64_t *__timer) __THROW;
-libc_hidden_proto (__ctime64);
+libc_hidden_proto (__ctime64)
 #endif
 
 #if __TIMESIZE == 64
@@ -69,7 +63,7 @@ libc_hidden_proto (__ctime64);
 #else
 extern char *__ctime64_r (const __time64_t *__restrict __timer,
 		          char *__restrict __buf) __THROW;
-libc_hidden_proto (__ctime64_r);
+libc_hidden_proto (__ctime64_r)
 #endif
 
 #if __TIMESIZE == 64
@@ -81,13 +75,13 @@ libc_hidden_proto (__localtime64)
 
 extern struct tm *__localtime_r (const time_t *__timer,
 				 struct tm *__tp) attribute_hidden;
-
-#if __TIMESIZE == 64
-# define __localtime64_r __localtime_r
-#else
+#if __TIMESIZE != 64
 extern struct tm *__localtime64_r (const __time64_t *__timer,
 				   struct tm *__tp);
 libc_hidden_proto (__localtime64_r)
+
+extern __time64_t __mktime64 (struct tm *__tp) __THROW;
+libc_hidden_proto (__mktime64)
 #endif
 
 extern struct tm *__gmtime_r (const time_t *__restrict __timer,
@@ -99,14 +93,13 @@ libc_hidden_proto (__gmtime_r)
 #else
 extern struct tm *__gmtime64 (const __time64_t *__timer);
 libc_hidden_proto (__gmtime64)
-#endif
 
-#if __TIMESIZE == 64
-# define __gmtime64_r __gmtime_r
-#else
 extern struct tm *__gmtime64_r (const __time64_t *__restrict __timer,
 				struct tm *__restrict __tp);
-libc_hidden_proto (__gmtime64_r);
+libc_hidden_proto (__gmtime64_r)
+
+extern __time64_t __timegm64 (struct tm *__tp) __THROW;
+libc_hidden_proto (__timegm64)
 #endif
 
 /* Compute the `struct tm' representation of T,
@@ -155,5 +148,13 @@ extern double __difftime (time_t time1, time_t time0);
    actual clock ID.  */
 #define CLOCK_IDFIELD_SIZE	3
 
+/* Check whether T fits in time_t.  */
+static inline bool
+in_time_t_range (__time64_t t)
+{
+  time_t s = t;
+  return s == t;
+}
+
 #endif
 #endif
diff --git a/posix/bits/types.h b/posix/bits/types.h
index 0de6c59bb4..cb737caff0 100644
--- a/posix/bits/types.h
+++ b/posix/bits/types.h
@@ -213,10 +213,13 @@ __STD_TYPE __U32_TYPE __socklen_t;
    It is not currently necessary for this to be machine-specific.  */
 typedef int __sig_atomic_t;
 
-#if __TIMESIZE == 64
+/* Seconds since the Epoch, visible to user code when time_t is too
+   narrow only for consistency with the old way of widening too-narrow
+   types.  User code should never use __time64_t.  */
+#if __TIMESIZE == 64 && defined __LIBC
 # define __time64_t __time_t
-#else
-__STD_TYPE __TIME64_T_TYPE __time64_t;	/* Seconds since the Epoch.  */
+#elif __TIMESIZE != 64
+__STD_TYPE __TIME64_T_TYPE __time64_t;
 #endif
 
 #undef __STD_TYPE
diff --git a/time/mktime-internal.h b/time/mktime-internal.h
index 6111c22880..4cdeb9c2f4 100644
--- a/time/mktime-internal.h
+++ b/time/mktime-internal.h
@@ -1,2 +1,79 @@
-/* Gnulib mktime-internal.h, tailored for glibc.  */
+/* Internals of mktime and related functions
+   Copyright 2016-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Paul Eggert <eggert@cs.ucla.edu>.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _LIBC
+# include <time.h>
+#endif
+
+/* mktime_offset_t is a signed type wide enough to hold a UTC offset
+   in seconds, and used as part of the type of the offset-guess
+   argument to mktime_internal.  In Glibc, it is always long int.
+   When in Gnulib, use time_t on platforms where time_t
+   is signed, to be compatible with platforms like BeOS that export
+   this implementation detail of mktime.  On platforms where time_t is
+   unsigned, GNU and POSIX code can assume 'int' is at least 32 bits
+   which is wide enough for a UTC offset.  */
+#ifdef _LIBC
 typedef long int mktime_offset_t;
+#elif defined TIME_T_IS_SIGNED
+typedef time_t mktime_offset_t;
+#else
+typedef int mktime_offset_t;
+#endif
+
+/* The source code uses identifiers like __time64_t for glibc
+   timestamps that can contain 64-bit values even when time_t is only
+   32 bits.  These are just macros for the ordinary identifiers unless
+   compiling within glibc when time_t is 32 bits.  */
+#if ! (defined _LIBC && __TIMESIZE != 64)
+# undef __time64_t
+# define __time64_t time_t
+# define __gmtime64_r __gmtime_r
+# define __localtime64_r __localtime_r
+# define __mktime64 mktime
+# define __timegm64 timegm
+#endif
+
+#ifndef _LIBC
+
+/* Although glibc source code uses leading underscores, Gnulib wants
+   ordinary names.
+
+   Portable standalone applications should supply a <time.h> that
+   declares a POSIX-compliant localtime_r, for the benefit of older
+   implementations that lack localtime_r or have a nonstandard one.
+   Similarly for gmtime_r.  See the gnulib time_r module for one way
+   to implement this.  */
+
+# undef __gmtime_r
+# undef __localtime_r
+# define __gmtime_r gmtime_r
+# define __localtime_r localtime_r
+
+# define __mktime_internal mktime_internal
+
+#endif
+
+/* Subroutine of mktime.  Return the time_t representation of TP and
+   normalize TP, given that a struct tm * maps to a time_t as performed
+   by FUNC.  Record next guess for localtime-gmtime offset in *OFFSET.  */
+extern __time64_t __mktime_internal (struct tm *tp,
+                                     struct tm *(*func) (__time64_t const *,
+                                                         struct tm *),
+                                     mktime_offset_t *offset) attribute_hidden;
diff --git a/time/mktime.c b/time/mktime.c
index 57efee9b25..8fe2f963ae 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -112,11 +112,11 @@ my_tzset (void)
    added to them, and then with another timestamp added, without
    worrying about overflow.
 
-   Much of the code uses long_int to represent time_t values, to
-   lessen the hassle of dealing with platforms where time_t is
+   Much of the code uses long_int to represent __time64_t values, to
+   lessen the hassle of dealing with platforms where __time64_t is
    unsigned, and because long_int should suffice to represent all
-   time_t values that mktime can generate even on platforms where
-   time_t is excessively wide.  */
+   __time64_t values that mktime can generate even on platforms where
+   __time64_t is wider than the int components of struct tm.  */
 
 #if INT_MAX <= LONG_MAX / 4 / 366 / 24 / 60 / 60
 typedef long int long_int;
@@ -144,16 +144,15 @@ shr (long_int a, int b)
 	  : a / (one << b) - (a % (one << b) < 0));
 }
 
-/* Bounds for the intersection of time_t and long_int.  */
+/* Bounds for the intersection of __time64_t and long_int.  */
 
 static long_int const mktime_min
-  = ((TYPE_SIGNED (time_t) && TYPE_MINIMUM (time_t) < TYPE_MINIMUM (long_int))
-     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (time_t));
+  = ((TYPE_SIGNED (__time64_t)
+      && TYPE_MINIMUM (__time64_t) < TYPE_MINIMUM (long_int))
+     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (__time64_t));
 static long_int const mktime_max
-  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (time_t)
-     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (time_t));
-
-verify (TYPE_IS_INTEGER (time_t));
+  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (__time64_t)
+     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (__time64_t));
 
 #define EPOCH_YEAR 1970
 #define TM_YEAR_BASE 1900
@@ -252,23 +251,23 @@ tm_diff (long_int year, long_int yday, int hour, int min, int sec,
 }
 
 /* Use CONVERT to convert T to a struct tm value in *TM.  T must be in
-   range for time_t.  Return TM if successful, NULL (setting errno) on
+   range for __time64_t.  Return TM if successful, NULL (setting errno) on
    failure.  */
 static struct tm *
-convert_time (struct tm *(*convert) (const time_t *, struct tm *),
+convert_time (struct tm *(*convert) (const __time64_t *, struct tm *),
 	      long_int t, struct tm *tm)
 {
-  time_t x = t;
+  __time64_t x = t;
   return convert (&x, tm);
 }
 
 /* Use CONVERT to convert *T to a broken down time in *TP.
    If *T is out of range for conversion, adjust it so that
    it is the nearest in-range value and then convert that.
-   A value is in range if it fits in both time_t and long_int.
+   A value is in range if it fits in both __time64_t and long_int.
    Return TP on success, NULL (setting errno) on failure.  */
 static struct tm *
-ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
+ranged_convert (struct tm *(*convert) (const __time64_t *, struct tm *),
 		long_int *t, struct tm *tp)
 {
   long_int t1 = (*t < mktime_min ? mktime_min
@@ -310,7 +309,7 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
 }
 
 
-/* Convert *TP to a time_t value, inverting
+/* Convert *TP to a __time64_t value, inverting
    the monotonic and mostly-unit-linear conversion function CONVERT.
    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.
@@ -318,9 +317,9 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
    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
+__time64_t
 __mktime_internal (struct tm *tp,
-		   struct tm *(*convert) (const time_t *, struct tm *),
+		   struct tm *(*convert) (const __time64_t *, struct tm *),
 		   mktime_offset_t *offset)
 {
   struct tm tm;
@@ -520,9 +519,9 @@ __mktime_internal (struct tm *tp,
 
 #if defined _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS
 
-/* Convert *TP to a time_t value.  */
-time_t
-mktime (struct tm *tp)
+/* Convert *TP to a __time64_t value.  */
+__time64_t
+__mktime64 (struct tm *tp)
 {
   /* POSIX.1 8.1.1 requires that whenever mktime() is called, the
      time zone names contained in the external variable 'tzname' shall
@@ -531,7 +530,7 @@ mktime (struct tm *tp)
 
 # if defined _LIBC || NEED_MKTIME_WORKING
   static mktime_offset_t localtime_offset;
-  return __mktime_internal (tp, __localtime_r, &localtime_offset);
+  return __mktime_internal (tp, __localtime64_r, &localtime_offset);
 # else
 #  undef mktime
   return mktime (tp);
@@ -539,11 +538,29 @@ mktime (struct tm *tp)
 }
 #endif /* _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS */
 
-#ifdef weak_alias
-weak_alias (mktime, timelocal)
+#if defined _LIBC && __TIMESIZE != 64
+
+libc_hidden_def (__mktime64)
+
+time_t
+mktime (struct tm *tp)
+{
+  struct tm tm = *tp;
+  __time64_t t = __mktime64 (&tm);
+  if (in_time_t_range (t))
+    {
+      *tp = tm;
+      return t;
+    }
+  else
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+}
+
 #endif
 
-#ifdef _LIBC
+weak_alias (mktime, timelocal)
 libc_hidden_def (mktime)
 libc_hidden_weak (timelocal)
-#endif
diff --git a/time/timegm.c b/time/timegm.c
index bfd36d0255..bae0ceee5e 100644
--- a/time/timegm.c
+++ b/time/timegm.c
@@ -18,17 +18,41 @@
    <http://www.gnu.org/licenses/>.  */
 
 #ifndef _LIBC
-# include <config.h>
+# include <libc-config.h>
 #endif
 
 #include <time.h>
+#include <errno.h>
 
 #include "mktime-internal.h"
 
-time_t
-timegm (struct tm *tmp)
+__time64_t
+__timegm64 (struct tm *tmp)
 {
   static mktime_offset_t gmtime_offset;
   tmp->tm_isdst = 0;
-  return __mktime_internal (tmp, __gmtime_r, &gmtime_offset);
+  return __mktime_internal (tmp, __gmtime64_r, &gmtime_offset);
 }
+
+#if defined _LIBC && __TIMESIZE != 64
+
+libc_hidden_def (__timegm64)
+
+time_t
+timegm (struct tm *tmp)
+{
+  struct tm tm = *tmp;
+  __time64_t t = __timegm64 (&tm);
+  if (in_time_t_range (t))
+    {
+      *tmp = tm;
+      return t;
+    }
+  else
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+}
+
+#endif
-- 
2.20.1