[v2,1/2] Y2038: make __mktime_internal compatible with __time64_t
diff mbox series

Message ID bf6ca98d-da93-0677-99d3-86c80e5470a2@cs.ucla.edu
State New
Headers show
Series
  • [v2,1/2] Y2038: make __mktime_internal compatible with __time64_t
Related show

Commit Message

Paul Eggert March 18, 2019, 9:23 p.m. UTC
On 3/11/19 11:58 PM, Lukasz Majewski wrote:
> The question is if there is a more suitable place than include/time.h
> header to have fits_in_time() reused by both Glibc and Gnulib?

Hmm, well, on further thought, fits_in_time_t can live in include/time.h
since only glibc will need it. Gnulib won't support two kinds of time_t
types, so it should't need the function. (The attached patch renames it
to "in_time_t_range" to avoid *_t pollution.)

That being said, we will have to make a place for private .h code shared
between glibc and Gnulib, because include/time.h can't easily be shared
with Gnulib. I suggest using time/mktime-internal.h for this

Some other comments on that patch that I noticed while looking into this.

The patch added duplicate "#include <errno.h>" lines to time/timegm.c.

I still don't get why we need __timelocal64. Glibc code can use
__mktime64. User code shouldn't see that symbol.

Come to think of it, user code shouldn't see __time64_t either. Yes, the
name begins with two underscores so user code should avoid it, but
putting __time64_t in /usr/include will just tempt users. It's easy to
keep __time64_t out of /usr/include so let's do that.

The body of __mktime64 can be simplified; there's no need for locals of
type __time64_t, time_t, and struct tm. And it should use
in_time_t_range instead of doing it by hand.

The bodies of mktime and timegm [__TIMESIZE != 64] should not pass tp to
__mktime64/__timegm64 directly, since *tp should not be updated if the
result is in __time64_t range but out of time_t range. And timegm should
use in_time_t_range instead of doing it by hand.

The "#ifdef weak_alias" etc. lines can be removed now, since Gnulib does
this stuff OK now.

timegm.c should include libc-config.h, not config.h, as this is the
current Gnulib style for code shared with Gnulib.

Revised proposed patch attached.

Comments

Lukasz Majewski March 19, 2019, 1:39 p.m. UTC | #1
Hi Paul,

> On 3/11/19 11:58 PM, Lukasz Majewski wrote:
> > The question is if there is a more suitable place than
> > include/time.h header to have fits_in_time() reused by both Glibc
> > and Gnulib?  
> 

Thanks for your patch.

> Hmm, well, on further thought, fits_in_time_t can live in
> include/time.h since only glibc will need it. Gnulib won't support
> two kinds of time_t types, so it should't need the function. (The
> attached patch renames it to "in_time_t_range" to avoid *_t
> pollution.)

Is the rewritten code correct?

+/* Check whether T fits in time_t.  */
+static inline bool
+in_time_t_range (__time64_t t)
+{
+  time_t s = t;
+  return s != t;
+}

Shouldn't we have: return s == t; ?

> 
> That being said, we will have to make a place for private .h code
> shared between glibc and Gnulib, because include/time.h can't easily
> be shared with Gnulib. I suggest using time/mktime-internal.h for this
> 

ACK/NAK for using time/mktime-internl.h shall be done by more
experienced glibc developer(s).

> Some other comments on that patch that I noticed while looking into
> this.
> 
> The patch added duplicate "#include <errno.h>" lines to time/timegm.c.

Ok.

> 
> I still don't get why we need __timelocal64. Glibc code can use
> __mktime64. User code shouldn't see that symbol.

If we don't need it for this conversion, then we should omit it. I will
add it if needed when preparing Y2038 patch set.

In the time/mktime.c there is:
weak_alias (mktime, timelocal), which makes the timelocal calls
aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
__REDIRECT will be added).

> 
> Come to think of it, user code shouldn't see __time64_t either. Yes,
> the name begins with two underscores so user code should avoid it,
> posix/bits/types.hbut putting __time64_t in /usr/include will just
> tempt users. It's easy to keep __time64_t out of /usr/include so
> let's do that.
> 

Is that the reason for removing __time64_t definition from
posix/bits/types.h ?

> The body of __mktime64 can be simplified; there's no need for locals
> of type __time64_t, time_t, and struct tm. And it should use
> in_time_t_range instead of doing it by hand.

Yes. Also there is no need to check for EOVERFLOW in the __mktime64()
function (as it operates inherently on 64 bit types).

> 
> The bodies of mktime and timegm [__TIMESIZE != 64] should not pass tp
> to __mktime64/__timegm64 directly, since *tp should not be updated if
> the result is in __time64_t range but out of time_t range. And timegm
> should use in_time_t_range instead of doing it by hand.
> 

Ok.

> The "#ifdef weak_alias" etc. lines can be removed now, since Gnulib
> does this stuff OK now.
> 
> timegm.c should include libc-config.h, not config.h, as this is the
> current Gnulib style for code shared with Gnulib.
> 
> Revised proposed patch attached.
> 

