[PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert

[PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert

[Albert ARIBAUD (3ADEV)]

This is the first batch of Y2038 support patches.

The first patch provides __time64_t, the 64-bit counterpart of time_t,
to be used in 64-bit-time implementations of public APIs related to time.

The second makes __tz_convert compatible with 64-bit time. This implies
creating 64-bit-time versions of its callers and turning their original
(32-bit-time) versions into wrappers. 

Albert ARIBAUD (3ADEV) (2):
  Y2038: add type __time64_t
  Y2038: make __tz_convert compatible with 64-bit-time

 bits/typesizes.h                              |  1 +
 include/time.h                                | 29 +++++++++++---
 posix/bits/types.h                            |  3 +-
 sysdeps/mach/hurd/bits/typesizes.h            |  1 +
 .../unix/sysv/linux/alpha/bits/typesizes.h    |  1 +
 .../unix/sysv/linux/generic/bits/typesizes.h  |  1 +
 sysdeps/unix/sysv/linux/s390/bits/typesizes.h |  1 +
 .../unix/sysv/linux/sparc/bits/typesizes.h    |  1 +
 sysdeps/unix/sysv/linux/x86/bits/typesizes.h  |  1 +
 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 ++++++---------
 17 files changed, 160 insertions(+), 56 deletions(-)

-- 
2.17.1

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

[Albert ARIBAUD (3ADEV)]

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(-)

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);
 }
 \f
-/* 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)
 \f
-/* 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;
     }
-- 
2.17.1

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

[Florian Weimer]

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

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

[Albert ARIBAUD]

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

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

[Florian Weimer]

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

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

[Albert ARIBAUD]

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

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

[Albert ARIBAUD]

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

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

[Florian Weimer]

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

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

[Joseph Myers]

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.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Joseph Myers]

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 S. Myers
joseph@codesourcery.com

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

[Florian Weimer]

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

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

[Paul Eggert]

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.

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

[Florian Weimer]

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

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

[Albert ARIBAUD]

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

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

[Joseph Myers]

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 S. Myers
joseph@codesourcery.com

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

[Joseph Myers]

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 S. Myers
joseph@codesourcery.com

[PATCH 1/2] Y2038: add type __time64_t

[Albert ARIBAUD (3ADEV)]

This type is public, so that time_t can be a typedef of it
when we switch the public API to 64-bit time.

Also, provide a function to check if a __time64_t value
fits in a (32-bit) __time_t.
---
 bits/typesizes.h                                 | 1 +
 include/time.h                                   | 9 ++++++++-
 posix/bits/types.h                               | 3 ++-
 sysdeps/mach/hurd/bits/typesizes.h               | 1 +
 sysdeps/unix/sysv/linux/alpha/bits/typesizes.h   | 1 +
 sysdeps/unix/sysv/linux/generic/bits/typesizes.h | 1 +
 sysdeps/unix/sysv/linux/s390/bits/typesizes.h    | 1 +
 sysdeps/unix/sysv/linux/sparc/bits/typesizes.h   | 1 +
 sysdeps/unix/sysv/linux/x86/bits/typesizes.h     | 1 +
 9 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/bits/typesizes.h b/bits/typesizes.h
index 85eacf2518..0b6a19c230 100644
--- a/bits/typesizes.h
+++ b/bits/typesizes.h
@@ -48,6 +48,7 @@
 #define	__ID_T_TYPE		__U32_TYPE
 #define __CLOCK_T_TYPE		__SLONGWORD_TYPE
 #define __TIME_T_TYPE		__SLONGWORD_TYPE
+#define __TIME64_T_TYPE		__SQUAD_TYPE
 #define __USECONDS_T_TYPE	__U32_TYPE
 #define __SUSECONDS_T_TYPE	__SLONGWORD_TYPE
 #define __DADDR_T_TYPE		__S32_TYPE
diff --git a/include/time.h b/include/time.h
index 23d2580528..93638aa215 100644
--- a/include/time.h
+++ b/include/time.h
@@ -3,6 +3,7 @@
 
 #ifndef _ISOMAC
 # include <bits/types/locale_t.h>
+# include <stdbool.h>
 
 extern __typeof (strftime_l) __strftime_l;
 libc_hidden_proto (__strftime_l)
@@ -101,10 +102,16 @@ extern char * __strptime_internal (const char *rp, const char *fmt,
 
 extern double __difftime (time_t time1, time_t time0);
 
-
 /* Use in the clock_* functions.  Size of the field representing the
    actual clock ID.  */
 #define CLOCK_IDFIELD_SIZE	3
 
