diff mbox series

[2/2] Y2038: make __tz_convert compatible with 64-bit-time

Message ID 20180613070019.4639-3-albert.aribaud@3adev.fr
State New
Headers show
Series Y2038 support batch 1 - __time64_t and __tz_convert | expand

Commit Message

Albert ARIBAUD (3ADEV) June 13, 2018, 7 a.m. UTC
This implies that its callers be 64-bit-time compatible too.
It is done by creating 64-bit-time versions of these and
turning their original 32-bit-time versions into wrappers
(at a slight execution time cost).

The callers affected are:

  * localtime
  * localtime_r
  * ctime
  * ctime_r
  * gmtime
  * gmtime_r

Note that in time/tzfile.c we do not need to check for time_t
overflows anymore as introduced by commit fc79706a323 since we
now use internal_time_t.
---
 include/time.h   | 20 +++++++++++++++-----
 time/Versions    |  5 +++++
 time/ctime.c     | 21 ++++++++++++++++++---
 time/ctime_r.c   | 21 ++++++++++++++++++---
 time/gmtime.c    | 38 +++++++++++++++++++++++++++++++++-----
 time/localtime.c | 36 ++++++++++++++++++++++++++++++++----
 time/offtime.c   | 12 ++++++------
 time/tzfile.c    | 14 ++++----------
 time/tzset.c     | 30 ++++++++++++------------------
 9 files changed, 143 insertions(+), 54 deletions(-)

Comments

Florian Weimer June 13, 2018, 9:10 a.m. UTC | #1
On 06/13/2018 09:00 AM, Albert ARIBAUD (3ADEV) wrote:
> +  GLIBC_2.28 {
> +    __ctime64; __ctime64_r;
> +    __gmtime64; __gmtime64_r;
> +    __localtime64; __localtime64_r;
> +  }

Functions in the private namespace should be exported as GLIBC_PRIVATE. 
Except __gmtime64_r, these functions have unwanted side effects and 
cannot really be called from other parts of glibc anyway.

Thanks,
Florian
Paul Eggert June 13, 2018, 9:11 a.m. UTC | #2
Albert ARIBAUD (3ADEV) wrote:

> -extern void __tz_compute (time_t timer, struct tm *tm, int use_localtime)
> +extern void __tz_compute (internal_time_t timer, struct tm *tm, int use_localtime)

Why have two types "internal_time_t" and "__time64_t" that are always the same? 
Wouldn't it be simpler to get rid of "internal_time_t" and use "__time64_t" 
uniformly? If we do need two types, the need should be documented clearly.

>   /* Return a string as returned by asctime which
>      is the representation of *T in that form.  */
>   char *
> +__ctime64 (const __time64_t *t)
> +{
> +  /* Apply the same rule as ctime:
> +     make ctime64 (t) is equivalent to asctime (localtime64 (t)).  */
> +  return asctime (__localtime64 (t));
> +}
> +
> +/* The 32-bit time wrapper.  */
> +char *
>   ctime (const time_t *t)
>   {
> -  /* The C Standard says ctime (t) is equivalent to asctime (localtime (t)).
> -     In particular, ctime and asctime must yield the same pointer.  */
> -  return asctime (localtime (t));
> +  __time64_t t64;
> +  if (t == NULL)
> +    {
> +      __set_errno (EINVAL);
> +      return NULL;
> +    }
> +  t64 = *t;
> +  return __ctime64 (&t64);
>   }

I don't see why 64-bit platforms need two entry points. If time_t is 64 bits, 
ctime and __ctime64 can be aliases, no? Also, it's a bit cleaner to declare and 
initialize t64 at the same time, i.e., '__time64_t t64 = *t;'; we can assume 
this C99ism in glibc nowadays.

Similarly for __ctime64_r, etc.

The comment inside __ctime64 is confusing, since it says "localtime64" but the 
code says "__localtime64". Also, I would not remove the comment that is inside 
ctime, as removing it omits important motivation.

>   /* Return a string as returned by asctime which is the representation
> -   of *T in that form.  Reentrant version.  */
> +   of *T in that form.  Reentrant Y2038-proof version.  */
>   char *
> -ctime_r (const time_t *t, char *buf)
> +__ctime64_r (const __time64_t *t, char *buf)

There is no need to add "Y2038-proof" to the comment.  It's obvious that the 
code is using 64-bit times here, and we shouldn't be putting "Y2038"s all over 
the place in the commentary.