Just out of curiosity:

In the time/mktime-internal.h you added a comment regarding BeOS users
and posix time_t - do you know any :-) ?



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Paul Eggert March 19, 2019, 11:12 p.m. UTC | #2
Lukasz Majewski wrote:

> Shouldn't we have: return s == t; ?

Yes, absolutely. Thanks for catching that. I tested only the Gnulib version, and 
Gnulib doesn't use that code.

Do you have glibc tests to catch bugs like this? If no, please add writing some 
tests to your lists of things to do.

> In the time/mktime.c there is:
> weak_alias (mktime, timelocal), which makes the timelocal calls
> aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> __REDIRECT will be added).

Sorry, I'm a bit lost here. How will that __REDIRECT work, exactly? Should it be 
part of this patch, or part of a later patch?

>> Come to think of it, user code shouldn't see __time64_t either....
> 
> Is that the reason for removing __time64_t definition from
> posix/bits/types.h ?

Yes.
> In the time/mktime-internal.h you added a comment regarding BeOS users
> and posix time_t - do you know any :-) ?

Just one. :-)  See:

https://lists.gnu.org/archive/html/bug-gnulib/2011-05/msg00470.html

Bruno's most recent BeOS-related submission to Gnulib was in October 2017:

https://lists.gnu.org/r/bug-gnulib/2017-10/msg00098.html
Lukasz Majewski March 20, 2019, 7:03 a.m. UTC | #3
Hi Paul,

> Lukasz Majewski wrote:
> 
> > Shouldn't we have: return s == t; ?  
> 
> Yes, absolutely. Thanks for catching that. I tested only the Gnulib
> version, and Gnulib doesn't use that code.

Do you plan to prepare (and send to mailing list) the next version of
this patch (including the above fix)?

> 
> Do you have glibc tests to catch bugs like this?

Actually yes:
https://github.com/lmajewski/y2038-tests/commits/master

> If no, please add
> writing some tests to your lists of things to do.
> 

The plan is to port above tests to glibc's test suite (as now they are
standalone).

There is also the Yocto/OE meta layer dedicated for testing/developing
Y2038 glibc with qemu:
https://github.com/lmajewski/meta-y2038/commits/master

And the Y2038 safe glibc:
https://github.com/lmajewski/y2038_glibc/commits/Y2038-2.29-glibc-11-03-2019


This code is going to be pushed also to sourceware.org git.

> > In the time/mktime.c there is:
> > weak_alias (mktime, timelocal), which makes the timelocal calls
> > aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> > __REDIRECT will be added).  
> 
> Sorry, I'm a bit lost here. How will that __REDIRECT work, exactly?
> Should it be part of this patch, or part of a later patch?

The __REDIRECT would be a part of the latter patch - the one which adds
Y2038 support for 32 bit SoCs.

It would simply redirect calls to mktime/timegm to internal
__mktime64()/__timegm64().

> 
> >> Come to think of it, user code shouldn't see __time64_t
> >> either....  
> > 
> > Is that the reason for removing __time64_t definition from
> > posix/bits/types.h ?  
> 
> Yes.
> > In the time/mktime-internal.h you added a comment regarding BeOS
> > users and posix time_t - do you know any :-) ?  
> 
> Just one. :-)  See:
> 
> https://lists.gnu.org/archive/html/bug-gnulib/2011-05/msg00470.html
> 
> Bruno's most recent BeOS-related submission to Gnulib was in October
> 2017:
> 
> https://lists.gnu.org/r/bug-gnulib/2017-10/msg00098.html


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Paul Eggert March 22, 2019, 9:49 p.m. UTC | #4
On 3/20/19 12:03 AM, Lukasz Majewski wrote:
> Do you plan to prepare (and send to mailing list) the next version of
> this patch (including the above fix)?

Sure, attached.

> In the time/mktime.c there is:
> weak_alias (mktime, timelocal), which makes the timelocal calls
> aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> __REDIRECT will be added).  
> Sorry, I'm a bit lost here. How will that __REDIRECT work, exactly?
> Should it be part of this patch, or part of a later patch?
> The __REDIRECT would be a part of the latter patch - the one which adds
> Y2038 support for 32 bit SoCs.
>
> It would simply redirect calls to mktime/timegm to internal
> __mktime64()/__timegm64().
>
OK, in that case, it can redirect timelocal calls to __mktime64, and
there is no need for a __timelocal64.
From 583877361f22d760fe15f7b6555cf9463c2f2d4a 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): 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.
---
 ChangeLog              | 44 +++++++++++++++++++++++
 include/time.h         | 39 ++++++++++----------
 posix/bits/types.h     |  6 ----
 time/mktime-internal.h | 80 +++++++++++++++++++++++++++++++++++++++++-
 time/mktime.c          | 71 +++++++++++++++++++++++--------------
 time/timegm.c          | 32 ++++++++++++++---
 6 files changed, 215 insertions(+), 57 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 45917ec56a..94a9a48ee9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,47 @@