+/* check whether a time64_t value fits in a time_t */
+static inline bool
+fits_in_time_t (__time64_t t)
+{
+  return t == (time_t) t;
+}
+
 #endif
 #endif
diff --git a/posix/bits/types.h b/posix/bits/types.h
index 5e22ce41bf..4c6553a266 100644
--- a/posix/bits/types.h
+++ b/posix/bits/types.h
@@ -155,7 +155,8 @@ __STD_TYPE __CLOCK_T_TYPE __clock_t;	/* Type of CPU usage counts.  */
 __STD_TYPE __RLIM_T_TYPE __rlim_t;	/* Type for resource measurement.  */
 __STD_TYPE __RLIM64_T_TYPE __rlim64_t;	/* Type for resource measurement (LFS).  */
 __STD_TYPE __ID_T_TYPE __id_t;		/* General type for IDs.  */
-__STD_TYPE __TIME_T_TYPE __time_t;	/* Seconds since the Epoch.  */
+__STD_TYPE __TIME_T_TYPE __time_t;	/* Seconds since the Epoch, Y2038-unsafe.  */
+__STD_TYPE __TIME64_T_TYPE __time64_t;	/* Seconds since the Epoch, Y2038-safe.  */
 __STD_TYPE __USECONDS_T_TYPE __useconds_t; /* Count of microseconds.  */
 __STD_TYPE __SUSECONDS_T_TYPE __suseconds_t; /* Signed count of microseconds.  */
 
diff --git a/sysdeps/mach/hurd/bits/typesizes.h b/sysdeps/mach/hurd/bits/typesizes.h
index d3026ba66e..c9dbe2bf33 100644
--- a/sysdeps/mach/hurd/bits/typesizes.h
+++ b/sysdeps/mach/hurd/bits/typesizes.h
@@ -48,6 +48,7 @@
 #define	__ID_T_TYPE		__U32_TYPE
 #define __CLOCK_T_TYPE		__SLONGWORD_TYPE
 #define __TIME_T_TYPE		__SLONGWORD_TYPE
+#define __TIME64_T_TYPE		__SQUAD_TYPE
 #define __USECONDS_T_TYPE	__U32_TYPE
 #define __SUSECONDS_T_TYPE	__SLONGWORD_TYPE
 #define __DADDR_T_TYPE		__S32_TYPE
diff --git a/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h b/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h
index d9a0b0467c..47b1e36165 100644
--- a/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h
+++ b/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h
@@ -47,6 +47,7 @@
 #define	__ID_T_TYPE		__U32_TYPE
 #define __CLOCK_T_TYPE		__SLONGWORD_TYPE
 #define __TIME_T_TYPE		__SLONGWORD_TYPE
+#define __TIME64_T_TYPE		__SQUAD_TYPE
 #define __USECONDS_T_TYPE	__U32_TYPE
 #define __SUSECONDS_T_TYPE	__S64_TYPE
 #define __DADDR_T_TYPE		__S32_TYPE
diff --git a/sysdeps/unix/sysv/linux/generic/bits/typesizes.h b/sysdeps/unix/sysv/linux/generic/bits/typesizes.h
index a2cb3433bf..db03b1ae8b 100644
--- a/sysdeps/unix/sysv/linux/generic/bits/typesizes.h
+++ b/sysdeps/unix/sysv/linux/generic/bits/typesizes.h
@@ -49,6 +49,7 @@
 #define	__ID_T_TYPE		__U32_TYPE
 #define __CLOCK_T_TYPE		__SLONGWORD_TYPE
 #define __TIME_T_TYPE		__SLONGWORD_TYPE
+#define __TIME64_T_TYPE		__SQUAD_TYPE
 #define __USECONDS_T_TYPE	__U32_TYPE
 #define __SUSECONDS_T_TYPE	__SLONGWORD_TYPE
 #define __DADDR_T_TYPE		__S32_TYPE
diff --git a/sysdeps/unix/sysv/linux/s390/bits/typesizes.h b/sysdeps/unix/sysv/linux/s390/bits/typesizes.h
index fdaa421958..138c6d6d10 100644
--- a/sysdeps/unix/sysv/linux/s390/bits/typesizes.h
+++ b/sysdeps/unix/sysv/linux/s390/bits/typesizes.h
@@ -48,6 +48,7 @@
 #define	__ID_T_TYPE		__U32_TYPE
 #define __CLOCK_T_TYPE		__SLONGWORD_TYPE
 #define __TIME_T_TYPE		__SLONGWORD_TYPE
