[RFC 0/7] y2038: clock_settime rework to be Y2038 safe on 32bit systems

[RFC 0/7] y2038: clock_settime rework to be Y2038 safe on 32bit systems

[Lukasz Majewski]

This patch set is a RFC to present the conversion and support
of clock_settime to be Y2038 safe.

This work is based on a previous development/patches:
https://libc-alpha.sourceware.narkive.com/zniMOWui/rfc-patch-00-52-make-glibc-y2038-proof#post68

And shall be applied on top of:
https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes

Github repository:
https://github.com/lmajewski/y2038_glibc/commits/Y2038-2.29-glibc-__clock_settime64_v1-27-03-2019

Shall be used with provided meta-y2038 for development and testing:
https://github.com/lmajewski/meta-y2038

I've used guidelines from:
https://www.gnu.org/software/libc/manual/html_mono/libc.html
"D.2.1 64-bit time symbol handling in the GNU C Library"
to convert *clock_settime*.


- The clock_settime() has been choosen as a relatively easy syscall
for conversion (i.e. no VDO)

- The idea for proceeding: convert a single syscall (clock_settime) or related group
  of syscalls (clock_gettime, clock_nanosleep,clock_getres, clock_settime)
 - to keep the number of changes manageable

- The explicit 64 bit time related syscalls (Y2038 safe) have been already accepted
to mainline Linux kernel (5.1-rc2):
https://elixir.bootlin.com/linux/v5.1-rc2/source/arch/arm/tools/syscall.tbl#L420

- Test(s) for Y2038 (with using clock_settime) would be added to glibc after the 
pair of clock_settime/clock_gettime is accepted/reviewed

- Last two patches of this series shows the way how the Y2038 support is going to
be added to glibc.

- The important question about enabling Y2038 support in glibc:
  -- If a well tested group of functions/syscalls (like clock_*) is accepted with
     the Y2038 support enabled (_TIME_BITS passed to user program enables 64 bit time)

  or

  -- The Y2038 support is enabled only when _ALL_ syscalls/functions are Y2038 ready

- I've read that clock_* functions were moved from librt to glibc - what was the
motivation behind this move? Why to we have compatibility code in the rt/* for
clock_settime() if the _real_ support is in time/* ?	
	
Comments are more than welcome.


Lukasz Majewski (7):
  y2038: Introduce struct __timespec64
  y2038: Provide conversion helpers for struct __timespec64
  y2038: clock_settime: Provide __clock_settime64 implementation for
    linux
  y2038: rt: clock_settime: Convert __clock_settime to __clock_settime64
  y2038: clock_settime: implementation for Unix generic
  y2038: Support for Y2038 safe time on 32 bit systems (generic code)
  y2038: Introduce support for clock_settime() being Y2038 safe

 include/features.h                      | 19 +++++++
 include/time.h                          | 97 ++++++++++++++++++++++++++++++++-
 manual/creature.texi                    | 28 ++++++++++
 rt/Versions                             |  2 +-
 rt/clock-compat.c                       |  3 -
 rt/clock_settime.c                      |  4 +-
 sysdeps/unix/clock_settime.c            | 23 ++++++--
 sysdeps/unix/sysv/linux/clock_settime.c | 51 +++++++++++++----
 time/Makefile                           |  2 +-
 time/bits/types/struct___timespec64.h   | 39 +++++++++++++
 time/bits/types/struct_timespec.h       |  8 +++
 time/bits/types/time_t.h                |  4 ++
 time/time.h                             |  9 +++
 13 files changed, 265 insertions(+), 24 deletions(-)
 create mode 100644 time/bits/types/struct___timespec64.h

-- 
2.11.0

[RFC 1/7] y2038: Introduce struct __timespec64

[Lukasz Majewski]

This type is a glibc's type similar to struct timespec
but whose tv_sec field is a __time64_t rather than a time_t,
which makes it Y2038-proof and usable to pass between user
code and Y2038-proof kernel syscalls (e.g. clock_gettime()).

On 64-bit architectures, and on X32, struct __timespec64 is
just an alias of struct timespec, which is already 64-bit.
On other architectures, it must be explicitly defined.

When passing this structure to the kernel - it ensures that the
higher half of tv_nsec is always 0, which means that glibc
can reuse the public type (as tv_pad is zeroed anyway).

Moreover, the tv_nsec has an anonymous 32 bit padding (ordered
according to the endianness of the architecture) to prevent user space
programs, depending on long, 32 bit, tv_nsec from breaking.

Tested on x86_64 and ARM.

* time/bits/types/struct___timespec64.h:
  Create new file time/bits/types/struct___timespec64.h
* include/time.h: Add # include <time/bits/types/struct___timespec64.h>
* time/Makefile:
  Add bits/types/struct___timespec64.h to Makefile's headers
---
 include/time.h                        |  1 +
 time/Makefile                         |  2 +-
 time/bits/types/struct___timespec64.h | 39 +++++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 time/bits/types/struct___timespec64.h

diff --git a/include/time.h b/include/time.h
index ac3163c2a5..d8a3ef35e2 100644
--- a/include/time.h
+++ b/include/time.h
@@ -5,6 +5,7 @@
 # include <bits/types/locale_t.h>
 # include <stdbool.h>
 # include <time/mktime-internal.h>
+# include <time/bits/types/struct___timespec64.h>
 
 extern __typeof (strftime_l) __strftime_l;
 libc_hidden_proto (__strftime_l)
diff --git a/time/Makefile b/time/Makefile
index 5c6304ece1..dc9ad55678 100644
--- a/time/Makefile
+++ b/time/Makefile
@@ -27,7 +27,7 @@ headers := time.h sys/time.h sys/timeb.h bits/time.h			\
 	   bits/types/struct_itimerspec.h				\
 	   bits/types/struct_timespec.h bits/types/struct_timeval.h	\
 	   bits/types/struct_tm.h bits/types/timer_t.h			\
-	   bits/types/time_t.h
+	   bits/types/time_t.h bits/types/struct___timespec64.h
 
 routines := offtime asctime clock ctime ctime_r difftime \
 	    gmtime localtime mktime time		 \
diff --git a/time/bits/types/struct___timespec64.h b/time/bits/types/struct___timespec64.h
new file mode 100644
index 0000000000..9946717af6
--- /dev/null
+++ b/time/bits/types/struct___timespec64.h
@@ -0,0 +1,39 @@
+/* NB: Include guard matches what <linux/time.h> uses.  */
+/* #ifndef _STRUCT_TIMESPEC
+# error "Never include <bits/types/struct___timespec64.h> directly !"
+#endif */
+
+#ifndef _STRUCT___TIMESPEC64
+#define _STRUCT___TIMESPEC64 1
+
+#include <bits/types.h>
+#include <bits/timesize.h>
+#include <endian.h>
+
+/* The glibc Y2038-proof structure for a time value.
+   To keep things Posix-ish, we keep the nanoseconds field a 32-bit
+   signed long, but since the Linux field is a 64-bit signed int, we
+   pad our tv_nsec with a 32-bit bitfield, which should always be 0.
+   */
+#if __TIMESIZE==64
+# define __timespec64 timespec
+#else
+#define TIMSPEC64_TV_PAD_DEFINED
+# if BYTE_ORDER == BIG_ENDIAN
+struct __timespec64
+{
+  __time64_t tv_sec;		/* Seconds */
+  int tv_pad: 32;		/* Padding named for checking/setting */
+  __syscall_slong_t tv_nsec;	/* Nanoseconds */
+};
+# else
+struct __timespec64
+{
+  __time64_t tv_sec;		/* Seconds */
+  __syscall_slong_t tv_nsec;	/* Nanoseconds */
+  int tv_pad: 32;		/* Padding named for checking/setting */
+};
+# endif
+#endif
+
+#endif
-- 
2.11.0

[RFC 2/7] y2038: Provide conversion helpers for struct __timespec64

[Lukasz Majewski]

Those functions allow easy conversion between Y2038 safe struct __timespec64
and other time related data structures.

* include/time.h (valid_timeval_to_timespec64): Add.
* include/time.h (valid_timespec_to_timespec64): Likewise.
* include/time.h (valid_timespec64_to_timespec): Likewise.
* include/time.h (valid_timespec64_to_timeval): Likewise.
* include/time.h (IS_VALID_NANOSECONDS): Likewise.
* include/time.h (timespec_to_timespec64): Likewise.
* include/time.h (timespec64_to_timespec): Likewise.
* include/time.h (timespec64_to_timeval): Likewise.
---
 include/time.h | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/include/time.h b/include/time.h
index d8a3ef35e2..09231073b5 100644
--- a/include/time.h
+++ b/include/time.h
@@ -157,5 +157,90 @@ in_time_t_range (__time64_t t)
   return s == t;
 }
 