+2019-03-22  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-03-22  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
 
 	* benchtests/Makefile (USE_CLOCK_GETTIME) Remove.
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..110081316b 100644
--- a/posix/bits/types.h
+++ b/posix/bits/types.h
@@ -213,12 +213,6 @@ __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
-# define __time64_t __time_t
-#else
-__STD_TYPE __TIME64_T_TYPE __time64_t;	/* Seconds since the Epoch.  */
-#endif
-
 #undef __STD_TYPE
 
 #endif /* bits/types.h */
diff --git a/time/mktime-internal.h b/time/mktime-internal.h
index 6111c22880..7cdc868f6d 100644
--- a/time/mktime-internal.h
+++ b/time/mktime-internal.h
@@ -1,2 +1,80 @@
-/* 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 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
+typedef __TIME64_T_TYPE __time64_t;
+#else
+# 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
Lukasz Majewski March 23, 2019, 11:59 a.m. UTC | #5
Hi Paul,

> Lukasz Majewski wrote:
> 
> > Shouldn't we have: return s == t; ?  
> 
> Yes, absolutely. Thanks for catching that. I tested only the Gnulib
> version, and Gnulib doesn't use that code.
> 
> Do you have glibc tests to catch bugs like this? If no, please add
> writing some tests to your lists of things to do.

Please find some patches to be applied on top of your patch to make the
Y2038 system working:

https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes

In short:

- The __time64_t needs to be exported to user space as it is a building
  block for Y2038 safe 32 bit systems' structures (like struct
  __timeval64). In the above use case the "normal" timeval is
  implicitly replaced with __timeval64.

- The correct condition for "in_time_t_range()"

- Some fixes necessary to make glibc building


Please consider merging those changes to your patch.

> 
> > In the time/mktime.c there is:
> > weak_alias (mktime, timelocal), which makes the timelocal calls
> > aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> > __REDIRECT will be added).  
> 
> Sorry, I'm a bit lost here. How will that __REDIRECT work, exactly?
> Should it be part of this patch, or part of a later patch?
> 
> >> Come to think of it, user code shouldn't see __time64_t
> >> either....  
> > 
> > Is that the reason for removing __time64_t definition from
> > posix/bits/types.h ?  
> 
> Yes.
> > In the time/mktime-internal.h you added a comment regarding BeOS
> > users and posix time_t - do you know any :-) ?  
> 
> Just one. :-)  See:
> 
> https://lists.gnu.org/archive/html/bug-gnulib/2011-05/msg00470.html
> 
> Bruno's most recent BeOS-related submission to Gnulib was in October
> 2017:
> 
> https://lists.gnu.org/r/bug-gnulib/2017-10/msg00098.html


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Lukasz Majewski March 23, 2019, 9:34 p.m. UTC | #6
Hi Paul,

> On 3/20/19 12:03 AM, Lukasz Majewski wrote:
> > Do you plan to prepare (and send to mailing list) the next version
> > of this patch (including the above fix)?
> 
> Sure, attached.
> 
> > In the time/mktime.c there is:
> > weak_alias (mktime, timelocal), which makes the timelocal calls
> > aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> > __REDIRECT will be added).  
> > Sorry, I'm a bit lost here. How will that __REDIRECT work, exactly?
> > Should it be part of this patch, or part of a later patch?
> > The __REDIRECT would be a part of the latter patch - the one which
> > adds Y2038 support for 32 bit SoCs.
> >
> > It would simply redirect calls to mktime/timegm to internal
> > __mktime64()/__timegm64().
> >
> OK, in that case, it can redirect timelocal calls to __mktime64, and
> there is no need for a __timelocal64.
> 

Thanks for the updated patch. I don't know why I've just received your
e-mail (but it is from Friday).

Today (Sat, around 12:00 CET), I've spent some time on testing your
previous work and write some comments/fixes in the other mail.

The code can be found in:
https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes

The biggest problem from Y2038 support point of view is with removing
__time64_t from posix/bits/types.h
(more explanation in the other mail).


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Lukasz Majewski March 24, 2019, 10:17 p.m. UTC | #7
Hi Paul,

> Hi Paul,
> 
> > On 3/20/19 12:03 AM, Lukasz Majewski wrote:  
> > > Do you plan to prepare (and send to mailing list) the next version
> > > of this patch (including the above fix)?  
> > 
> > Sure, attached.
> >   
> > > In the time/mktime.c there is:
> > > weak_alias (mktime, timelocal), which makes the timelocal calls
> > > aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> > > __REDIRECT will be added).  
> > > Sorry, I'm a bit lost here. How will that __REDIRECT work,
> > > exactly? Should it be part of this patch, or part of a later
> > > patch? The __REDIRECT would be a part of the latter patch - the
> > > one which adds Y2038 support for 32 bit SoCs.
> > >
> > > It would simply redirect calls to mktime/timegm to internal
> > > __mktime64()/__timegm64().
> > >  
> > OK, in that case, it can redirect timelocal calls to __mktime64, and
> > there is no need for a __timelocal64.
> >   
> 
> Thanks for the updated patch. I don't know why I've just received your
> e-mail (but it is from Friday).
> 
> Today (Sat, around 12:00 CET), I've spent some time on testing your
> previous work and write some comments/fixes in the other mail.
> 
> The code can be found in:
> https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes

Please find the notification about this branch being updated (in the
case you have already fetched it).

> 
> The biggest problem from Y2038 support point of view is with removing
> __time64_t from posix/bits/types.h
> (more explanation in the other mail).
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma@denx.de




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Paul Eggert March 27, 2019, 8:06 p.m. UTC | #8
On 3/23/19 4:59 AM, Lukasz Majewski wrote:
> https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes
>
> In short:
>
> - The __time64_t needs to be exported to user space as it is a building
>   block for Y2038 safe 32 bit systems' structures (like struct
>   __timeval64). In the above use case the "normal" timeval is
>   implicitly replaced with __timeval64.

I looked into that URL, and I don't see any explanation of why
__time64_t needs to be visible to user code.

Surely struct timeval ought to be consistent with time_t. That is, it
ought to continue to be defined like this:

struct timeval
{
  __time_t tv_sec;        /* Seconds.  */
  __suseconds_t tv_usec;    /* Microseconds.  */
};