> +/* Return the `struct tm' representation of 64-bit-time *T
> +   in UTC, using *TP to store the result.  */
> +struct tm *
> +__gmtime64_r (const __time64_t *t, struct tm *tp)
> +{
> +  return __tz_convert (*t, 0, tp);
> +}
> +
> +/* The 32-bit-time wrapper.  */
>   struct tm *
>   __gmtime_r (const time_t *t, struct tm *tp)
>   {
> -  return __tz_convert (t, 0, tp);
> +  __time64_t t64;
> +  if (t == NULL)
> +    {
> +      __set_errno (EINVAL);
> +      return NULL;
> +    }
> +  t64 = *t;
> +  return __gmtime64_r (&t64, tp);
>   }

__gmtime_r sets errno=EINVAL for a null first argument, whereas __gmtime64_r 
dumps core instead. Why is that? If it's intended, there should be a comment as 
to why the two functions differ in behavior here. Similarly for __localtime_r, 
gmtime, localtime, and ctime. However, won't this introduce compatibility 
issues? Wouldn't be better for the __time64_t versions to mimic the time_t behavior?

> -      tz_rules[0].change = tz_rules[1].change = (time_t) -1;
> +      tz_rules[0].change = tz_rules[1].change = (internal_time_t) -1;

The cast is just getting making maintenance harder, so I suggest changing this 
to use plain "= -1;" at the end.
Florian Weimer June 13, 2018, 9:14 a.m. UTC | #3
On 06/13/2018 11:11 AM, Paul Eggert wrote:
> Albert ARIBAUD (3ADEV) wrote:
> 
>> -extern void __tz_compute (time_t timer, struct tm *tm, int 
>> use_localtime)
>> +extern void __tz_compute (internal_time_t timer, struct tm *tm, int 
>> use_localtime)
> 
> Why have two types "internal_time_t" and "__time64_t" that are always 
> the same? Wouldn't it be simpler to get rid of "internal_time_t" and use 
> "__time64_t" uniformly? If we do need two types, the need should be 
> documented clearly.

Yes, we can get rid of internal_time_t if we have __time64_t.

Thanks,
Florian
Albert ARIBAUD (3ADEV) June 13, 2018, 9:29 a.m. UTC | #4
Hi Florian,

On Wed, 13 Jun 2018 11:14:22 +0200, Florian Weimer <fweimer@redhat.com>
wrote :

> On 06/13/2018 11:11 AM, Paul Eggert wrote:
> > Albert ARIBAUD (3ADEV) wrote:
> >   
> >> -extern void __tz_compute (time_t timer, struct tm *tm, int 
> >> use_localtime)
> >> +extern void __tz_compute (internal_time_t timer, struct tm *tm, int 
> >> use_localtime)  
> > 
> > Why have two types "internal_time_t" and "__time64_t" that are always 
> > the same? Wouldn't it be simpler to get rid of "internal_time_t" and use 
> > "__time64_t" uniformly? If we do need two types, the need should be 
> > documented clearly.  
> 
> Yes, we can get rid of internal_time_t if we have __time64_t.

Fine for me.

> Thanks,
> Florian

Cordialement,
Albert ARIBAUD
3ADEV
Albert ARIBAUD (3ADEV) June 13, 2018, 9:36 a.m. UTC | #5
Hi Florian,

On Wed, 13 Jun 2018 11:10:09 +0200, Florian Weimer <fweimer@redhat.com>
wrote :

> On 06/13/2018 09:00 AM, Albert ARIBAUD (3ADEV) wrote:
> > +  GLIBC_2.28 {
> > +    __ctime64; __ctime64_r;
> > +    __gmtime64; __gmtime64_r;
> > +    __localtime64; __localtime64_r;
> > +  }  
> 
> Functions in the private namespace should be exported as GLIBC_PRIVATE. 
> Except __gmtime64_r, these functions have unwanted side effects and 
> cannot really be called from other parts of glibc anyway.

They're going to be implementations of APIs called from user source code
if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in
the whole series), so I don't understand how they could be considered
GLIBC_PRIVATE.

As for the side effects, which ones are you thinking of? The ones I am
aware of are those already present in the 32-bit-time versions and are
"regrettable but established behavior".

> Thanks,
> Florian