+/* Convert a known valid struct timeval into a struct __timespec64.  */
+static inline void
+valid_timeval_to_timespec64 (const struct timeval *tv32,
+			     struct __timespec64 *ts64)
+{
+  ts64->tv_sec = tv32->tv_sec;
+  ts64->tv_nsec = tv32->tv_usec * 1000;
+}
+
+/* Convert a known valid struct timespec into a struct __timespec64.  */
+static inline void
+valid_timespec_to_timespec64 (const struct timespec *ts32,
+			      struct __timespec64 *ts64)
+{
+  ts64->tv_sec = ts32->tv_sec;
+  ts64->tv_nsec = ts32->tv_nsec;
+  /* We only need to zero ts64->tv_pad if we pass it to the kernel.  */
+}
+
+/* Convert a known valid struct __timespec64 into a struct timespec.  */
+static inline void
+valid_timespec64_to_timespec (const struct __timespec64 *ts64,
+			      struct timespec *ts32)
+{
+  ts32->tv_sec = (time_t) ts64->tv_sec;
+  ts32->tv_nsec = ts64->tv_nsec;
+}
+
+/* Convert a known valid struct __timespec64 into a struct timeval.  */
+static inline void
+valid_timespec64_to_timeval (const struct __timespec64 *ts64,
+			     struct timeval *tv32)
+{
+  tv32->tv_sec = (time_t) ts64->tv_sec;
+  tv32->tv_usec = ts64->tv_nsec / 1000;
+}
+
+/* Check if a value lies with the valid nanoseconds range.  */
+#define IS_VALID_NANOSECONDS(ns) (ns >= 0 && ns <= 999999999)
+
+/* Check and convert a struct timespec into a struct __timespec64.  */
+static inline bool
+timespec_to_timespec64 (const struct timespec *ts32,
+					   struct __timespec64 *ts64)
+{
+  /* Check that ts32 holds a valid count of nanoseconds.  */
+  if (! IS_VALID_NANOSECONDS (ts32->tv_nsec))
+    return false;
+  /* All ts32 fields can fit in ts64, so copy them.  */
+  valid_timespec_to_timespec64 (ts32, ts64);
+  /* We only need to zero ts64->tv_pad if we pass it to the kernel.  */
+  return true;
+}
+
+/* Check and convert a struct __timespec64 into a struct timespec.  */
+static inline bool
+timespec64_to_timespec (const struct __timespec64 *ts64,
+					   struct timespec *ts32)
+{
+  /* Check that tv_nsec holds a valid count of nanoseconds.  */
+  if (! IS_VALID_NANOSECONDS (ts64->tv_nsec))
+    return false;
+  /* Check that tv_sec can fit in a __time_t.  */
+  if (! in_time_t_range (ts64->tv_sec))
+    return false;
+  /* All ts64 fields can fit in ts32, so copy them.  */
+  valid_timespec64_to_timespec (ts64, ts32);
+  return true;
+}
+
+/* Check and convert a struct __timespec64 into a struct timeval.  */
+static inline bool
+timespec64_to_timeval (const struct __timespec64 *ts64,
+					  struct timeval *tv32)
+{
+  /* Check that tv_nsec holds a valid count of nanoseconds.  */
+  if (! IS_VALID_NANOSECONDS (ts64->tv_nsec))
+    return false;
+  /* Check that tv_sec can fit in a __time_t.  */
+  if (! in_time_t_range (ts64->tv_sec))
+    return false;
+  /* All ts64 fields can fit in tv32, so copy them.  */
+  valid_timespec64_to_timeval (ts64, tv32);
+  return true;
+}
 #endif
 #endif
-- 
2.11.0

[RFC 3/7] y2038: clock_settime: Provide __clock_settime64 implementation for linux

[Lukasz Majewski]

* include/time.h: Remove __clock_settime typeof
* include/time.h: Add __clock_settime64 definition according to __TIMESIZE
* sysdeps/unix/sysv/linux/clock_settime.c: Remove clock_settime alias
* sysdeps/unix/sysv/linux/clock_settime.c:
  Add clock_settime when __TIMESIZE != 64
  Rewrite __clock_settime() to __clock_settime64 with explicit support for
  64 bit time
---
 include/time.h                          |  9 +++++++-
 sysdeps/unix/sysv/linux/clock_settime.c | 40 +++++++++++++++++++++++----------
 2 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/include/time.h b/include/time.h
index 09231073b5..c8c0a6936e 100644
--- a/include/time.h
+++ b/include/time.h
@@ -22,7 +22,6 @@ libc_hidden_proto (strptime)
 extern __typeof (clock_getres) __clock_getres;
 extern __typeof (clock_gettime) __clock_gettime;
 libc_hidden_proto (__clock_gettime)
-extern __typeof (clock_settime) __clock_settime;
 extern __typeof (clock_nanosleep) __clock_nanosleep;
 extern __typeof (clock_getcpuclockid) __clock_getcpuclockid;
 
@@ -103,6 +102,14 @@ extern __time64_t __timegm64 (struct tm *__tp) __THROW;
 libc_hidden_proto (__timegm64)
 #endif
 
+#if __TIMESIZE == 64
+# define __clock_settime64 clock_settime
+#else
+extern int __clock_settime64 (clockid_t clock_id,
+                              const struct __timespec64 *tp);
+libc_hidden_proto (__clock_settime64)
+#endif
+
 /* 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.
diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c
index d837e3019c..5b7ab7fcb3 100644
--- a/sysdeps/unix/sysv/linux/clock_settime.c
+++ b/sysdeps/unix/sysv/linux/clock_settime.c
@@ -19,19 +19,35 @@
 #include <sysdep.h>
 #include <time.h>
 
-#include "kernel-posix-cpu-timers.h"
+int
+__clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp)
+{
+	/* Make sure the time cvalue is OK.  */
+	if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000)
+		{
+			__set_errno (EINVAL);
+			return -1;
+		}
+
+	return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp);
+}
 
-/* Set CLOCK to value TP.  */
+#if __TIMESIZE != 64
 int
-__clock_settime (clockid_t clock_id, const struct timespec *tp)
+clock_settime (clockid_t clock_id, const struct timespec *tp)
 {
-  /* Make sure the time cvalue is OK.  */
-  if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000)
-    {
-      __set_errno (EINVAL);
-      return -1;
-    }
-
-  return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp);
+	struct __timespec64 ts64;
+
+	if (! in_time_t_range (tp->tv_sec))
+		{
+			__set_errno (EOVERFLOW);
+			return -1;
+		}
+
+	valid_timespec_to_timespec64 (tp, &ts64);
+	return __clock_settime64 (clock_id, &ts64);
 }
-weak_alias (__clock_settime, clock_settime)
+
+/* The clock_settime symbol needs to be public as librt is also
+   using it */
+#endif
-- 
2.11.0

[RFC 4/7] y2038: rt: clock_settime: Convert __clock_settime to __clock_settime64

[Lukasz Majewski]

* rt/Versions: Replace __clock_settime with __clock_settime64 in GLIBC_PRIVATE
* rt/clock-compat.c:
  Remove COMPAT_REDIRECT for clock_settime (redirection done in time/time.h)
* rt/clock_settime.c: Rename __clock_settime to __clock_settime64
---
 rt/Versions        | 2 +-
 rt/clock-compat.c  | 3 ---
 rt/clock_settime.c | 4 ++--
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/rt/Versions b/rt/Versions
index 91e3fd2a20..d1cf04a56d 100644
--- a/rt/Versions
+++ b/rt/Versions
@@ -5,7 +5,7 @@ libc {
     clock_nanosleep;
   }
   GLIBC_PRIVATE {
-    __clock_getres; __clock_gettime; __clock_settime; __clock_getcpuclockid;
+    __clock_getres; __clock_gettime; __clock_settime64; __clock_getcpuclockid;
     __clock_nanosleep;
   }
 }
diff --git a/rt/clock-compat.c b/rt/clock-compat.c
index d8ced3cdc1..6987a0b6bf 100644
--- a/rt/clock-compat.c
+++ b/rt/clock-compat.c
@@ -48,9 +48,6 @@ COMPAT_REDIRECT (clock_getres,
 COMPAT_REDIRECT (clock_gettime,
 		 (clockid_t clock_id, struct timespec *tp),
 		 (clock_id, tp))
-COMPAT_REDIRECT (clock_settime,
-		 (clockid_t clock_id, const struct timespec *tp),
-		 (clock_id, tp))
 COMPAT_REDIRECT (clock_getcpuclockid,
 		 (pid_t pid, clockid_t *clock_id),
 		 (pid, clock_id))
diff --git a/rt/clock_settime.c b/rt/clock_settime.c
index 891925ab2c..598eb3edc5 100644
--- a/rt/clock_settime.c
+++ b/rt/clock_settime.c
@@ -21,10 +21,10 @@
 
 /* Set CLOCK to value TP.  */
 int
-__clock_settime (clockid_t clock_id, const struct timespec *tp)
+__clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp)
 {
   __set_errno (ENOSYS);
   return -1;
 }