where __time_t is merely an alias for time_t and so is 64 bits if
_TIME_BITS=64 and is the current size (32 or 64) otherwise.

The only reason I can see why __time64_t needs to be visible to user
code, would be if a single user source file accesses both time_t and
__time64_t (or needs to access both struct timeval and struct timeval64,
or similarly for struct timespec etc.). However, we should not support a
complicated API like that, as it's typically not useful in practice and
its mere availability causes more confusion than it's worth - as I've
discovered with _FILE_OFFSET_BITS and __off64_t. Instead, glibc should
have a simple user API where _TIME_BITS=64 merely says "I want 64-bit
time_t everywhere" and a module either uses this option or it doesn't.

I did see a fix in that URL, to use "#elif defined TIME_T_IS_SIGNED"
instead of "#elif TIME_T_IS_SIGNED" for the benefit of picky glibc
compiles, and that fix is incorporated in the revised patch attached.
From 8d6ea5e533fb3c071ae182dbf16a32b959f473b0 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): 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.
---
 ChangeLog              | 44 +++++++++++++++++++++++
 include/time.h         | 39 ++++++++++----------
 posix/bits/types.h     |  6 ----
 time/mktime-internal.h | 80 +++++++++++++++++++++++++++++++++++++++++-
 time/mktime.c          | 71 +++++++++++++++++++++++--------------
 time/timegm.c          | 32 ++++++++++++++---
 6 files changed, 215 insertions(+), 57 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index bd76c1e28d..4de7b142a5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,47 @@
+2019-03-27  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-02-26  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
 
 	* math/math.h (fpclassify, isfinite, isnormal, isnan): Use builtin for
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..110081316b 100644
--- a/posix/bits/types.h
+++ b/posix/bits/types.h
@@ -213,12 +213,6 @@ __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
-# define __time64_t __time_t
-#else
-__STD_TYPE __TIME64_T_TYPE __time64_t;	/* Seconds since the Epoch.  */
-#endif
-
 #undef __STD_TYPE
 
 #endif /* bits/types.h */
diff --git a/time/mktime-internal.h b/time/mktime-internal.h
index 6111c22880..15dec0317c 100644
--- a/time/mktime-internal.h
+++ b/time/mktime-internal.h
@@ -1,2 +1,80 @@
-/* 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
+typedef __TIME64_T_TYPE __time64_t;
+#else
+# 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
Lukasz Majewski March 28, 2019, 8:59 a.m. UTC | #9
Hi Paul,

> On 3/23/19 4:59 AM, Lukasz Majewski wrote:
> > https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes
> >
> > In short:
> >
> > - The __time64_t needs to be exported to user space as it is a
> > building block for Y2038 safe 32 bit systems' structures (like
> > struct __timeval64). In the above use case the "normal" timeval is
> >   implicitly replaced with __timeval64.  
> 
> I looked into that URL, and I don't see any explanation of why
> __time64_t needs to be visible to user code.

On top of this patchset there is an ongoing work for Y2038 support:
https://github.com/lmajewski/y2038_glibc/commits/Y2038-2.29-glibc-__clock_settime64_v1-27-03-2019

The idea till now was to introduce new "installed" glibc type:

struct __timeval64
{
  __time64_t tv_sec;		/* Seconds */
  __int64_t tv_usec;		/* Microseconds */
};

Which uses explicit __time64_t to store tv_sec.

The above structure on 32bit Y2038 safe devices would replace in user
code struct timeval.


