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

Message ID e2295394-1af5-1e5a-d3da-9bcf28bc847c@cs.ucla.edu
State Superseded
Headers

Commit Message

Paul Eggert March 27, 2019, 8:06 p.m. UTC
  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.
  

Comments

Lukasz Majewski March 28, 2019, 8:59 a.m. UTC | #1
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 | #2
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 | #3
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 | #4
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 | #5
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 | #6
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 | #7
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 | #8
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
  

Patch

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