-weak_alias (__clock_settime, clock_settime)
+weak_alias (__clock_settime64, clock_settime)
 stub_warning (clock_settime)
-- 
2.11.0

[RFC 5/7] y2038: clock_settime: implementation for Unix generic

[Lukasz Majewski]

* sysdeps/unix/clock_settime (hp_timing_settime):
  Use __timespec64 in function prototype
* sysdeps/unix/clock_settime: Rename __clock_settime to __clock_settime64
* sysdeps/unix/clock_settime: Remove weak_alias for __clock_settime
* sysdeps/unix/clock_settime: Add clock_settime when __TIMESIZE != 64
---
 sysdeps/unix/clock_settime.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/sysdeps/unix/clock_settime.c b/sysdeps/unix/clock_settime.c
index dcf9ff660a..abaac6e293 100644
--- a/sysdeps/unix/clock_settime.c
+++ b/sysdeps/unix/clock_settime.c
@@ -34,7 +34,7 @@ extern void __pthread_clock_settime (clockid_t clock_id, hp_timing_t offset)
 
 
 static int
-hp_timing_settime (clockid_t clock_id, const struct timespec *tp)
+hp_timing_settime (clockid_t clock_id, const struct __timespec64 *tp)
 {
   hp_timing_t tsc;
   hp_timing_t usertime;
@@ -68,10 +68,9 @@ hp_timing_settime (clockid_t clock_id, const struct timespec *tp)
 }
 #endif
 
-
 /* Set CLOCK to value TP.  */
 int
-__clock_settime (clockid_t clock_id, const struct timespec *tp)
+__clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp)
 {
   int retval;
 
@@ -108,4 +107,20 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp)
 
   return retval;
 }
-weak_alias (__clock_settime, clock_settime)
+
+#if __TIMESIZE != 64
+int
+clock_settime (clockid_t clock_id, const struct timespec *tp)
+{
+	struct __timespec64 ts64;
+
+	if (! in_time_t_range (tp->tv_sec))
+		{
+			__set_errno (EOVERFLOW);
+			return -1;
+		}
+
+	valid_timespec_to_timespec64 (tp, &ts64);
+	return __clock_settime64 (clock_id, &ts64);
+}
+#endif
-- 
2.11.0

[RFC 6/7] y2038: Support for Y2038 safe time on 32 bit systems (generic code)

[Lukasz Majewski]

* include/features.h:
  Add logic to set __USE_TIME_BITS64 according to passed (when building
  userspace Y2038 safe program on 32 bit system) _TIME_BITS and
  _FILE_OFFSET_BITS
* manual/creature.texi: Add description of _TIME_BITS
* time/bits/types/struct_timespec.h:
  Add "aliasing" (in preprocessor level) for struct timespec to struct
  __timespec64 (in userspace) when running on 32 bit system with Y2038
  support
* time/bits/types/time_t.h:
  Use __time64_t as time_t when __USE_TIME_BITS64 is defined
---
 include/features.h                | 19 +++++++++++++++++++
 manual/creature.texi              | 28 ++++++++++++++++++++++++++++
 time/bits/types/struct_timespec.h |  8 ++++++++
 time/bits/types/time_t.h          |  4 ++++
 4 files changed, 59 insertions(+)

diff --git a/include/features.h b/include/features.h
index e016b3e5c7..bf1d3ccb4f 100644
--- a/include/features.h
+++ b/include/features.h
@@ -365,6 +365,25 @@
 # define __USE_FILE_OFFSET64	1
 #endif
 
+/* we need to know the word size in order to check the time size */
+#include <bits/wordsize.h>
+
+#if defined _TIME_BITS
+# if _TIME_BITS == 64
+#  if ! defined(_FILE_OFFSET_BITS) || _FILE_OFFSET_BITS != 64
+#   error _TIME_BIT==64 is allowed only when _FILE_OFFSET_BITS==64
+#  elif __WORDSIZE == 32
+#   define __USE_TIME_BITS64	1
+#  endif
+# elif __TIME_BITS == 32
+#  if __WORDSIZE > 32
+#   error __TIME_BITS=32 is not compatible with __WORDSIZE > 32
+#  endif
+# else
+#  error Invalid _TIME_BITS value (can only be 32 or 64)
+# endif
+#endif
+
 #if defined _DEFAULT_SOURCE
 # define __USE_MISC	1
 #endif
diff --git a/manual/creature.texi b/manual/creature.texi
index 8876b2ab77..10b7111043 100644
--- a/manual/creature.texi
+++ b/manual/creature.texi
@@ -165,6 +165,34 @@ This macro was introduced as part of the Large File Support extension
 (LFS).
 @end defvr
 
+@defvr Macro _TIME_BITS
+This macro determines the bit size of @code{time_t} (and therefore the
+bit size of all @code{time_t} derived types and the prototypes of all
+related functions). If @code{_TIME_BITS} is undefined, the bit size of
+time_t equals the bit size of the architecture.
+
+If @code{_TIME_BITS} is undefined, or if @code{_TIME_BITS} is defined
+to the value @code{32} and @code{__WORDSIZE} is defined to the value
+@code{32}, or or if @code{_TIME_BITS} is defined to the value @code{64}
+and @code{__WORDSIZE} is defined to the value @code{64}, nothing changes.
+
+If @code{_TIME_BITS} is defined to the value @code{64} and if
+@code{__WORDSIZE} is defined to the value @code{32}, then the @w{64 bit}
+time API and implementation are used even though the architecture word
+size is @code{32}. Also, if the kernel provides @w{64 bit} time support,
+it is used; otherwise, the @w{32 bit} kernel time support is used (with
+no provision to address kernel Y2038 shortcomings).
+
+If @code{_TIME_BITS} is defined to the value @code{32} and if
+@code{__WORDSIZE} is defined to the value @code{64}, then a compile-time
+error is emitted.
+
+If @code{_TIME_BITS} is defined to a value different from both @code{32}
+and @code{64}, then a compile-time error is emitted.
+
+This macro was introduced as part of the Y2038 support.
+@end defvr
+
 @defvr Macro _ISOC99_SOURCE
 @standards{GNU, (none)}
 If this macro is defined, features from ISO C99 are included.  Since
diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h
index 5b77c52b4f..fc5a7ec25b 100644
--- a/time/bits/types/struct_timespec.h
+++ b/time/bits/types/struct_timespec.h
@@ -4,6 +4,9 @@
 
 #include <bits/types.h>
 
+/* Use the original definition for 64-bit arches
+   or when 64-bit-time by default has *not* been requested */
+#if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
 /* POSIX.1b structure for a time value.  This is like a `struct timeval' but
    has nanoseconds instead of microseconds.  */
 struct timespec
@@ -11,5 +14,10 @@ struct timespec
   __time_t tv_sec;		/* Seconds.  */
   __syscall_slong_t tv_nsec;	/* Nanoseconds.  */
 };
+#else
+# include <bits/types/struct___timespec64.h>
+/* Use the 64-bit-time timespec by default */
+#define timespec __timespec64
+# endif
 
 #endif
diff --git a/time/bits/types/time_t.h b/time/bits/types/time_t.h
index ab8287c6fe..84d67f6ac3 100644
--- a/time/bits/types/time_t.h
+++ b/time/bits/types/time_t.h
@@ -4,6 +4,10 @@
 #include <bits/types.h>
 
 /* Returned by `time'.  */
+#ifdef __USE_TIME_BITS64
+typedef __time64_t time_t;
+#else
 typedef __time_t time_t;
+#endif
 
 #endif
-- 
2.11.0

[RFC 7/7] y2038: Introduce support for clock_settime() being Y2038 safe

[Lukasz Majewski]

This patch introduces the Y2038 specific code to make clock_settime()/
__clock_settime64() Y2038 safe on 32 bit systems.

* include/time.h:
  Remove libc_hidden_proto (__clock_settime64) - make the __clock_settime64
  publicly available for Y2038 safe system
* sysdeps/unix/sysv/linux/clock_settime.c:
  Use clock_settime64 kernel syscall (available from 5.1-rc1+ Linux) by
  32 bit Y2038 safe systems
* time/time.h: Redirect clock_settime call to __clock_settime64 on 32 bit
  Y2038 safe systems
---
 include/time.h                          |  4 +++-
 sysdeps/unix/sysv/linux/clock_settime.c | 11 +++++++++++
 time/time.h                             |  9 +++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/include/time.h b/include/time.h
index c8c0a6936e..25ddb70377 100644
--- a/include/time.h
+++ b/include/time.h
@@ -107,7 +107,9 @@ libc_hidden_proto (__timegm64)
 #else
 extern int __clock_settime64 (clockid_t clock_id,
                               const struct __timespec64 *tp);