Cordialement,
Albert ARIBAUD
3ADEV
Florian Weimer June 13, 2018, 9:40 a.m. UTC | #6
On 06/13/2018 11:36 AM, Albert ARIBAUD wrote:
> Hi Florian,
> 
> On Wed, 13 Jun 2018 11:10:09 +0200, Florian Weimer <fweimer@redhat.com>
> wrote :
> 
>> On 06/13/2018 09:00 AM, Albert ARIBAUD (3ADEV) wrote:
>>> +  GLIBC_2.28 {
>>> +    __ctime64; __ctime64_r;
>>> +    __gmtime64; __gmtime64_r;
>>> +    __localtime64; __localtime64_r;
>>> +  }
>>
>> Functions in the private namespace should be exported as GLIBC_PRIVATE.
>> Except __gmtime64_r, these functions have unwanted side effects and
>> cannot really be called from other parts of glibc anyway.
> 
> They're going to be implementations of APIs called from user source code
> if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in
> the whole series), so I don't understand how they could be considered
> GLIBC_PRIVATE.

Why do they use the __ prefix?  We generally do not do that.

> As for the side effects, which ones are you thinking of? The ones I am
> aware of are those already present in the 32-bit-time versions and are
> "regrettable but established behavior".

The side effects simply mean that we cannot call this functions as an 
internal implementation detail of another function, so there should be 
no reason for an export in the private namespace (with the __prefix and 
GLIBC_PRIVATE).

Thanks,
Florian
Albert ARIBAUD (3ADEV) June 13, 2018, 10:20 a.m. UTC | #7
Hi Florian,

On Wed, 13 Jun 2018 11:40:04 +0200, Florian Weimer <fweimer@redhat.com>
wrote :

> On 06/13/2018 11:36 AM, Albert ARIBAUD wrote:
> > Hi Florian,
> > 
> > On Wed, 13 Jun 2018 11:10:09 +0200, Florian Weimer <fweimer@redhat.com>
> > wrote :
> >   
> >> On 06/13/2018 09:00 AM, Albert ARIBAUD (3ADEV) wrote:  
>  [...]  
> >>
> >> Functions in the private namespace should be exported as GLIBC_PRIVATE.
> >> Except __gmtime64_r, these functions have unwanted side effects and
> >> cannot really be called from other parts of glibc anyway.  
> > 
> > They're going to be implementations of APIs called from user source code
> > if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in
> > the whole series), so I don't understand how they could be considered
> > GLIBC_PRIVATE.  
> 
> Why do they use the __ prefix?  We generally do not do that.