+#define __TIME64_T_TYPE		__SQUAD_TYPE
 #define __USECONDS_T_TYPE	__U32_TYPE
 #define __SUSECONDS_T_TYPE	__SLONGWORD_TYPE
 #define __DADDR_T_TYPE		__S32_TYPE
diff --git a/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h b/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h
index b59f2ced75..5d6fb00a93 100644
--- a/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h
+++ b/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h
@@ -48,6 +48,7 @@
 #define	__ID_T_TYPE		__U32_TYPE
 #define __CLOCK_T_TYPE		__SLONGWORD_TYPE
 #define __TIME_T_TYPE		__SLONGWORD_TYPE
+#define __TIME64_T_TYPE		__SQUAD_TYPE
 #define __USECONDS_T_TYPE	__U32_TYPE
 #define __SUSECONDS_T_TYPE	__S32_TYPE
 #define __DADDR_T_TYPE		__S32_TYPE
diff --git a/sysdeps/unix/sysv/linux/x86/bits/typesizes.h b/sysdeps/unix/sysv/linux/x86/bits/typesizes.h
index e6f7481a19..f5d792d8f3 100644
--- a/sysdeps/unix/sysv/linux/x86/bits/typesizes.h
+++ b/sysdeps/unix/sysv/linux/x86/bits/typesizes.h
@@ -62,6 +62,7 @@
 #define __ID_T_TYPE		__U32_TYPE
 #define __CLOCK_T_TYPE		__SYSCALL_SLONG_TYPE
 #define __TIME_T_TYPE		__SYSCALL_SLONG_TYPE
+#define __TIME64_T_TYPE		__SQUAD_TYPE
 #define __USECONDS_T_TYPE	__U32_TYPE
 #define __SUSECONDS_T_TYPE	__SYSCALL_SLONG_TYPE
 #define __DADDR_T_TYPE		__S32_TYPE
-- 
2.17.1

Re: [PATCH 1/2] Y2038: add type __time64_t

[Paul Eggert]

Albert ARIBAUD (3ADEV) wrote:
> -__STD_TYPE __TIME_T_TYPE __time_t;	/* Seconds since the Epoch.  */
> +__STD_TYPE __TIME_T_TYPE __time_t;	/* Seconds since the Epoch, Y2038-unsafe.  */

This change to commentary is confusing, as __time_t is Y2038-safe on 64-bit 
hosts. I suggest omitting the change, and changing the comment on the next line 
to "Seconds since the Epoch (TIME_BITS==64)" (or whatever the name of that macro 
is).

Re: [PATCH 1/2] Y2038: add type __time64_t

[Albert ARIBAUD]

Hi Paul,

On Wed, 13 Jun 2018 01:38:09 -0700, Paul Eggert <eggert@cs.ucla.edu>
wrote :

> Albert ARIBAUD (3ADEV) wrote:
> > -__STD_TYPE __TIME_T_TYPE __time_t;	/* Seconds since the Epoch.  */
> > +__STD_TYPE __TIME_T_TYPE __time_t;	/* Seconds since the Epoch, Y2038-unsafe.  */  
> 
> This change to commentary is confusing, as __time_t is Y2038-safe on 64-bit 
> hosts. I suggest omitting the change, and changing the comment on the next line 
> to "Seconds since the Epoch (TIME_BITS==64)" (or whatever the name of that macro 
> is).

Fixed locally as suggested, will repost once 2/2 discussion is complete.

Cordialement,
Albert ARIBAUD
3ADEV

Re: [PATCH 1/2] Y2038: add type __time64_t

[Joseph Myers]

There is a key definition you are missing in this patch: one that says 
whether the particular glibc configuration supports 32-bit time_t at all.

This new macro could go in bits/wordsize.h, or in its own installed 
header.  The value is normally the same as (__WORDSIZE == 32), *except* on 
x32, which already has 64-bit time_t despite 32-bit wordsize.