-libc_hidden_proto (__clock_settime64)
+/* For Y2038 safe system the __clock_settime64 needs to be a visible
+   symbol as a replacement for Y2038 unsafe clock_settime()
+*/
 #endif
 
 /* Compute the `struct tm' representation of T,
diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c
index 5b7ab7fcb3..3fcce623c9 100644
--- a/sysdeps/unix/sysv/linux/clock_settime.c
+++ b/sysdeps/unix/sysv/linux/clock_settime.c
@@ -29,6 +29,17 @@ __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp)
 			return -1;
 		}
 
+/* For being Y2038 safe we check if __TIMESIZE != 64 and run the
+   64 bit syscall only when supported by the kernel.
+   As __clock_settime64() may be also called by 32 bit machine (not Y2038
+   safe) - in case of error - the default kernel syscall (clock_settime)
+   is executed */
+#if defined __NR_clock_settime64 && defined __TIMESIZE != 64
+	int retval = INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp);
+	if (! retval)
+		return retval;
+#endif
+
 	return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp);
 }
 
diff --git a/time/time.h b/time/time.h
index cba6d15260..c9c8aeccfa 100644
--- a/time/time.h
+++ b/time/time.h
@@ -222,6 +222,15 @@ extern int clock_gettime (clockid_t __clock_id, struct timespec *__tp) __THROW;
 extern int clock_settime (clockid_t __clock_id, const struct timespec *__tp)
      __THROW;
 
+#ifdef __USE_TIME_BITS64
+# if defined(__REDIRECT)
+extern int __REDIRECT (clock_settime, (clockid_t __clock_id, const struct
+     timespec *__tp), __clock_settime64) __THROW;
+# else
+# define clock_settime __clock_settime64
+# endif
+#endif
+
 # ifdef __USE_XOPEN2K
 /* High-resolution sleep with the specified clock.
 
-- 
2.11.0

Re: [RFC 2/7] y2038: Provide conversion helpers for struct __timespec64

[Andreas Schwab]

On Mär 27 2019, Lukasz Majewski <lukma@denx.de> wrote:

> +/* Check and convert a struct timespec into a struct __timespec64.  */
> +static inline bool
> +timespec_to_timespec64 (const struct timespec *ts32,
> +					   struct __timespec64 *ts64)

Style: please align with the open paren.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Joseph Myers]

On Wed, 27 Mar 2019, Lukasz Majewski wrote:

> This type is a glibc's type similar to struct timespec
> but whose tv_sec field is a __time64_t rather than a time_t,
> which makes it Y2038-proof and usable to pass between user
> code and Y2038-proof kernel syscalls (e.g. clock_gettime()).

Could you please give a detailed explanation of why this needs to go in an 
installed header, as opposed to purely in a glibc-internal header?  (All 
bits/ headers should be installed headers; the set of installed headers is 
determined by the "headers" settings in makefiles.  Since you're not 
actually including this from the public time.h in this patch, only the 
internal one, there doesn't seem to be a reason here for either the naming 
or installing the header.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 2/7] y2038: Provide conversion helpers for struct __timespec64

[Joseph Myers]

On Wed, 27 Mar 2019, Lukasz Majewski wrote:

> Those functions allow easy conversion between Y2038 safe struct __timespec64
> and other time related data structures.

Please include a detailed comment explaining whether these functions are 
relevant (but trivial) in the case where the default time_t is 32-bit.  
If not, they shouldn't be compiled in that case.

> +/* Check if a value lies with the valid nanoseconds range.  */
> +#define IS_VALID_NANOSECONDS(ns) (ns >= 0 && ns <= 999999999)

Surround macro arguments with parentheses so arbitrary expressions are 
valid.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 3/7] y2038: clock_settime: Provide __clock_settime64 implementation for linux

[Joseph Myers]

On Wed, 27 Mar 2019, Lukasz Majewski wrote:

> * include/time.h: Remove __clock_settime typeof
> * include/time.h: Add __clock_settime64 definition according to __TIMESIZE
> * sysdeps/unix/sysv/linux/clock_settime.c: Remove clock_settime alias
> * sysdeps/unix/sysv/linux/clock_settime.c:
>   Add clock_settime when __TIMESIZE != 64
>   Rewrite __clock_settime() to __clock_settime64 with explicit support for
>   64 bit time

For every patch posted, please state explicitly how that patch was tested 
(should include the glibc testsuite for at least one 32-bit and at least 
one 64-bit configuration).  That's critical information to have any 
confidence in the lack of obvious ABI / linknamespace issues in a patch.

> +int
> +__clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp)
> +{
> +	/* Make sure the time cvalue is OK.  */
> +	if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000)
> +		{
> +			__set_errno (EINVAL);
> +			return -1;
> +		}
> +
> +	return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp);
> +}

Globally, in the patch series, follow GNU style for indentation 
(two-column).

> +/* The clock_settime symbol needs to be public as librt is also
> +   using it */

Globally, in the patch series, follow GNU style for comments (start with a 
capital letter, end with '.' and two spaces).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 6/7] y2038: Support for Y2038 safe time on 32 bit systems (generic code)

[Joseph Myers]

On Wed, 27 Mar 2019, Lukasz Majewski wrote:

> * include/features.h:
>   Add logic to set __USE_TIME_BITS64 according to passed (when building
>   userspace Y2038 safe program on 32 bit system) _TIME_BITS and
>   _FILE_OFFSET_BITS

Nothing like this should be added until *every* public API has support for 
_TIME_BITS=64 (then the support for it should be added for all APIs at 
once - all the header changes and all the new symbol versions).

> +/* we need to know the word size in order to check the time size */

Comment style.

> +#if defined _TIME_BITS
> +# if _TIME_BITS == 64
> +#  if ! defined(_FILE_OFFSET_BITS) || _FILE_OFFSET_BITS != 64

Space before '('

> +#   error _TIME_BIT==64 is allowed only when _FILE_OFFSET_BITS==64

> +#   error __TIME_BITS=32 is not compatible with __WORDSIZE > 32

It should be _TIME_BITS.  Not _TIME_BIT, not __TIME_BITS.  Please spell 
the name consistently throughout.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 7/7] y2038: Introduce support for clock_settime() being Y2038 safe

[Joseph Myers]

On Wed, 27 Mar 2019, Lukasz Majewski wrote:

> +/* For Y2038 safe system the __clock_settime64 needs to be a visible
> +   symbol as a replacement for Y2038 unsafe clock_settime()
> +*/

Do not use () after a function name to indicate it's a function; see the 
GNU Coding Standards.  This applies to other comments in this patch series 
as well.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Lukasz Majewski]

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

Hi Joseph,

Thanks for a prompt response.

> On Wed, 27 Mar 2019, Lukasz Majewski wrote:
> 
> > This type is a glibc's type similar to struct timespec
> > but whose tv_sec field is a __time64_t rather than a time_t,
> > which makes it Y2038-proof and usable to pass between user
> > code and Y2038-proof kernel syscalls (e.g. clock_gettime()).  
> 
> Could you please give a detailed explanation of why this needs to go
> in an installed header, as opposed to purely in a glibc-internal
> header? 

For Y2038 safe code on 32 bit machines we do need 64 bit representation
for some legacy data structures: time_t, struct timespec, etc.

Linux kernel (5.0-rc2) is using (expecting):
struct __kernel_timespec {
	__kernel_time64_t tv_sec; /* seconds */
	long long tv_nsec; /* nanoseconds */
};

in its syscalls.

With glibc I could introduce an "internal"  struct timespec64 (with 64
bit tv_sec field) which would match the above kernel definition and
"installed" one which would be posix compliant (to be still long - 32
bits on 32 bits systems).

Instead - only one ("installed") struct __timespec64 has been
introduced to be also used as glibc internal and OS installed
one to be used by user space programs. 

> (All bits/ headers should be installed headers; the set of
> installed headers is determined by the "headers" settings in
> makefiles.

Yes. The struct___timespec64.h has been added to "headers" and on
purpose made "installed".

>  Since you're not actually including this from the public
> time.h in this patch, only the internal one, there doesn't seem to be
> a reason here for either the naming or installing the header.)
> 

The idea with this exported/installed struct __timespec64 is as
follows:

User space programs on 32 bit machines use struct timespec, which when
one passes -D_TIME_BITS==64 -D_FILE_OFFSET_BITS==64 during compilation
is replaced by struct __timespec64, which correctly handles time after
Y2038.
Then glibc functions work on struct __timespec64 and provide results to
user space program.



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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC 0/7] y2038: clock_settime rework to be Y2038 safe on 32bit systems

[Joseph Myers]

On Wed, 27 Mar 2019, Lukasz Majewski wrote:

>   -- The Y2038 support is enabled only when _ALL_ syscalls/functions are Y2038 ready