> 
> Surely struct timeval ought to be consistent with time_t. That is, it
> ought to continue to be defined like this:
> 
> struct timeval
> {
>   __time_t tv_sec;        /* Seconds.  */
>   __suseconds_t tv_usec;    /* Microseconds.  */
> };
> 
> where __time_t is merely an alias for time_t and so is 64 bits if
> _TIME_BITS=64 and is the current size (32 or 64) otherwise.

In other words - I shall not introduce new "installed" type for struct
timeval and just in posix/bits/types.h define:

#ifndef __USE_TIME_BITS64
  __STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */
#else
  __STD_TYPE·__TIME64_T_TYPE __time_t;
#endif

In that way all structures which use __time_t are Y2038 safe.

ONE NOTE:
I also guess that the above change keeps those structs posix compliant
for 32 bit machines ?

> 
> The only reason I can see why __time64_t needs to be visible to user
> code, would be if a single user source file accesses both time_t and
> __time64_t (or needs to access both struct timeval and struct
> timeval64, or similarly for struct timespec etc.).

The only supported use case would be that user defines -D_TIME_BITS=64
during compilation of the SW (in OE/yocto) and use Y2038 safe types.

> However, we should
> not support a complicated API like that, as it's typically not useful
> in practice and its mere availability causes more confusion than it's
> worth - as I've discovered with _FILE_OFFSET_BITS and __off64_t.


If I may ask - what were the biggest problems?


I would appreciate if we could make the decision/agreement on this -

if the 64bit time support shall be done by above redefinition of
__time_t and not "exporting" __time64_t (and struct __timeval64,
__timespec64, etc).

> Instead, glibc should have a simple user API where _TIME_BITS=64
> merely says "I want 64-bit time_t everywhere" and a module either
> uses this option or it doesn't.
> 

So according to above I shall only introduce glibc _internal_ struct
__timespec64/__timeval64 in include/bits/types/ :

#if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
# define __timespec64 timespec;
#else
struct __timespec64 {
	__time64_t tv_sec;
	__int64_t tv_nsec;
}
#endif

and rewrite the relevant functions/syscalls (like clock_settime() in
this particular case) to use it in glibc ?

PROBLEM(S) with internal struct __timespec64:

- Would be misleading for 32 bit architectures (minor issue)

- Needs to met specific Linux kernel's ABI as it is passed as an
  argument to Linux syscalls (like clock_settime64).


> I did see a fix in that URL, to use "#elif defined TIME_T_IS_SIGNED"
> instead of "#elif TIME_T_IS_SIGNED" for the benefit of picky glibc
> compiles, and that fix is incorporated in the revised patch attached.
> 

Thanks for the revised patch.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Paul Eggert March 28, 2019, 4:09 p.m. UTC | #10
On 3/28/19 1:59 AM, Lukasz Majewski wrote:
> In other words - I shall not introduce new "installed" type for struct
> timeval and just in posix/bits/types.h define:
>
> #ifndef __USE_TIME_BITS64
>   __STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */
> #else
>   __STD_TYPE·__TIME64_T_TYPE __time_t;
> #endif
>
> In that way all structures which use __time_t are Y2038 safe.
>
> ONE NOTE:
> I also guess that the above change keeps those structs posix compliant
> for 32 bit machines ?

Yes, that's the idea.


>
>> However, we should
>> not support a complicated API like that, as it's typically not useful
>> in practice and its mere availability causes more confusion than it's
>> worth - as I've discovered with _FILE_OFFSET_BITS and __off64_t.
>
> If I may ask - what were the biggest problems?

The biggest problem is confusion and complexity. For example, see
/usr/include/zlib.h (assuming you have zlib installed). zlib is one of
the few libraries that tries to support both 32- and 64-bit file offsets
in user code. Almost nobody understands how it is supposed to work, and
I'm not sure that it even does work. And even in code that doesn't
attempt to support this sort of thing, there are still problems. See,
for example, the comedy of errors in
<https://stackoverflow.com/questions/22663897/unknown-type-name-off64-t>
where people suggest monstrosities like -Doff64_t=_off64_t to work
around compilation issues with the Apache Portable Runtime.

Rather than go down those rabbit holes, it's better to say that the
entire program must be compiled consistently, i.e., one must compile all
libraries with -D_TIME_BITS=64 if one plans to use -D_TIME_BITS=64 in
user code that passes time_t to these libraries. This includes OpenSSL,
GnuTLS, Kerberos, Glib/Gtk, libpng, ImageMagick, etc., etc., as all
these libraries expose time_t in their APIs. And it's not just
libraries: for example, one should compile Python with -D_TIME_BITS=64
and all Python modules requiring native code and using time_t in their
API should also be built with -D_TIME_BITS=64.