The new macro would be used, in the installed headers, to control whether 
an explicit _TIME_BITS=32 is valid or not.  It would also be used, in 
glibc internals, to control whether there are separate 32-bit interfaces 
that wrap 64-bit ones, or whether the 64-bit names are just #defined to 
the generic ones (#define __clock_gettime64 __clock_gettime, in some 
internal header, for example) to avoid creating any new interfaces in the 
case where time_t is already 64-bit, while avoiding the need for lots of 
conditionals where glibc-internal code written for 64-bit time_t wishes to 
call such interfaces.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 1/2] Y2038: add type __time64_t

[Albert ARIBAUD]

Hi Joseph,

On Wed, 13 Jun 2018 14:13:00 +0000, Joseph Myers
<joseph@codesourcery.com> wrote :

> There is a key definition you are missing in this patch: one that says 
> whether the particular glibc configuration supports 32-bit time_t at all.
> 
> This new macro could go in bits/wordsize.h, or in its own installed 
> header.  The value is normally the same as (__WORDSIZE == 32), *except* on 
> x32, which already has 64-bit time_t despite 32-bit wordsize.
> 
> The new macro would be used, in the installed headers, to control whether 
> an explicit _TIME_BITS=32 is valid or not.  It would also be used, in 
> glibc internals, to control whether there are separate 32-bit interfaces 
> that wrap 64-bit ones, or whether the 64-bit names are just #defined to 
> the generic ones (#define __clock_gettime64 __clock_gettime, in some 
> internal header, for example) to avoid creating any new interfaces in the 
> case where time_t is already 64-bit, while avoiding the need for lots of 
> conditionals where glibc-internal code written for 64-bit time_t wishes to 
> call such interfaces.

Which would be best:

- a "boolean" macro (e.g. __TIMESIZE32) defined as (__WORDSIZE == 32)
  except for x32 where it would be defined as 1, or

- a "size" macro (e.g. __TIMESIZE) defined equal to __WORDSIZE except
  for x32 where it would be defined as 64?

I have a slight preference for the "size" form.

Cordialement,
Albert ARIBAUD
3ADEV

Re: [PATCH 1/2] Y2038: add type __time64_t

[Paul Eggert]

Albert ARIBAUD wrote:
> - a "size" macro (e.g. __TIMESIZE) defined equal to __WORDSIZE except
>    for x32 where it would be defined as 64?
> 
> I have a slight preference for the "size" form.

Works for me. Though C11 uses _WIDTH for names that count bit widths of 
integers, so I'd mildly prefer __TIME_WIDTH as it is a new name.

Re: [PATCH 1/2] Y2038: add type __time64_t

[Joseph Myers]

On Wed, 13 Jun 2018, Paul Eggert wrote:

> Albert ARIBAUD wrote:
> > - a "size" macro (e.g. __TIMESIZE) defined equal to __WORDSIZE except
> >    for x32 where it would be defined as 64?
> > 
> > I have a slight preference for the "size" form.
> 
> Works for me. Though C11 uses _WIDTH for names that count bit widths of
> integers, so I'd mildly prefer __TIME_WIDTH as it is a new name.

Whatever the name, it needs to be clear that this macro relates to the 
time_t used by default (unsuffixed) functions in this glibc - *not* 
necessarily the time_t for the current compilation.  (Rather, _TIME_BITS 
== 64 && (new macro) == 32 would be the condition for the installed 
headers to need to redirect calls to time-related functions.)

The width macros are from TS 18661-1, not C11 (hopefully they'll end up as 
an unconditional feature of C2x without requiring 
__STDC_WANT_IEC_60559_BFP_EXT__, since there is demand for them outside 
the floating-point context of passing them to the fromfp functions).  If 
there were a TIME_WIDTH addition to that set of macros, I'd expect it to 
relate to the current compilation.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 1/2] Y2038: add type __time64_t

[Paul Eggert]

On 06/13/2018 09:39 AM, Joseph Myers wrote:
> Whatever the name, it needs to be clear that this macro relates to the
> time_t used by default (unsuffixed) functions in this glibc -*not*  
> necessarily the time_t for the current compilation.

OK, in that case perhaps it's better to stick to __TIMESIZE, as the 
_WIDTH names are intended to apply to the current compilation. (Sorry 
about confusing C11 with TS 18661-1.)