Yes, that's correct.  All the header changes to support _TIME_BITS=64, new 
symbol versions and corresponding changes to ABI baselines should be made 
at once.

> - I've read that clock_* functions were moved from librt to glibc - what 
> was the motivation behind this move?

librt depends on libpthread.  Linking against libpthread causes some 
multithreaded code paths to be used, so slowing down programs even if they 
are single-threaded.  The clock_* functions are commonly usable without 
threads, so it makes sense to have them in libc.

(Whether we should somehow avoid the slowdown from pthreads functions 
being present, and have everything in libc, is a separate question.)

> Why to we have compatibility code in the rt/* for
> clock_settime() if the _real_ support is in time/* ?	

We have to stay compatible with existing programs built with older glibc 
versions that expect to get functions such as clock_settime from librt.so.

When making changes in this area, running the glibc testsuite for a range 
of architectures is helpful - the ABI tests and linknamespace tests will 
catch some possible bugs.  But you should also manually inspect the 
clock_* symbols exported from librt.so to make sure they look the same 
before and after the change (as opposed to any GLIBC_PRIVATE __clock_* 
exports, where changes are OK as long as the symbols exported match the 
ones used in other libraries).

For example, the ABI tests do not verify whether a given symbol is a 
compat symbol or not.  If you look at the output of objdump --dynamic-syms 
on librt.so you'll see e.g.

0000000000004dc0 g   iD  .text  0000000000000008 (GLIBC_2.2.5) clock_gettime

where the parentheses around the symbol version indicate it's a compat 
symbol - so you should verify that remains the same before and after the 
patch.  The 'i' in 'iD' indicates an IFUNC because that's how the 
redirection from librt to libc versions works, so make sure it remains an 
IFUNC after the changes (on architectures supporting IFUNCs; on others, it 
should remain a wrapper).  And do this verification for both 32-bit and 
64-bit architectures.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 5/7] y2038: clock_settime: implementation for Unix generic

[Joseph Myers]

On Wed, 27 Mar 2019, Lukasz Majewski wrote:

> * sysdeps/unix/clock_settime (hp_timing_settime):
>   Use __timespec64 in function prototype
> * sysdeps/unix/clock_settime: Rename __clock_settime to __clock_settime64
> * sysdeps/unix/clock_settime: Remove weak_alias for __clock_settime
> * sysdeps/unix/clock_settime: Add clock_settime when __TIMESIZE != 64

Please write an actual textual explanation, however many paragraphs are 
needed, for each patch.  Not just the ChangeLog entry.  There are various 
formatting issues with this ChangeLog entry, but the textual description 
is what's needed first for review.

(You should also, for every patch, give details of how it was tested, on 
at least one 32-bit and at least one 64-bit architecture, with the full 
glibc testsuite without causing any regressions.  If the code being 
changed is generic code not actually used for any glibc configuration, 
please say so.  In the case of this patch, the code is used *only* for 
Hurd, as systems using the Linux kernel use 
sysdeps/unix/sysv/linux/clock_settime.c.  Thus, you should at least 
compile and run the compilation tests for i686-gnu for this patch, using 
build-many-glibcs.py, and report on that verification in the proposed 
commit message.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Joseph Myers]

On Wed, 27 Mar 2019, Lukasz Majewski wrote:

> User space programs on 32 bit machines use struct timespec, which when
> one passes -D_TIME_BITS==64 -D_FILE_OFFSET_BITS==64 during compilation
> is replaced by struct __timespec64, which correctly handles time after
> Y2038.
> Then glibc functions work on struct __timespec64 and provide results to
> user space program.

By comparison, for _FILE_OFFSET_BITS=64 the headers e.g. continue to 
define "struct stat", but with different contents in that case.  So it 
would be possible to do the same with "struct timespec" rather than 
defining it to __timespec64.

There are arguments either way (e.g. defining timespec to __timespec64 
results in different C++ name mangling when _TIME_BITS=64 is used, which 
may be a good idea).  But given the question recently raised about whether 
__time64_t belongs in the installed headers, there needs to be a careful 
justification given for the particular approach chosen, whichever approach 
it ends up being.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Lukasz Majewski]

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

Hi Joseph,

> On Wed, 27 Mar 2019, Lukasz Majewski wrote:
> 
> > User space programs on 32 bit machines use struct timespec, which
> > when one passes -D_TIME_BITS==64 -D_FILE_OFFSET_BITS==64 during
> > compilation is replaced by struct __timespec64, which correctly
> > handles time after Y2038.
> > Then glibc functions work on struct __timespec64 and provide
> > results to user space program.  
> 
> By comparison, for _FILE_OFFSET_BITS=64 the headers e.g. continue to 
> define "struct stat", but with different contents in that case.  So
> it would be possible to do the same with "struct timespec" rather
> than defining it to __timespec64.

Ok. I've looked into the ./bits/stat.h source code:

#ifndef __USE_FILE_OFFSET64
    __ino_t st_ino;             /* File serial number.  */
#else
    __ino64_t st_ino;           /* File serial number.  */
#endif

In this case explicit 64 bit type is used (__ino64_t).

On important information from the above - it is allowed to use
__USE_FILE_OFFSET64 (which corresponds to __USE_TIME_BITS64 for time).

> 
> There are arguments either way (e.g. defining timespec to
> __timespec64 results in different C++ name mangling when
> _TIME_BITS=64 is used, which may be a good idea).  But given the
> question recently raised about whether __time64_t belongs in the
> installed headers, there needs to be a careful justification given
> for the particular approach chosen, whichever approach it ends up
> being.
> 

I will state more details about this problem in the other mail - one
related to Paul's mktime rework (and hiding __time64_t to be only
glibc/gnulib private type).


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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC 0/7] y2038: clock_settime rework to be Y2038 safe on 32bit systems

[Lukasz Majewski]

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

Hi Joseph,

> On Wed, 27 Mar 2019, Lukasz Majewski wrote:
> 
> >   -- The Y2038 support is enabled only when _ALL_
> > syscalls/functions are Y2038 ready  
> 
> Yes, that's correct.  All the header changes to support
> _TIME_BITS=64, new symbol versions and corresponding changes to ABI
> baselines should be made at once.

Ok.

So the work plan is as follows:

1. Rewrite relevant syscalls (for sysdeps/unix/sysv/linux - Linux) to
use

__clock_settime64 () 
{

}

#if TIMESIZE != 64

clock_settime ()
{
	...
	__clock_settime64();
	...
}
#endif

Those shouldn't introduce make xcheck/check regressions.

And can be added syscall by syscall - i.e.
clock_{settime|gettime|nanosleep|getres}

In this step the internal glibc explicit 64bit types shall be
introduced (struct __timespec64, __time64_t, struct __timeval64, etc).


2. When _TIME_BITS=64 patch is introduced:

This shall be done in a single patch (!), which would include:
redirection of functions, manual entry, __USE_TIME_BITS64 definition
etc.

To reduce the overwhelming patch size from point 2 - tests shall be
added in point 1 (so I only then add -D_TIME_BITS=64 switch to be
compiled in).

> 
> > - I've read that clock_* functions were moved from librt to glibc -
> > what was the motivation behind this move?  
> 
> librt depends on libpthread.  Linking against libpthread causes some 
> multithreaded code paths to be used, so slowing down programs even if
> they are single-threaded.  The clock_* functions are commonly usable
> without threads, so it makes sense to have them in libc.
> 
> (Whether we should somehow avoid the slowdown from pthreads functions 
> being present, and have everything in libc, is a separate question.)
> 
> > Why to we have compatibility code in the rt/* for
> > clock_settime() if the _real_ support is in time/* ?	  
> 
> We have to stay compatible with existing programs built with older
> glibc versions that expect to get functions such as clock_settime
> from librt.so.
> 
> When making changes in this area, running the glibc testsuite for a
> range of architectures is helpful - the ABI tests and linknamespace
> tests will catch some possible bugs.  But you should also manually
> inspect the clock_* symbols exported from librt.so to make sure they
> look the same before and after the change (as opposed to any
> GLIBC_PRIVATE __clock_* exports, where changes are OK as long as the
> symbols exported match the ones used in other libraries).
> 
> For example, the ABI tests do not verify whether a given symbol is a 
> compat symbol or not.  If you look at the output of objdump
> --dynamic-syms on librt.so you'll see e.g.
> 
> 0000000000004dc0 g   iD  .text  0000000000000008 (GLIBC_2.2.5)
> clock_gettime
> 
> where the parentheses around the symbol version indicate it's a
> compat symbol - so you should verify that remains the same before and
> after the patch.  The 'i' in 'iD' indicates an IFUNC because that's
> how the redirection from librt to libc versions works, so make sure
> it remains an IFUNC after the changes (on architectures supporting
> IFUNCs; on others, it should remain a wrapper).  And do this
> verification for both 32-bit and 64-bit architectures.
> 

And this explains why this RFC causes following regression:

FAIL: rt/check-abi-librt
	^^^ - I do not export clock_settime anymore for librt

FAIL: time/check-wrapper-headers
	^^^ What are the "wrapper-headers"? Is this a different name
	for "installed" headers in glibc terms ?

Summary of test results:
      9 FAIL
   6048 PASS
     21 UNSUPPORTED
     17 XFAIL
      2 XPASS



If I may ask:

What is the workflow/setup for build-many-glibcs.py ?

Is it 
./src/glibc/scripts/build-many-glibcs.py -j8 . checkout

and then 
./src/glibc/scripts/build-many-glibcs.py -j8 . bot


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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Joseph Myers]

On Thu, 28 Mar 2019, Lukasz Majewski wrote:

> Ok. I've looked into the ./bits/stat.h source code:
> 
> #ifndef __USE_FILE_OFFSET64
>     __ino_t st_ino;             /* File serial number.  */
> #else
>     __ino64_t st_ino;           /* File serial number.  */
> #endif
> 
> In this case explicit 64 bit type is used (__ino64_t).