I believe it stemmed from the fact that source code should not spell
these functions by their explicit name -- that name is to be used by
glibc only. User source code should keep using the historical names
(here, "gmtime_r"); if it has defined _TIME_BITS equal to 64, then the
glibc public headers will alias (or barring that, #define) gmtime_r to
__gmtime64_r (the 64-bit-time implementation); otherwise, "gmtime_r"
will be used as-is (the 32-bit-time implementation).

So to make sure the symbols were considered to not be for (direct)
public use, they have to start with an underscore.

> > As for the side effects, which ones are you thinking of? The ones I am
> > aware of are those already present in the 32-bit-time versions and are
> > "regrettable but established behavior".  
> 
> The side effects simply mean that we cannot call this functions as an 
> internal implementation detail of another function, so there should be 
> no reason for an export in the private namespace (with the __prefix and 
> GLIBC_PRIVATE).

Actually another function can call these functions -- the 32-bit-time
wrappers do that exactly. I must be missing your point.

> Thanks,
> Florian

Cordialement,
Albert ARIBAUD
3ADEV
Albert ARIBAUD (3ADEV) June 13, 2018, 10:55 a.m. UTC | #8
On Wed, 13 Jun 2018 12:20:50 +0200, Albert ARIBAUD
<albert.aribaud@3adev.fr> wrote :

> I believe it stemmed from the fact that source code should not spell

... that *user* source code...

Cordialement,
Albert ARIBAUD
3ADEV
Florian Weimer June 13, 2018, 1:08 p.m. UTC | #9
On 06/13/2018 12:20 PM, Albert ARIBAUD wrote:
> Hi Florian,
> 
> On Wed, 13 Jun 2018 11:40:04 +0200, Florian Weimer <fweimer@redhat.com>
> wrote :
> 
>> On 06/13/2018 11:36 AM, Albert ARIBAUD wrote:
>>> Hi Florian,
>>>
>>> On Wed, 13 Jun 2018 11:10:09 +0200, Florian Weimer <fweimer@redhat.com>
>>> wrote :
>>>    
>>>> On 06/13/2018 09:00 AM, Albert ARIBAUD (3ADEV) wrote:
>>   [...]
>>>>
>>>> Functions in the private namespace should be exported as GLIBC_PRIVATE.
>>>> Except __gmtime64_r, these functions have unwanted side effects and
>>>> cannot really be called from other parts of glibc anyway.
>>>
>>> They're going to be implementations of APIs called from user source code
>>> if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in
>>> the whole series), so I don't understand how they could be considered
>>> GLIBC_PRIVATE.
>>
>> Why do they use the __ prefix?  We generally do not do that.
> 
> I believe it stemmed from the fact that source code should not spell
> these functions by their explicit name -- that name is to be used by
> glibc only. User source code should keep using the historical names
> (here, "gmtime_r"); if it has defined _TIME_BITS equal to 64, then the
> glibc public headers will alias (or barring that, #define) gmtime_r to
> __gmtime64_r (the 64-bit-time implementation); otherwise, "gmtime_r"
> will be used as-is (the 32-bit-time implementation).
> 
> So to make sure the symbols were considered to not be for (direct)
> public use, they have to start with an underscore.

We use non-__ names for the LFS redirects (slightly trimmed):

# ifdef __REDIRECT
extern FILE *__REDIRECT (fopen, (const char *__restrict,
                                  const char *__restrict), fopen64);
extern FILE *__REDIRECT (freopen, (const char *__restrict,
                                    const char *__restrict,
                                    FILE *__restrict), freopen64);
# else
#  define fopen fopen64
#  define freopen freopen64
# endif
#endif

I don't see a totally conforming way to implement this using redirects 
anyway, so whether __ is used or not is secondary because it doesn't 
address the main problem.

>>> As for the side effects, which ones are you thinking of? The ones I am
>>> aware of are those already present in the 32-bit-time versions and are
>>> "regrettable but established behavior".
>>
>> The side effects simply mean that we cannot call this functions as an
>> internal implementation detail of another function, so there should be
>> no reason for an export in the private namespace (with the __prefix and
>> GLIBC_PRIVATE).
> 
> Actually another function can call these functions -- the 32-bit-time
> wrappers do that exactly. I must be missing your point.

Yes, but that those are libc-only and they won't need the exports.  A 
hidden alias with a __ name would be sufficient under such circumstances.

Thanks,
Florian
Joseph Myers June 13, 2018, 2:17 p.m. UTC | #10
On Wed, 13 Jun 2018, Albert ARIBAUD (3ADEV) wrote:

> diff --git a/time/Versions b/time/Versions
> index fd838181e4..8b83f5b041 100644
> --- a/time/Versions
> +++ b/time/Versions
> @@ -65,4 +65,9 @@ libc {
>    GLIBC_2.16 {
>      timespec_get;
>    }
> +  GLIBC_2.28 {
> +    __ctime64; __ctime64_r;
> +    __gmtime64; __gmtime64_r;
> +    __localtime64; __localtime64_r;
> +  }

I suggest not adding any such public ABI exports until everything 
necessary to support _TIME_BITS=64 is in glibc.  (If a function is needed 
from another glibc shared library, a GLIBC_PRIVATE export may be added.)

> +  if (t == NULL)
> +    {
> +      __set_errno (EINVAL);
> +      return NULL;
> +    }

Such explicit NULL checks, for functions where NULL is not a valid input, 
are contrary to glibc standards; you should just dereference the input 
unconditionally.

https://sourceware.org/glibc/wiki/Style_and_Conventions#Error_Handling
Joseph Myers June 13, 2018, 2:22 p.m. UTC | #11
On Wed, 13 Jun 2018, Paul Eggert wrote:

> I don't see why 64-bit platforms need two entry points. If time_t is 64 bits,
> ctime and __ctime64 can be aliases, no? Also, it's a bit cleaner to declare
> and initialize t64 at the same time, i.e., '__time64_t t64 = *t;'; we can
> assume this C99ism in glibc nowadays.

My suggestion is that an internal header does

#if !__TIME32_SUPPORTED
# define __ctime64 ctime
#endif

and then the definition of the 32-bit ctime wrapper is conditional on "#if 
__TIME32_SUPPORTED", so that the main 64-bit version just gets compiled as 
ctime in the case where time_t has always only been 64-bit for this ABI.

That way, internal code can call the __*64 interfaces unconditionally and 
the calls will just end up calling the non-*64 interfaces when the *64 
interfaces don't need to exist as separate functions - and no exports of 
*64 in the public ABI are needed in that case either.
Joseph Myers June 13, 2018, 2:24 p.m. UTC | #12
On Wed, 13 Jun 2018, Florian Weimer wrote:

> > They're going to be implementations of APIs called from user source code
> > if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in
> > the whole series), so I don't understand how they could be considered
> > GLIBC_PRIVATE.
> 
> Why do they use the __ prefix?  We generally do not do that.

Because we want to fix bug 14106, and not replicate it for new interfaces 
(and don't intend to have explicit *64 APIs at all for 64-bit time_t, just 
new ABIs, on platforms where time_t is currently 32-bit, which can be 
selected using _TIME_BITS=64).
Joseph Myers June 13, 2018, 2:29 p.m. UTC | #13
On Wed, 13 Jun 2018, Florian Weimer wrote:

> I don't see a totally conforming way to implement this using redirects anyway,
> so whether __ is used or not is secondary because it doesn't address the main
> problem.

Strictly speaking there's the C and POSIX rule that you can declare and 
call a function yourself without including the relevant header, which 
doesn't work with redirects in headers.  But that only applies *if the 
function declaration doesn't need any type defined in a header* (so is 
only an issue for a few LFS functions, and not at all for the 64-bit time 
functions).  If desired that issue could be addressed by having a header 
using "#pragma redefine_extname" which you could include from the command 
line with -include.
Florian Weimer June 18, 2018, 1:34 p.m. UTC | #14
On 06/13/2018 04:24 PM, Joseph Myers wrote:
> On Wed, 13 Jun 2018, Florian Weimer wrote:
> 
>>> They're going to be implementations of APIs called from user source code
>>> if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in
>>> the whole series), so I don't understand how they could be considered
>>> GLIBC_PRIVATE.
>>
>> Why do they use the __ prefix?  We generally do not do that.
> 
> Because we want to fix bug 14106, and not replicate it for new interfaces
> (and don't intend to have explicit *64 APIs at all for 64-bit time_t, just
> new ABIs, on platforms where time_t is currently 32-bit, which can be
> selected using _TIME_BITS=64).