Although it'll be a real hassle to insist on -D_TIME_BITS=64 everywhere,
it's the only approach that will really work in practice. That's life.
Frankly if it were up to me, I'd make 64-bit time_t the default and ask
people to compile with -D_TIME_BITS=32 everywhere if they have
backward-compability concerns, as this will be much better in the long
run, and would help us avoid many of the issues we ran into during the
off_t debacle.


>> Instead, glibc should have a simple user API where _TIME_BITS=64
>> merely says "I want 64-bit time_t everywhere" and a module either
>> uses this option or it doesn't.
>>
> So according to above I shall only introduce glibc _internal_ struct
> __timespec64/__timeval64 in include/bits/types/ :
>
> #if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
> # define __timespec64 timespec;
> #else
> struct __timespec64 {
> 	__time64_t tv_sec;
> 	__int64_t tv_nsec;
> }
> #endif

No, these internal interfaces should not be in any file installed under
/usr/include where users can find them and get confused. They should be
only in .h files that are private to glibc and are not installed. A
logical place for them would be include/time.h.

>
> and rewrite the relevant functions/syscalls (like clock_settime() in
> this particular case) to use it in glibc ?

Yes, implementations of syscalls can use these internal interfaces.


>
> PROBLEM(S) with internal struct __timespec64:
>
> - Would be misleading for 32 bit architectures (minor issue)

I'm not sure I understand this point. How would we be misleading users
by keeping the __time64_t interfaces private?


>
> - Needs to met specific Linux kernel's ABI as it is passed as an
>   argument to Linux syscalls (like clock_settime64).

Yes, internally glibc will need to know what the kernel ABI is, and do
the right thing. This should be reasonably easy to do if the internal
glibc interfaces are 64-bit, which they should be.
Joseph Myers March 28, 2019, 4:34 p.m. UTC | #11
On Thu, 28 Mar 2019, Lukasz Majewski wrote:

> > where __time_t is merely an alias for time_t and so is 64 bits if
> > _TIME_BITS=64 and is the current size (32 or 64) otherwise.
> 
> In other words - I shall not introduce new "installed" type for struct
> timeval and just in posix/bits/types.h define:
> 
> #ifndef __USE_TIME_BITS64
>   __STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */
> #else
>   __STD_TYPE·__TIME64_T_TYPE __time_t;
> #endif

Note that would conflict with the practice for all the other types such as 
__off_t.

> In that way all structures which use __time_t are Y2038 safe.

I don't think you can avoid explicit conditionals in struct timespec, 
because of the issue of endian-dependent padding around 32-bit 
nanoseconds.
Lukasz Majewski March 29, 2019, 2:24 p.m. UTC | #12
Hi Paul,

> On 3/28/19 1:59 AM, Lukasz Majewski wrote:
> > In other words - I shall not introduce new "installed" type for
> > struct timeval and just in posix/bits/types.h define:
> >
> > #ifndef __USE_TIME_BITS64
> >   __STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */
> > #else
> >   __STD_TYPE·__TIME64_T_TYPE __time_t;
> > #endif
> >
> > In that way all structures which use __time_t are Y2038 safe.
> >
> > ONE NOTE:
> > I also guess that the above change keeps those structs posix
> > compliant for 32 bit machines ?  
> 
> Yes, that's the idea.
> 
> 
> >  
> >> However, we should
> >> not support a complicated API like that, as it's typically not
> >> useful in practice and its mere availability causes more confusion
> >> than it's worth - as I've discovered with _FILE_OFFSET_BITS and
> >> __off64_t.  
> >
> > If I may ask - what were the biggest problems?  
> 
> The biggest problem is confusion and complexity. For example, see
> /usr/include/zlib.h (assuming you have zlib installed). zlib is one of
> the few libraries that tries to support both 32- and 64-bit file
> offsets in user code. Almost nobody understands how it is supposed to
> work, and I'm not sure that it even does work. And even in code that
> doesn't attempt to support this sort of thing, there are still
> problems. See, for example, the comedy of errors in
> <https://stackoverflow.com/questions/22663897/unknown-type-name-off64-t>
> where people suggest monstrosities like -Doff64_t=_off64_t to work
> around compilation issues with the Apache Portable Runtime.
> 
> Rather than go down those rabbit holes, it's better to say that the
> entire program must be compiled consistently, i.e., one must compile
> all libraries with -D_TIME_BITS=64 if one plans to use
> -D_TIME_BITS=64 in user code that passes time_t to these libraries.
> This includes OpenSSL, GnuTLS, Kerberos, Glib/Gtk, libpng,
> ImageMagick, etc., etc., as all these libraries expose time_t in
> their APIs. And it's not just libraries: for example, one should
> compile Python with -D_TIME_BITS=64 and all Python modules requiring
> native code and using time_t in their API should also be built with
> -D_TIME_BITS=64.
> 
> Although it'll be a real hassle to insist on -D_TIME_BITS=64
> everywhere, it's the only approach that will really work in practice.

The above approach is doable if building the embedded system rootfs in
OE/Yocto.