Yes, consistency with that is an argument for having __time64_t in the 
installed headers (__ino_t and __ino64_t always exist, and their types 
don't depend on _FILE_OFFSET_BITS, but which one is used to define ino_t 
does depend on _FILE_OFFSET_BITS - so consistency with that approach would 
suggest __time64_t in installed headers even if no __timespec64 in 
installed headers, with __time_t continuing to have the same type it has 
at present).

There should not, however, be a public interface time64_t (or 
corresponding non-reserved function names), to avoid excess complexity 
around having explicit 64-bit-time public interfaces for each function as 
well as the _TIME_BITS=64 interfaces.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 0/7] y2038: clock_settime rework to be Y2038 safe on 32bit systems

[Joseph Myers]

On Thu, 28 Mar 2019, Lukasz Majewski wrote:

> FAIL: time/check-wrapper-headers
> 	^^^ What are the "wrapper-headers"? Is this a different name
> 	for "installed" headers in glibc terms ?

Wrapper headers are the headers in include/ that are *not* installed but 
wrap round the headers in other directories that *are* installed, so that 
the glibc build process can find the right header from the source tree 
rather than one that might have been installed by an older glibc version 
and so is found by the compiler by default.  For example, the installed 
<time.h> comes from time/time.h in the glibc source tree; compilations in 
subdirectories other than time/ don't search time/ for headers, so they 
can only find time/time.h, rather than an older header like 
/usr/include/time.h, because of the include/time.h wrapper that includes 
<time/time.h> explicitly (and then adds some internal declarations).

Recent discussions and commits (which it is advisable to follow when 
maintaining any large patch series over an extended period of time, to 
keep that series up to date with such global changes) will show how we 
agreed that all installed headers should have a wrapper, even if they are 
not actually included from any source file outside the directory 
containing the header, and how a test was added to verify this requirement 
(after missing wrappers had been added).  If you see a recently added test 
failing with (or indeed without) your patches, the commit adding that 
test, and surrounding discussions on libc-alpha, are the first place to 
look to understand what is being tested.

> If I may ask:
> 
> What is the workflow/setup for build-many-glibcs.py ?

The commit message for the commit that added the script includes 
instructions, which are still current.  If you only wish to test one 
configuration (e.g. i686-gnu), naming that on the "compilers" and "glibcs" 
commands will save a lot of time.

> and then 
> ./src/glibc/scripts/build-many-glibcs.py -j8 . bot

You are unlikely to want to use it with "bot" unless you actually intend 
to run your own bot that continuously builds and sends test logs to a 
mailing list.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Paul Eggert]

On 3/28/19 9:19 AM, Joseph Myers wrote:
>> In this case explicit 64 bit type is used (__ino64_t).
> Yes, consistency with that is an argument for having __time64_t in the 
> installed headers

But the two cases are not consistent. Glibc exposes the type ino64_t to
user code, because ino_t is part of the file API and Glibc attempts to
support mixed-mode 32-bit programs that use both 32-bit ino_t and 64-bit
ino64_t in the same program.

> There should not, however, be a public interface time64_t
Yes, and that's why the two cases are not consistent. We're planning to
do time_t differently: we are not attempting to support mixed-mode user
code, so there is no need to export either __time64_t or time64_t to
user code.

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Joseph Myers]

On Thu, 28 Mar 2019, Paul Eggert wrote:

> > There should not, however, be a public interface time64_t
> Yes, and that's why the two cases are not consistent. We're planning to
> do time_t differently: we are not attempting to support mixed-mode user
> code, so there is no need to export either __time64_t or time64_t to
> user code.

I'm not convinced that the absence of one set of interfaces (public time64 
functions, time64_t, etc.) is a reason for inconsistency relating to a 
piece of internals that is present in both cases (__time_t, __off_t, 
etc.).  I think having __time_t handled differently from __off_t in that 
regard (by making the definition depend on _TIME_BITS, rather than only 
having the definition of time_t depend on _TIME_BITS) would be liable to 
confuse people reading glibc headers.

It's more plausible that the headers should not define __time_t at all in 
the _TIME_BITS=64 case (if there are no interfaces that would use it).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Paul Eggert]

On 3/28/19 9:39 AM, Joseph Myers wrote:
>
> having __time_t handled differently from __off_t in that 
> regard (by making the definition depend on _TIME_BITS, rather than only 
> having the definition of time_t depend on _TIME_BITS) would be liable to 
> confuse people reading glibc headers.

It depends on what sort of confusion is more likely to cause trouble.
I'm worried about confusion in user code mistakenly using __time64_t -
something that *will* happen, I'm afraid (we've seen that with
__off64_t). The confusion you're worried about would be limited to
people reading glibc source code (i.e., code not installed under
/usr/include) and so it is considerably more limited in scope and can be
addressed by a comment as needed.


> It's more plausible that the headers should not define __time_t at all in 
> the _TIME_BITS=64 case (if there are no interfaces that would use it).
>
Several installed .h files use __time_t now, e.g., <bits/shm.h>. Are you
suggesting that it's plausible to change them to use time_t instead?
Something like this should work, yes. But then when _TIME_BITS=64 public
headers won't need to define __time64_t either, for the same reason they
won't need to define __time_t. I.e., if _TIME_BITS=64 then neither
__time_t nor __time64_t would be defined.

That would be OK for me (as I intend to use _TIME_BITS=64 in all my
apps, and by 2038 _TIME_BITS=64 will be the default). However, I'm still
puzzled as to why glibc should expose __time_t and __time64_t to
_TIME_BITS!=64 user code, given that we don't want to support mixed-mode
user code.

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Joseph Myers]

On Thu, 28 Mar 2019, Paul Eggert wrote:

> > It's more plausible that the headers should not define __time_t at all in 
> > the _TIME_BITS=64 case (if there are no interfaces that would use it).
> >
> Several installed .h files use __time_t now, e.g., <bits/shm.h>. Are you
> suggesting that it's plausible to change them to use time_t instead?

I'm suggesting that such structures will end up having contents that 
depend on _TIME_BITS.  What is the Linux kernel using as the 64-bit time 
version of shmid_ds?  Note that the existing padding around the time 
fields there (where present) does *not* generally correspond to 
endianness, i.e. a separate structure layout is needed.

It may well be the case that these headers can and should use the public 
*_t types more.  Historically POSIX didn't always require the *_t types to 
be defined in all headers which had interfaces using them, but they are 
always reserved in POSIX so it's OK to define them anyway in such headers, 
and POSIX has moved towards generally requiring the types to be defined in 
such cases.

> That would be OK for me (as I intend to use _TIME_BITS=64 in all my
> apps, and by 2038 _TIME_BITS=64 will be the default). However, I'm still
> puzzled as to why glibc should expose __time_t and __time64_t to
> _TIME_BITS!=64 user code, given that we don't want to support mixed-mode
> user code.

I don't think we should be particularly concerned by what __* names are 
visible to user code; the visibility of such names is simply a side-effect 
of how the C language works.  They should be present, or not, however is 
convenient for implementing and maintaining the installed headers (which 
includes keeping those headers consistent between different *_t typedefs 
in most cases).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Paul Eggert]

On 3/28/19 10:25 AM, Joseph Myers wrote:
>
> I don't think we should be particularly concerned by what __* names are 
> visible to user code; the visibility of such names is simply a side-effect 
> of how the C language works.

Hmm, well, our opinions differ, as I've seen too many people abusing the
C-language rules and would rather avoid these problems when it's easy,
as it is here.

However, as long as __time64_t and __time_t are not visible to user code
when _TIME_BITS=64, this will address most of my own practical concerns
and should be OK as a compromise.