Ohh, right.

On the other hand, it is quite awkward why these obscure interfaces 
receive protection from accidental interposition, when others where we 
know that there is ongoing interposition (fadd, fdiv, canonicalize, 
explicit_bzero, getrandom, even getline) do not.

This relates to an older thread:

   <https://sourceware.org/ml/libc-alpha/2016-10/msg00294.html>

It also affects _GNU_SOURCE avoidance for libstdc++, depending on how we 
view this.  (Formal compliance vs avoiding collisions which occur in 
practice.)

Thanks,
Florian
diff mbox series

Patch

diff --git a/include/time.h b/include/time.h
index 93638aa215..c67e163eb8 100644
--- a/include/time.h
+++ b/include/time.h
@@ -9,6 +9,8 @@  extern __typeof (strftime_l) __strftime_l;
 libc_hidden_proto (__strftime_l)
 extern __typeof (strptime_l) __strptime_l;
 
+extern struct tm *__localtime64 (const __time64_t *__timer);
+
 libc_hidden_proto (time)
 libc_hidden_proto (asctime)
 libc_hidden_proto (mktime)
@@ -17,6 +19,8 @@  libc_hidden_proto (localtime)
 libc_hidden_proto (strftime)
 libc_hidden_proto (strptime)
 
+libc_hidden_proto (__localtime64)
+
 extern __typeof (clock_getres) __clock_getres;
 extern __typeof (clock_gettime) __clock_gettime;
 libc_hidden_proto (__clock_gettime)
@@ -51,7 +55,7 @@  extern void __tzfile_default (const char *std, const char *dst,
 			      long int stdoff, long int dstoff)
   attribute_hidden;
 extern void __tzset_parse_tz (const char *tz) attribute_hidden;
-extern void __tz_compute (time_t timer, struct tm *tm, int use_localtime)
+extern void __tz_compute (internal_time_t timer, struct tm *tm, int use_localtime)
   __THROW attribute_hidden;
 
 /* Subroutine of `mktime'.  Return the `time_t' representation of TP and
@@ -64,15 +68,21 @@  extern time_t __mktime_internal (struct tm *__tp,
 extern struct tm *__localtime_r (const time_t *__timer,
 				 struct tm *__tp) attribute_hidden;
 
+extern struct tm *__localtime64_r (const __time64_t *__timer,
+				   struct tm *__tp) attribute_hidden;
+
 extern struct tm *__gmtime_r (const time_t *__restrict __timer,
 			      struct tm *__restrict __tp);
 libc_hidden_proto (__gmtime_r)
 
-/* Compute the `struct tm' representation of *T,
+extern struct tm *__gmtime64_r (const __time64_t *__restrict __timer,
+			        struct tm *__restrict __tp);
+
+/* Compute the `struct tm' representation of T,
    offset OFFSET seconds east of UTC,
    and store year, yday, mon, mday, wday, hour, min, sec into *TP.
    Return nonzero if successful.  */