I do have a setup where only for glibc compilation I do disable
-D_TIME_BITS64, but for _all_ other source code it is enabled.

> That's life. Frankly if it were up to me, I'd make 64-bit time_t the
> default and ask people to compile with -D_TIME_BITS=32 everywhere if
> they have backward-compability concerns, as this will be much better
> in the long run, and would help us avoid many of the issues we ran
> into during the off_t debacle.
> 

I see your point.

> 
> >> Instead, glibc should have a simple user API where _TIME_BITS=64
> >> merely says "I want 64-bit time_t everywhere" and a module either
> >> uses this option or it doesn't.
> >>  
> > So according to above I shall only introduce glibc _internal_ struct
> > __timespec64/__timeval64 in include/bits/types/ :
> >
> > #if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
> > # define __timespec64 timespec;
> > #else
> > struct __timespec64 {
> > 	__time64_t tv_sec;
> > 	__int64_t tv_nsec;
> > }
> > #endif  
> 
> No, these internal interfaces should not be in any file installed
> under /usr/include where users can find them and get confused. 

Yes, correct.

I've proposed to install struct __timespec64/__timeval64
in ./include/bits/types directory of glibc (and as fair as I understand
those would be private).

But also, those can be placed in ./include/time.h

> They
> should be only in .h files that are private to glibc and are not
> installed. A logical place for them would be include/time.h.
> 

Ok.

> >
> > and rewrite the relevant functions/syscalls (like clock_settime() in
> > this particular case) to use it in glibc ?  
> 
> Yes, implementations of syscalls can use these internal interfaces.
> 

Ok. Good.

> 
> >
> > PROBLEM(S) with internal struct __timespec64:
> >
> > - Would be misleading for 32 bit architectures (minor issue)  
> 
> I'm not sure I understand this point. How would we be misleading users
> by keeping the __time64_t interfaces private?