> They should be present, or not, however is 
> convenient for implementing and maintaining the installed headers (which 
> includes keeping those headers consistent between different *_t typedefs 
> in most cases).
My impression is that it will be more convenient to put __time64_t in a
private header, to help remind maintainers that that the situation with
time_t is different, as there is no intent to support mixed-mode user
code with time_t. But I understand if your opinion differs.

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Lukasz Majewski]

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

Hi Paul, Joseph,

Thank you for your feedback.

> On 3/28/19 10:25 AM, Joseph Myers wrote:
> >
> > I don't think we should be particularly concerned by what __* names
> > are visible to user code; the visibility of such names is simply a
> > side-effect of how the C language works.  
> 
> Hmm, well, our opinions differ, as I've seen too many people abusing
> the C-language rules and would rather avoid these problems when it's
> easy, as it is here.
> 
> However, as long as __time64_t and __time_t are not visible to user
> code when _TIME_BITS=64, this will address most of my own practical
> concerns and should be OK as a compromise.

To sum up:
----------

1. __time64_t and __time_t are "exported" (visible in /usr/include/*)
always (similar to __off_t and __off64_t).

2. The "exported" struct timespec would look like:

struct timespec
{
/* Use the original definition for 64-bit arches
   or when 64-bit-time by default has *not* been requested */
#if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
  __time_t tv_sec; /* Seconds. */
#else
  __time64_t tv_sec;
#end
  __syscall_slong_t tv_nsec;
};

The same code would be use with e.g. struct timeval

3. Put the internal struct __timespec64

struct __timespec64
{
  __time64_t tv_sec;
  __int64_t tv_nsec;
}

into include/time.h and don't introduce separate
include/bits/types/struct___timespec64.h 

The same idea would be applied to struct timeval

4. Syscalls conversion - starting with Linux only - performed as
described in: "D.2.1 64-bit time symbol handling in the GNU C Library"
https://www.gnu.org/software/libc/manual/html_mono/libc.html

(As done with clock_settime() in this RFC patch set).

5. Add some glibc tests (which could be also easily adapted to test
after Y2038 scenario) after converting some calls (like e.g.
clock_{settime|gettime|nanosleep|getres,etc}).

> 
> > They should be present, or not, however is 
> > convenient for implementing and maintaining the installed headers
> > (which includes keeping those headers consistent between different
> > *_t typedefs in most cases).  
> My impression is that it will be more convenient to put __time64_t in
> a private header, to help remind maintainers that that the situation
> with time_t is different, as there is no intent to support mixed-mode
> user code with time_t. But I understand if your opinion differs.
> 




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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Paul Eggert]

>> However, as long as __time64_t and __time_t are not visible to user
>> code when _TIME_BITS=64, this will address most of my own practical
>> concerns and should be OK as a compromise.
> 
> To sum up:
> ----------
> 
> 1. __time64_t and __time_t are "exported" (visible in /usr/include/*)
> always (similar to __off_t and __off64_t).

No, the compromise is that these types are "exported" only when _TIME_BITS<64. 
Exporting them when _TIME_BITS==64 is unnecessary and confusing. (It's also 
unnecessary and confusing when _TIME_BITS<64, but I'm willing to compromise 
there because the _TIME_BITS<64 case is obsolescent and will eventually go away.)

> 2. The "exported" struct timespec would look like:
> 
> struct timespec
> {
> /* Use the original definition for 64-bit arches
>     or when 64-bit-time by default has *not* been requested */
> #if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
>    __time_t tv_sec; /* Seconds. */
> #else
>    __time64_t tv_sec;
> #end
>    __syscall_slong_t tv_nsec;
> };

Sorry, I'm not following this. Why can't this be the natural 'struct timespec { 
time_t tv_sec; ...; }'? The two forms are equivalent, regardless of whether 
__WORDSIZE > 32 || ! defined(__USE_TIME_BITS64) and regardless of what we do 
about (1), so why use the more-complicated and more-confusing definition?

As Joseph mentioned, there may need to be ifdeffery around tv_nsec due to the 
issue of endian-dependent padding around tv_nsec. However, there does not need 
to be ifdeffery around tv_sec. Just use time_t there; problem solved.

> 3. Put the internal struct __timespec64 ...
> into include/time.h and don't introduce separate
> include/bits/types/struct___timespec64.h
> 
> The same idea would be applied to struct timeval

Yes.

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Lukasz Majewski]

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

Hi Paul,

> >> However, as long as __time64_t and __time_t are not visible to user
> >> code when _TIME_BITS=64, this will address most of my own practical
> >> concerns and should be OK as a compromise.  
> > 
> > To sum up:
> > ----------
> > 
> > 1. __time64_t and __time_t are "exported" (visible
> > in /usr/include/*) always (similar to __off_t and __off64_t).  
> 
> No, the compromise is that these types are "exported" only when
> _TIME_BITS<64. Exporting them when _TIME_BITS==64 is unnecessary and
> confusing. (It's also unnecessary and confusing when _TIME_BITS<64,
> but I'm willing to compromise there because the _TIME_BITS<64 case is
> obsolescent and will eventually go away.)

Ok. As __time64_t has been "hidden" in your previous mktime patch set,
I suppose that this would be changed in the next version?

> 
> > 2. The "exported" struct timespec would look like:
> > 
> > struct timespec
> > {
> > /* Use the original definition for 64-bit arches
> >     or when 64-bit-time by default has *not* been requested */
> > #if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
> >    __time_t tv_sec; /* Seconds. */
> > #else
> >    __time64_t tv_sec;
> > #end
> >    __syscall_slong_t tv_nsec;
> > };  
> 
> Sorry, I'm not following this. Why can't this be the natural 'struct
> timespec { time_t tv_sec; ...; }'? The two forms are equivalent,
> regardless of whether __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
> and regardless of what we do about (1), so why use the
> more-complicated and more-confusing definition?

Currently there is __time_t tv_sec, so I've followed the pattern.

I don't mind to change it to time_t tv_sec.

> 
> As Joseph mentioned, there may need to be ifdeffery around tv_nsec
> due to the issue of endian-dependent padding around tv_nsec. 

I thought that we may only be concerned about endiannes on the
"internal" (i.e. struct __timespec64) representation, which would be
passed as the argument to Linux syscalls.

> However,
> there does not need to be ifdeffery around tv_sec. Just use time_t
> there; problem solved.

Ok.

> 
> > 3. Put the internal struct __timespec64 ...
> > into include/time.h and don't introduce separate
> > include/bits/types/struct___timespec64.h
> > 
> > The same idea would be applied to struct timeval  
> 
> Yes.


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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Lukasz Majewski]

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

Hi Paul, Joseph,

> >> However, as long as __time64_t and __time_t are not visible to user
> >> code when _TIME_BITS=64, this will address most of my own practical
> >> concerns and should be OK as a compromise.  
> > 
> > To sum up:
> > ----------
> > 
> > 1. __time64_t and __time_t are "exported" (visible
> > in /usr/include/*) always (similar to __off_t and __off64_t).  
> 
> No, the compromise is that these types are "exported" only when
> _TIME_BITS<64. Exporting them when _TIME_BITS==64 is unnecessary and
> confusing. (It's also unnecessary and confusing when _TIME_BITS<64,
> but I'm willing to compromise there because the _TIME_BITS<64 case is
> obsolescent and will eventually go away.)
> 
> > 2. The "exported" struct timespec would look like:
> > 
> > struct timespec
> > {
> > /* Use the original definition for 64-bit arches
> >     or when 64-bit-time by default has *not* been requested */
> > #if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
> >    __time_t tv_sec; /* Seconds. */
> > #else
> >    __time64_t tv_sec;
> > #end
> >    __syscall_slong_t tv_nsec;
> > };  
> 
> Sorry, I'm not following this. Why can't this be the natural 'struct
> timespec { time_t tv_sec; ...; }'? The two forms are equivalent,
> regardless of whether __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
> and regardless of what we do about (1), so why use the
> more-complicated and more-confusing definition?

Ok, time_t shall be used for tv_sec.

> 
> As Joseph mentioned, there may need to be ifdeffery around tv_nsec
> due to the issue of endian-dependent padding around tv_nsec.

It is not only the padding. 
The problem is with the size of this struct passed to the kernel as
syscall.

1. __TIMESIZE = 64 -> No problem with kernel ABI
	- sizeof(tv_sec) = 64 bit
	- sizeof(tv_nsec) = 64 bit

	Kernel looks for tv_sec and tv_nsec, each 64 bit

2. __TIMESIZE = 32 (no, Y2038 support) ->  No problem with kernel ABI
	- sizeof(tv_sec) = 32 bit
	- sizeof(tv_nsec) = 32 bit

	Kernel looks for tv_sec and tv_nsec, each 32 bit (those
	syscalls would be removed from the kernel)