Re: [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert

[Albert ARIBAUD]

Added note (will be part of cover letter for all future Y2038 patch
posts):

All Y2038 patches are tested functionally using an ad hoc framework
published at https://github.com/3adev/y2038 which runs tests using times
below, at and beyond the Y2038 limit. The tests are run on ARM
architecture.

These patches have also been tested using build-many-glibcs.py, always
for x86_64, aarch64, arm and powerpc compilers, and regularly for all
compilers, and found to give the same results as those of the current
glibc master above which the patches were created.

On Wed, 13 Jun 2018 09:00:17 +0200, "Albert ARIBAUD (3ADEV)"
<albert.aribaud@3adev.fr> wrote :

> This is the first batch of Y2038 support patches.
> 
> The first patch provides __time64_t, the 64-bit counterpart of time_t,
> to be used in 64-bit-time implementations of public APIs related to time.
> 
> The second makes __tz_convert compatible with 64-bit time. This implies
> creating 64-bit-time versions of its callers and turning their original
> (32-bit-time) versions into wrappers. 
> 
> Albert ARIBAUD (3ADEV) (2):
>   Y2038: add type __time64_t
>   Y2038: make __tz_convert compatible with 64-bit-time
> 
>  bits/typesizes.h                              |  1 +
>  include/time.h                                | 29 +++++++++++---
>  posix/bits/types.h                            |  3 +-
>  sysdeps/mach/hurd/bits/typesizes.h            |  1 +
>  .../unix/sysv/linux/alpha/bits/typesizes.h    |  1 +
>  .../unix/sysv/linux/generic/bits/typesizes.h  |  1 +
>  sysdeps/unix/sysv/linux/s390/bits/typesizes.h |  1 +
>  .../unix/sysv/linux/sparc/bits/typesizes.h    |  1 +
>  sysdeps/unix/sysv/linux/x86/bits/typesizes.h  |  1 +
>  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 ++++++---------
>  17 files changed, 160 insertions(+), 56 deletions(-)
> 



Cordialement,
Albert ARIBAUD
3ADEV

Re: [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert

[Joseph Myers]

On Wed, 13 Jun 2018, Albert ARIBAUD wrote:

> These patches have also been tested using build-many-glibcs.py, always
> for x86_64, aarch64, arm and powerpc compilers, and regularly for all
> compilers, and found to give the same results as those of the current
> glibc master above which the patches were created.

Since build-many-glibcs.py should give clean results everywhere except 
i686-gnu, and since this patch adds Versions entries at a public ABI 
version but has no *.abilist updates, I'm skeptical of the above 
description of testing.  You should have no ABI test failures before the 
patch series, anywhere, whether in your normal ARM testing or with 
build-many-glibcs.py, and without *.abilist updates you should be seeing 
such failures after the patch series.

Any patch adding new ABIs at public versions always needs to include the 
ABI baseline updates for *all* glibc configurations.

You should do full execution testing for at least one 64-bit 
configuration, as well.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert

[Albert ARIBAUD]

Hi Joseph,

On Wed, 13 Jun 2018 14:08:16 +0000, Joseph Myers
<joseph@codesourcery.com> wrote :

> On Wed, 13 Jun 2018, Albert ARIBAUD wrote:
> 
> > These patches have also been tested using build-many-glibcs.py, always
> > for x86_64, aarch64, arm and powerpc compilers, and regularly for all
> > compilers, and found to give the same results as those of the current
> > glibc master above which the patches were created.  
> 
> Since build-many-glibcs.py should give clean results everywhere except 
> i686-gnu, and since this patch adds Versions entries at a public ABI 
> version but has no *.abilist updates, I'm skeptical of the above 
> description of testing.  You should have no ABI test failures before the 
> patch series, anywhere, whether in your normal ARM testing or with 
> build-many-glibcs.py, and without *.abilist updates you should be seeing 
> such failures after the patch series.
>
> Any patch adding new ABIs at public versions always needs to include the 
> ABI baseline updates for *all* glibc configurations.

Right now I only update abilists for ARM and PowerPC as these are
the ones for which I run functional testing. I can update others but I
won't be able to test them functionally.

In any case, I'll group together related changes to Versions and
abilists in the same patch.  

> You should do full execution testing for at least one 64-bit 
> configuration, as well.

My minimal set includes x86_64-linux-gnu, aarch64-linux-gnu,
arm-linux-gnueabi and powerpc-linux-gnu, so this covers two different
64-bit configurations.

(my maximal set is 'all compilers and all glibcs' -- including i686-gnu
for the sake of it -- but as this takes around 30 hours on a reasonably
fast machine, I don't run it for every change.)

Cordialement,
Albert ARIBAUD
3ADEV