-extern int __offtime (const time_t *__timer,
+extern int __offtime (const internal_time_t __timer,
 		      long int __offset,
 		      struct tm *__tp) attribute_hidden;
 
@@ -81,8 +91,8 @@  extern char *__asctime_r (const struct tm *__tp, char *__buf)
 extern void __tzset (void) attribute_hidden;
 
 /* Prototype for the internal function to get information based on TZ.  */
-extern struct tm *__tz_convert (const time_t *timer, int use_localtime,
-				struct tm *tp) attribute_hidden;
+extern struct tm *__tz_convert (const internal_time_t timer, int use_localtime,
+			        struct tm *tp) attribute_hidden;
 
 extern int __nanosleep (const struct timespec *__requested_time,
 			struct timespec *__remaining);
diff --git a/time/Versions b/time/Versions
index fd838181e4..8b83f5b041 100644
--- a/time/Versions
+++ b/time/Versions
@@ -65,4 +65,9 @@  libc {
   GLIBC_2.16 {
     timespec_get;
   }
+  GLIBC_2.28 {
+    __ctime64; __ctime64_r;
+    __gmtime64; __gmtime64_r;
+    __localtime64; __localtime64_r;
+  }
 }
diff --git a/time/ctime.c b/time/ctime.c
index 1222614f29..de7f3c5bd3 100644
--- a/time/ctime.c
+++ b/time/ctime.c
@@ -16,13 +16,28 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <errno.h>
 
 /* Return a string as returned by asctime which
    is the representation of *T in that form.  */
 char *
+__ctime64 (const __time64_t *t)
+{
+  /* Apply the same rule as ctime:
+     make ctime64 (t) is equivalent to asctime (localtime64 (t)).  */
+  return asctime (__localtime64 (t));
+}
+
+/* The 32-bit time wrapper.  */
+char *
 ctime (const time_t *t)
 {
-  /* The C Standard says ctime (t) is equivalent to asctime (localtime (t)).
-     In particular, ctime and asctime must yield the same pointer.  */
-  return asctime (localtime (t));
+  __time64_t t64;
+  if (t == NULL)
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
+  t64 = *t;
+  return __ctime64 (&t64);
 }
diff --git a/time/ctime_r.c b/time/ctime_r.c
index c111146d76..eb6e2f3ed6 100644
--- a/time/ctime_r.c
+++ b/time/ctime_r.c
@@ -18,12 +18,27 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <errno.h>
 
 /* Return a string as returned by asctime which is the representation
-   of *T in that form.  Reentrant version.  */
+   of *T in that form.  Reentrant Y2038-proof version.  */
 char *
-ctime_r (const time_t *t, char *buf)
+__ctime64_r (const __time64_t *t, char *buf)
 {
   struct tm tm;
-  return __asctime_r (__localtime_r (t, &tm), buf);
+  return __asctime_r (__localtime64_r (t, &tm), buf);
+}
+
+/* The 32-bit-time wrapper.  */
+char *
+ctime_r (const time_t *t, char *buf)
+{
+  __time64_t t64;
+  if (t == NULL)
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
+  t64 = *t;
+  return __ctime64_r (&t64, buf);
 }
diff --git a/time/gmtime.c b/time/gmtime.c
index dc33b3e68a..b5ec84f36a 100644
--- a/time/gmtime.c
+++ b/time/gmtime.c
@@ -17,21 +17,49 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <errno.h>
 
-/* Return the `struct tm' representation of *T in UTC,
-   using *TP to store the result.  */
+/* Return the `struct tm' representation of 64-bit-time *T
+   in UTC, using *TP to store the result.  */
+struct tm *
+__gmtime64_r (const __time64_t *t, struct tm *tp)
+{
+  return __tz_convert (*t, 0, tp);
+}
+
+/* The 32-bit-time wrapper.  */
 struct tm *
 __gmtime_r (const time_t *t, struct tm *tp)
 {
-  return __tz_convert (t, 0, tp);
+  __time64_t t64;
+  if (t == NULL)
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
+  t64 = *t;
+  return __gmtime64_r (&t64, tp);
 }
 libc_hidden_def (__gmtime_r)
 weak_alias (__gmtime_r, gmtime_r)
 
+/* Return the `struct tm' representation of 64-bit-time *T in UTC.	*/
+struct tm *
+__gmtime64 (const __time64_t *t)
+{
+  return __tz_convert (*t, 0, &_tmbuf);
+}
 
-/* Return the `struct tm' representation of *T in UTC.	*/
+/* The 32-bit-time wrapper. */
 struct tm *
 gmtime (const time_t *t)
 {
-  return __tz_convert (t, 0, &_tmbuf);
+  __time64_t t64;
+  if (t == NULL)
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
+  t64 = *t;
+  return __gmtime64 (&t64);
 }
diff --git a/time/localtime.c b/time/localtime.c
index 8684a8a971..68f5322b21 100644
--- a/time/localtime.c
+++ b/time/localtime.c
@@ -17,25 +17,53 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <errno.h>
 
 /* The C Standard says that localtime and gmtime return the same pointer.  */
 struct tm _tmbuf;
 
-
 /* Return the `struct tm' representation of *T in local time,
    using *TP to store the result.  */
 struct tm *
+__localtime64_r (const __time64_t *t, struct tm *tp)
+{
+  return __tz_convert (*t, 1, tp);
+}
+
+/* The 32-bit-time wrapper.  */
+struct tm *
 __localtime_r (const time_t *t, struct tm *tp)
 {
-  return __tz_convert (t, 1, tp);
+  __time64_t t64;
+  if (t == NULL)
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
+  t64 = *t;
+  return __localtime64_r (&t64, tp);
 }
 weak_alias (__localtime_r, localtime_r)
 
-
 /* Return the `struct tm' representation of *T in local time.  */
 struct tm *
+__localtime64 (const __time64_t *t)
+{
+  return __tz_convert (*t, 1, &_tmbuf);
+}
+
+/* The 32-bit-time wrapper.  */
+struct tm *
 localtime (const time_t *t)
 {
-  return __tz_convert (t, 1, &_tmbuf);
+  __time64_t t64;
+  if (t == NULL)
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
+  t64 = *t;
+  return __localtime64 (&t64);
 }
 libc_hidden_def (localtime)
+
diff --git a/time/offtime.c b/time/offtime.c
index 04c48389fc..1ff71e3c7e 100644
--- a/time/offtime.c
+++ b/time/offtime.c
@@ -21,18 +21,18 @@ 
 #define	SECS_PER_HOUR	(60 * 60)
 #define	SECS_PER_DAY	(SECS_PER_HOUR * 24)
 
-/* Compute the `struct tm' representation of *T,
+/* Compute the `struct tm' representation of T,
    offset OFFSET seconds east of UTC,
    and store year, yday, mon, mday, wday, hour, min, sec into *TP.
    Return nonzero if successful.  */
 int
-__offtime (const time_t *t, long int offset, struct tm *tp)
+__offtime (const internal_time_t t, long int offset, struct tm *tp)
 {
-  time_t days, rem, y;
+  internal_time_t days, rem, y;
   const unsigned short int *ip;
 
-  days = *t / SECS_PER_DAY;
-  rem = *t % SECS_PER_DAY;
+  days = t / SECS_PER_DAY;
+  rem = t % SECS_PER_DAY;
   rem += offset;
   while (rem < 0)
     {
@@ -60,7 +60,7 @@  __offtime (const time_t *t, long int offset, struct tm *tp)
   while (days < 0 || days >= (__isleap (y) ? 366 : 365))
     {
       /* Guess a corrected year, assuming 365 days per year.  */
-      time_t yg = y + days / 365 - (days % 365 < 0);
+      internal_time_t yg = y + days / 365 - (days % 365 < 0);
 
       /* Adjust DAYS and Y to match the guessed year.  */
       days -= ((yg - y) * 365
diff --git a/time/tzfile.c b/time/tzfile.c
index 2a385b92bc..94ca3323d5 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -635,16 +635,10 @@  __tzfile_compute (internal_time_t timer, int use_localtime,
 
 	  /* Convert to broken down structure.  If this fails do not
 	     use the string.  */
-	  {
-	    time_t truncated = timer;
-	    if (__glibc_unlikely (truncated != timer
-				  || ! __offtime (&truncated, 0, tp)))
-	      goto use_last;
-	  }
-
-	  /* Use the rules from the TZ string to compute the change.
-	     timer fits into time_t due to the truncation check
-	     above.  */
+	  if (__glibc_unlikely (! __offtime (timer, 0, tp)))
+	    goto use_last;
+
+	  /* Use the rules from the TZ string to compute the change.  */
 	  __tz_compute (timer, tp, 1);
 
 	  /* If tzspec comes from posixrules loaded by __tzfile_default,
diff --git a/time/tzset.c b/time/tzset.c
index a828b9fb75..c2bb6cdffa 100644
--- a/time/tzset.c
+++ b/time/tzset.c
@@ -16,7 +16,6 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #include <ctype.h>
-#include <errno.h>
 #include <libc-lock.h>
 #include <stdbool.h>
 #include <stddef.h>
@@ -27,7 +26,7 @@ 
 
 #include <timezone/tzfile.h>
 
-#define SECSPERDAY ((time_t) 86400)
+#define SECSPERDAY ((internal_time_t) 86400)
 
 char *__tzname[2] = { (char *) "GMT", (char *) "GMT" };
 int __daylight = 0;
@@ -55,7 +54,7 @@  typedef struct
 
     /* We cache the computed time of change for a
        given year so we don't have to recompute it.  */
-    time_t change;	/* When to change to this zone.  */
+    internal_time_t change;	/* When to change to this zone.  */
     int computed_for;	/* Year above is computed for.  */
   } tz_rule;
 
@@ -416,7 +415,7 @@  tzset_internal (int always)
       tz_rules[0].name = tz_rules[1].name = "UTC";
       if (J0 != 0)
 	tz_rules[0].type = tz_rules[1].type = J0;
-      tz_rules[0].change = tz_rules[1].change = (time_t) -1;
+      tz_rules[0].change = tz_rules[1].change = (internal_time_t) -1;
       update_vars ();
       return;
     }
@@ -424,13 +423,13 @@  tzset_internal (int always)
   __tzset_parse_tz (tz);
 }
 
-/* Figure out the exact time (as a time_t) in YEAR
+/* Figure out the exact time (as an internal_time_t) in YEAR
    when the change described by RULE will occur and
    put it in RULE->change, saving YEAR in RULE->computed_for.  */
 static void
 compute_change (tz_rule *rule, int year)
 {
-  time_t t;
+  internal_time_t t;
 
   if (year != -1 && rule->computed_for == year)
     /* Operations on times in 2 BC will be slower.  Oh well.  */
@@ -514,9 +513,10 @@  compute_change (tz_rule *rule, int year)
 
 
 /* Figure out the correct timezone for TM and set `__tzname',
-   `__timezone', and `__daylight' accordingly.  */
+   `__timezone', and `__daylight' accordingly.
+   NOTE: this takes an internal_time_t value, so passing a __time_t value is OK. */
 void
-__tz_compute (time_t timer, struct tm *tm, int use_localtime)
+__tz_compute (internal_time_t timer, struct tm *tm, int use_localtime)
 {
   compute_change (&tz_rules[0], 1900 + tm->tm_year);
   compute_change (&tz_rules[1], 1900 + tm->tm_year);
@@ -562,20 +562,14 @@  __tzset (void)
 }
 weak_alias (__tzset, tzset)
 
-/* Return the `struct tm' representation of *TIMER in the local timezone.
+/* Return the `struct tm' representation of TIMER in the local timezone.
    Use local time if USE_LOCALTIME is nonzero, UTC otherwise.  */
 struct tm *
-__tz_convert (const time_t *timer, int use_localtime, struct tm *tp)
+__tz_convert (const internal_time_t timer, int use_localtime, struct tm *tp)
 {
   long int leap_correction;
   int leap_extra_secs;
 
-  if (timer == NULL)
-    {
-      __set_errno (EINVAL);
-      return NULL;
-    }
-
   __libc_lock_lock (tzset_lock);
 
   /* Update internal database according to current TZ setting.
@@ -584,14 +578,14 @@  __tz_convert (const time_t *timer, int use_localtime, struct tm *tp)
   tzset_internal (tp == &_tmbuf && use_localtime);
 
   if (__use_tzfile)
-    __tzfile_compute (*timer, use_localtime, &leap_correction,
+    __tzfile_compute (timer, use_localtime, &leap_correction,
 		      &leap_extra_secs, tp);
   else
     {
       if (! __offtime (timer, 0, tp))
 	tp = NULL;
       else
-	__tz_compute (*timer, tp, use_localtime);
+	__tz_compute (timer, tp, use_localtime);
       leap_correction = 0L;
       leap_extra_secs = 0;
     }