With __timespec64 exported (installed in usr/include/*) - there would
be an explicit type to show that we are Y2038 safe (when e.g.
debugging).

> 
> 
> >
> > - Needs to met specific Linux kernel's ABI as it is passed as an
> >   argument to Linux syscalls (like clock_settime64).  
> 
> Yes, internally glibc will need to know what the kernel ABI is, and do
> the right thing. This should be reasonably easy to do if the internal
> glibc interfaces are 64-bit, which they should be.

As presented above - kernel expects in e.g. timespec 64 bit values for
both tv_sec and tv_nsec. Also internal types doesn't need to comply with
POSIX (tv_nsec shall be single long as seen from user space).

> 
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Paul Eggert March 29, 2019, 9:10 p.m. UTC | #13
On 3/29/19 7:24 AM, Lukasz Majewski wrote:
> With __timespec64 exported (installed in usr/include/*) - there would
> be an explicit type to show that we are Y2038 safe (when e.g.
> debugging).

That's not much of an argument for exporting __timespec64 to user code.
When debugging a module, a developer can easily determine whether
whether time_t is 32- or 64-bit by printing sizeof (time_t) in GDB, so
the visibility of __timespec64 wouldn't give them any information that
they don't already have.

> I do have a setup where only for glibc compilation I do disable
> -D_TIME_BITS64, but for _all_ other source code it is enabled.

Sorry, I'm not following what you mean by "disable -DTIME_BITS64". Do
you mean that you compile applications with -D_TIME_BITS=64 and compile
glibc without it?

Does -D_TIME_BITS=64 affect building the GNU C library itself, and if
so, why? (Of course -D_TIME_BITS=64 will affect glibc's auxiliary
programs like zdump, but these are exceptional cases; I'm worried about
the library proper.) I thought _TIME_BITS was intended for use in user
programs, not for use in glibc itself.

The answers to the above questions should be documented in
manual/maint.texi; could you add that to your list of things to do?
Lukasz Majewski March 30, 2019, 2:39 p.m. UTC | #14
Hi Paul,

> On 3/29/19 7:24 AM, Lukasz Majewski wrote:
> > With __timespec64 exported (installed in usr/include/*) - there
> > would be an explicit type to show that we are Y2038 safe (when e.g.
> > debugging).  
> 
> That's not much of an argument for exporting __timespec64 to user
> code. When debugging a module, a developer can easily determine
> whether whether time_t is 32- or 64-bit by printing sizeof (time_t)
> in GDB, so the visibility of __timespec64 wouldn't give them any
> information that they don't already have.

Yes, indeed. From practical point of view it would be best to avoid
introducing any new types.

> 
> > I do have a setup where only for glibc compilation I do disable
> > -D_TIME_BITS64, but for _all_ other source code it is enabled.  
> 
> Sorry, I'm not following what you mean by "disable -DTIME_BITS64". Do
> you mean that you compile applications with -D_TIME_BITS=64 and
> compile glibc without it?

Yes, exactly.

That was the case when I last time was trying to use/compile the glibc
being Y2038 safe and as the only libc on the OE/Yocto created system.

However, it was some time ago (before I did some cleanup)
The meta-y2038 relevant branch (but it is outdated now a bit):
https://github.com/lmajewski/meta-y2038/commits/y2038-glibc2.29-deploy

> 
> Does -D_TIME_BITS=64 affect building the GNU C library itself, and if
> so, why? 

I did not checked it with the new code, but yes the glibc was not
buildable with -D_TIME_BITS=64

> (Of course -D_TIME_BITS=64 will affect glibc's auxiliary
> programs like zdump, but these are exceptional cases; I'm worried
> about the library proper.) I thought _TIME_BITS was intended for use
> in user programs, not for use in glibc itself.

Yes, exactly. The _TIME_BITS shall be only supported when building user
programs. Glibc shall not be build with it.

> 
> The answers to the above questions should be documented in
> manual/maint.texi; could you add that to your list of things to do?
> 

Yes, noted.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Joseph Myers April 1, 2019, 8:17 p.m. UTC | #15
On Fri, 29 Mar 2019, Lukasz Majewski wrote:

> I've proposed to install struct __timespec64/__timeval64
> in ./include/bits/types directory of glibc (and as fair as I understand
> those would be private).

Almost all include/bits headers are single-line wrappers round the 
installed header.  Given that <bits/*.h> is a namespace specifically for 
*installed* header fragments included from other installed headers, you 
need a very good reason to put actual type definitions in include/bits/.

> But also, those can be placed in ./include/time.h

Yes, that's a more appropriate place for internal type definitions.
Lukasz Majewski April 1, 2019, 8:51 p.m. UTC | #16
Hi Joseph,

> On Fri, 29 Mar 2019, Lukasz Majewski wrote:
> 
> > I've proposed to install struct __timespec64/__timeval64
> > in ./include/bits/types directory of glibc (and as fair as I
> > understand those would be private).  
> 
> Almost all include/bits headers are single-line wrappers round the 
> installed header.  Given that <bits/*.h> is a namespace specifically
> for *installed* header fragments included from other installed
> headers, you need a very good reason to put actual type definitions
> in include/bits/.
> 
> > But also, those can be placed in ./include/time.h  
> 
> Yes, that's a more appropriate place for internal type definitions.
> 

Ok, I see. Thanks for clarification.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Paul Eggert April 28, 2019, 10:45 p.m. UTC | #17
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.
Lukasz Majewski April 30, 2019, 7:59 a.m. UTC | #18
Hi Paul,

> 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.

Thank you very much for this patch - I've tested it in my setup and it
works.

Tested-by: Lukasz Majewski <lukma@denx.de>


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Paul Eggert April 30, 2019, 4:25 p.m. UTC | #19
On 4/30/19 12:59 AM, Lukasz Majewski wrote:
> Thank you very much for this patch - I've tested it in my setup and it
> works.

Thanks for checking. This seems ready, so I installed the patch into
glibc and propagated it into Gnulib.
Lukasz Majewski May 2, 2019, 7:19 a.m. UTC | #20
Hi Paul,

> On 4/30/19 12:59 AM, Lukasz Majewski wrote:
> > Thank you very much for this patch - I've tested it in my setup and
> > it works.  
> 
> Thanks for checking. This seems ready, so I installed the patch into
> glibc and propagated it into Gnulib.
> 

Thanks for the inclusion.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Patch
diff mbox series

From 509d363dc0f96ea194589e90b27c7f1c2de51371 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): 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.
---
 ChangeLog              | 44 +++++++++++++++++++++++
 include/time.h         | 39 ++++++++++----------
 posix/bits/types.h     |  6 ----
 time/mktime-internal.h | 80 +++++++++++++++++++++++++++++++++++++++++-
 time/mktime.c          | 71 +++++++++++++++++++++++--------------
 time/timegm.c          | 32 ++++++++++++++---
 6 files changed, 215 insertions(+), 57 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f7c9ee51ad..5d9f936c19 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,47 @@ 
+2019-03-18  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-03-16  Samuel Thibault  <samuel.thibault@ens-lyon.org>
 
 	* hurd/hurd/signal.h (_hurd_critical_section_lock): Document how EINTR
diff --git a/include/time.h b/include/time.h
index 61dd9e180b..3e55f7ba83 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..110081316b 100644
--- a/posix/bits/types.h
+++ b/posix/bits/types.h
@@ -213,12 +213,6 @@  __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
-# define __time64_t __time_t
-#else
-__STD_TYPE __TIME64_T_TYPE __time64_t;	/* Seconds since the Epoch.  */
-#endif
-
 #undef __STD_TYPE
 
 #endif /* bits/types.h */
diff --git a/time/mktime-internal.h b/time/mktime-internal.h
index 6111c22880..7cdc868f6d 100644
--- a/time/mktime-internal.h
+++ b/time/mktime-internal.h
@@ -1,2 +1,80 @@ 
-/* 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 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
+typedef __TIME64_T_TYPE __time64_t;
+#else
+# 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