3. __TIMESIZE = 32 (Y2038 support) -> ABI problem 
	- sizeof(tv_sec) = 64 bit (time_t)
	- sizeof(tv_nsec) = 32 bit
	- needed padding (tv_pad) 32 bit

	Kernel expects tv_sec and tv_nsec to be both 64 bits.

Exported (installed at /usr/include) struct timespec (to work in above
three cases):

struct timespec
 {
   time_t tv_sec;   /* Seconds.  */
#ifdef __USE_TIME_BITS64
# if BYTE_ORDER == BIG_ENDIAN
   int tv_pad: 32;  /* Padding named for checking/setting */
   __syscall_slong_t tv_nsec; /* Nanoseconds */ 
# else
   __syscall_slong_t tv_nsec; /* Nanoseconds */
   int tv_pad: 32;  /* Padding named for checking/setting */ 
# endif
#else
   __syscall_slong_t tv_nsec;	/* Nanoseconds.  */
#endif
 };


and internal (in ./include/time.h) definition of struct __timespec64:

#if __TIMESIZE == 64
# define __timespec64 timespec
#else
struct __timespec64
{
	__time64_t tv_sec;         /* Seconds */
	__time64_t tv_nsec;         /* Nanoseconds */
};
#endif



> However,
> there does not need to be ifdeffery around tv_sec. Just use time_t
> there; problem solved.
> 
> > 3. Put the internal struct __timespec64 ...
> > into include/time.h and don't introduce separate
> > include/bits/types/struct___timespec64.h
> > 
> > The same idea would be applied to struct timeval  
> 
> Yes.




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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Lukasz Majewski]

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

On Wed, 10 Apr 2019 15:05:27 +0200
Lukasz Majewski <lukma@denx.de> wrote:

> Hi Paul, Joseph,
> 
> > >> However, as long as __time64_t and __time_t are not visible to
> > >> user code when _TIME_BITS=64, this will address most of my own
> > >> practical concerns and should be OK as a compromise.    
> > > 
> > > To sum up:
> > > ----------
> > > 
> > > 1. __time64_t and __time_t are "exported" (visible
> > > in /usr/include/*) always (similar to __off_t and __off64_t).    
> > 
> > No, the compromise is that these types are "exported" only when
> > _TIME_BITS<64. Exporting them when _TIME_BITS==64 is unnecessary and
> > confusing. (It's also unnecessary and confusing when _TIME_BITS<64,
> > but I'm willing to compromise there because the _TIME_BITS<64 case
> > is obsolescent and will eventually go away.)
> >   
> > > 2. The "exported" struct timespec would look like:
> > > 
> > > struct timespec
> > > {
> > > /* Use the original definition for 64-bit arches
> > >     or when 64-bit-time by default has *not* been requested */
> > > #if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
> > >    __time_t tv_sec; /* Seconds. */
> > > #else
> > >    __time64_t tv_sec;
> > > #end
> > >    __syscall_slong_t tv_nsec;
> > > };    
> > 
> > Sorry, I'm not following this. Why can't this be the natural 'struct
> > timespec { time_t tv_sec; ...; }'? The two forms are equivalent,
> > regardless of whether __WORDSIZE > 32 || !
> > defined(__USE_TIME_BITS64) and regardless of what we do about (1),
> > so why use the more-complicated and more-confusing definition?  
> 
> Ok, time_t shall be used for tv_sec.
> 
> > 
> > As Joseph mentioned, there may need to be ifdeffery around tv_nsec
> > due to the issue of endian-dependent padding around tv_nsec.  
> 
> It is not only the padding. 
> The problem is with the size of this struct passed to the kernel as
> syscall.
> 
> 1. __TIMESIZE = 64 -> No problem with kernel ABI
> 	- sizeof(tv_sec) = 64 bit
> 	- sizeof(tv_nsec) = 64 bit
> 
> 	Kernel looks for tv_sec and tv_nsec, each 64 bit
> 
> 2. __TIMESIZE = 32 (no, Y2038 support) ->  No problem with kernel ABI
> 	- sizeof(tv_sec) = 32 bit
> 	- sizeof(tv_nsec) = 32 bit
> 
> 	Kernel looks for tv_sec and tv_nsec, each 32 bit (those
> 	syscalls would be removed from the kernel)
> 
> 3. __TIMESIZE = 32 (Y2038 support) -> ABI problem 
> 	- sizeof(tv_sec) = 64 bit (time_t)
> 	- sizeof(tv_nsec) = 32 bit
> 	- needed padding (tv_pad) 32 bit
> 
> 	Kernel expects tv_sec and tv_nsec to be both 64 bits.
> 
> Exported (installed at /usr/include) struct timespec (to work in above
> three cases):
> 
> struct timespec
>  {
>    time_t tv_sec;   /* Seconds.  */
> #ifdef __USE_TIME_BITS64
> # if BYTE_ORDER == BIG_ENDIAN
>    int tv_pad: 32;  /* Padding named for checking/setting */
>    __syscall_slong_t tv_nsec; /* Nanoseconds */ 
> # else
>    __syscall_slong_t tv_nsec; /* Nanoseconds */
>    int tv_pad: 32;  /* Padding named for checking/setting */ 
> # endif
> #else
>    __syscall_slong_t tv_nsec;	/* Nanoseconds.  */
> #endif
>  };
> 
> 
> and internal (in ./include/time.h) definition of struct __timespec64:
> 
> #if __TIMESIZE == 64
> # define __timespec64 timespec
> #else
> struct __timespec64
> {
> 	__time64_t tv_sec;         /* Seconds */
> 	__time64_t tv_nsec;         /* Nanoseconds */
> };
> #endif
> 

I've tested it a bit more and the internal representation also needs
padding to be safely passed to Linux kernel.

Hence, it would look like:

struct __timespec64
{
	__time64_t tv_sec;         /* Seconds */
# if BYTE_ORDER == BIG_ENDIAN
	int tv_pad: 32;            /* Padding named for
	__int32_t tv_nsec;         /* Nanoseconds */
# else
	__int32_t tv_nsec;         /* Nanoseconds */
	int tv_pad: 32;            /* Padding named for
# endif
};
#endif


> 
> 
> > However,
> > there does not need to be ifdeffery around tv_sec. Just use time_t
> > there; problem solved.
> >   
> > > 3. Put the internal struct __timespec64 ...
> > > into include/time.h and don't introduce separate
> > > include/bits/types/struct___timespec64.h
> > > 
> > > The same idea would be applied to struct timeval    
> > 
> > Yes.  
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma@denx.de




Best regards,

Lukasz Majewski

--

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Joseph Myers]

On Wed, 10 Apr 2019, Lukasz Majewski wrote:

> Exported (installed at /usr/include) struct timespec (to work in above
> three cases):
> 
> struct timespec
>  {
>    time_t tv_sec;   /* Seconds.  */
> #ifdef __USE_TIME_BITS64
> # if BYTE_ORDER == BIG_ENDIAN

BYTE_ORDER and BIG_ENDIAN are in the user's namespace.  You have to use 
implementation-namespace macros in such conditionals in installed headers.

>    int tv_pad: 32;  /* Padding named for checking/setting */

No.  This *must* be unnamed in the installed header, so that initializers 
in user code { seconds, nanoseconds } continue to work as expected (even 
if not formally required to work by the standards, I think it's clear we 
should keep code with such initializers working).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 1/7] y2038: Introduce struct __timespec64

[Lukasz Majewski]

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

Hi Joseph,

> On Wed, 10 Apr 2019, Lukasz Majewski wrote:
> 
> > Exported (installed at /usr/include) struct timespec (to work in
> > above three cases):
> > 
> > struct timespec
> >  {
> >    time_t tv_sec;   /* Seconds.  */
> > #ifdef __USE_TIME_BITS64
> > # if BYTE_ORDER == BIG_ENDIAN  
> 
> BYTE_ORDER and BIG_ENDIAN are in the user's namespace.  You have to
> use implementation-namespace macros in such conditionals in installed
> headers.

This patch has been dropped and superseded by new one:
https://patchwork.ozlabs.org/patch/1085395/

In the above patch the struct __timespec64 is only internal for glibc.
The padding is necessary to avoid passing some random data to kernel
(however, Linux kernel is clearing upper 32 bits of tv_nsec anyway).

The struct __timespec64 is not "installed" anymore - as suggested by
Paul it has been changed as:
https://github.com/lmajewski/y2038_glibc/commit/ea9e29e4d8dbde203325a16962e99e5a6891c741#diff-4ddbc47d3262d4f00f3825e4f3627dbb

In that way we would keep exported/installed struct timespec (as it is
now). 

> 
> >    int tv_pad: 32;  /* Padding named for checking/setting */  
> 
> No.  This *must* be unnamed in the installed header, so that
> initializers in user code { seconds, nanoseconds } continue to work
> as expected (even if not formally required to work by the standards,
> I think it's clear we should keep code with such initializers
> working).
> 




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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]