[v7,0/3] y2038: Linux: Introduce __clock_settime64 function

  On Tue, Sep 17, 2019 at 9:51 AM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Tue, 17 Sep 2019, Lukasz Majewski wrote:
>
> > - New 32 bits glibc ports (like RISC-V 32) will get __TIMESIZE ==
> >   64 (__WORDSIZE == 32) and no need to define the -D_TIME_BITS=64
> >   during the compilation. They will just get 64 bit time API support
> >   from the outset.
>
> Yes, at least if such ports wish to use 64-bit time; I don't think we've
> really discussed if we want to *require* 64-bit time for future ports
> (e.g. the next revised resubmissions of the ARC and NDS32 ports).
> Certainly the work required right now for ARC or NDS32 to use 64-bit time
> would be significantly more than the work for RV32 (because they also
> support older kernel versions without the 64-bit-time syscalls, so all the
> Y2038 work for fallback at runtime to older syscalls becomes relevant),
> unless they decide on 5.1 or later as minimum kernel version.
>
> > - Already supported 32 bits architectures (like armv7-a with __WORDSIZE
> >   == 32) will keep __TIMESIZE == 32 and require -D_TIME_BITS=64 for
> >   compilation.
>
> Yes.
>
> >   After glibc sets the minimal supported kernel version to 5.1 and all
> >   conversions for syscalls to support 64 bit time API are done the
> >   __TIMESIZE will be set to 64 and -D_TIME_BITS=64 will not be required
> >   anymore for compilation.
>
> No.  __TIMESIZE means the size of time_t in the unsuffixed ABIs in glibc,
> not the _TIME_BITS-dependent size of time_t in the current compilation.
> We hope in future to make _TIME_BITS=64 the default and only API supported
> for new compilations (which is independent of what the minimum kernel
> version is), but __TIMESIZE would still be 32, because the unsuffixed ABIs
> would remain compatible with existing binaries using 32-bit time.
>
> > Ok. So then we shall keep the condition:
> >
> > #if __WORDSIZE == 64 \
> >   || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > # define __timespec64 timespec
> > #else
>
> No.  __timespec64 should be defined to timespec whenever __TIMESIZE == 64.
> The timespec to which it is defined, in the public header, would gain
> padding.
>
> The condition
>
> #if __WORDSIZE == 64 \
>   || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
>
> is correct as a condition for struct timespec (in the public header) *not*
> to have padding.

Are you going to incorporate this into your series Lukasz?

I currently have this diff which fixes the build failures for me:


As well as that the timeval struct has the same issue. I'll have to
look into that and see what the solution is there.

Alistair

>
> --
> Joseph S. Myers
> joseph@codesourcery.com
  

On Wed, 18 Sep 2019, Alistair Francis wrote:

> +#include <endian.h>
> 
>  /* POSIX.1b structure for a time value.  This is like a `struct timeval' but
>     has nanoseconds instead of microseconds.  */
>  struct timespec
>  {
>    __time_t tv_sec;             /* Seconds.  */
> +#if __WORDSIZE == 64 \
> +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
>    __syscall_slong_t tv_nsec;   /* Nanoseconds.  */
> +#else
> +# if BYTE_ORDER == BIG_ENDIAN
> +  __int32_t tv_pad;           /* Padding */
> +  __syscall_slong_t tv_nsec;  /* Nanoseconds */
> +# else
> +  __int32_t tv_nsec;          /* Nanoseconds */
> +  __syscall_slong_t tv_pad;   /* Padding */
> +# endif
> +#endif

The padding must be an *unnamed bit-field* so that { tv_sec, tv_nsec } 
initializers (common in practice even if not officially supported by the 
standards) continue to work.  Also, I think you should just use "long int" 
for tv_nsec in the case where there is padding, as the standard-defined 
type (and then the padding can be "int: 32", so avoiding any dependence on 
whether compilers support non-int bit-fields).  Certainly the choice of 
types for tv_nsec and padding should not depend on the endianness (the 
patch above is using __int32_t for the first field and __syscall_slong_t 
for the second, regardless of which is tv_nsec and which is padding).

There are namespace issues when changing installed headers.  You can't use 
macros such as BYTE_ORDER or BIG_ENDIAN because they aren't in the 
standard-reserved namespaces.

Unfortunately the definitions of __LITTLE_ENDIAN and __BIG_ENDIAN are in 
<endian.h> (__BYTE_ORDER is in the architecture-specific <bits/endian.h>), 
and while the non-reserved names therein are all conditional on 
__USE_MISC, I don't think we really want to start exporting them from 
every header that uses struct timespec.  My inclination would be to have a 
separate bits/ header that only defines the __LITTLE_ENDIAN / __BIG_ENDIAN 
/ __PDP_ENDIAN macros (or that defines those and includes the 
architecture-specific header for __BYTE_ORDER), so that other headers can 
test endianness without bringing in all the other __USE_MISC 
endian-related macros from <endian.h>, but Zack might advise on how such 
changes would fit into his header cleanups.
  

Hi Alistair,

> On Tue, Sep 17, 2019 at 9:51 AM Joseph Myers
> <joseph@codesourcery.com> wrote:
> >
> > On Tue, 17 Sep 2019, Lukasz Majewski wrote:
> >  
> > > - New 32 bits glibc ports (like RISC-V 32) will get __TIMESIZE ==
> > >   64 (__WORDSIZE == 32) and no need to define the -D_TIME_BITS=64
> > >   during the compilation. They will just get 64 bit time API
> > > support from the outset.  
> >
> > Yes, at least if such ports wish to use 64-bit time; I don't think
> > we've really discussed if we want to *require* 64-bit time for
> > future ports (e.g. the next revised resubmissions of the ARC and
> > NDS32 ports). Certainly the work required right now for ARC or
> > NDS32 to use 64-bit time would be significantly more than the work
> > for RV32 (because they also support older kernel versions without
> > the 64-bit-time syscalls, so all the Y2038 work for fallback at
> > runtime to older syscalls becomes relevant), unless they decide on
> > 5.1 or later as minimum kernel version. 
> > > - Already supported 32 bits architectures (like armv7-a with
> > > __WORDSIZE == 32) will keep __TIMESIZE == 32 and require
> > > -D_TIME_BITS=64 for compilation.  
> >
> > Yes.
> >  
> > >   After glibc sets the minimal supported kernel version to 5.1
> > > and all conversions for syscalls to support 64 bit time API are
> > > done the __TIMESIZE will be set to 64 and -D_TIME_BITS=64 will
> > > not be required anymore for compilation.  
> >
> > No.  __TIMESIZE means the size of time_t in the unsuffixed ABIs in
> > glibc, not the _TIME_BITS-dependent size of time_t in the current
> > compilation. We hope in future to make _TIME_BITS=64 the default
> > and only API supported for new compilations (which is independent
> > of what the minimum kernel version is), but __TIMESIZE would still
> > be 32, because the unsuffixed ABIs would remain compatible with
> > existing binaries using 32-bit time. 
> > > Ok. So then we shall keep the condition:
> > >
> > > #if __WORDSIZE == 64 \
> > >   || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > > # define __timespec64 timespec
> > > #else  
> >
> > No.  __timespec64 should be defined to timespec whenever __TIMESIZE
> > == 64. The timespec to which it is defined, in the public header,
> > would gain padding.
> >
> > The condition
> >
> > #if __WORDSIZE == 64 \
> >   || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> >
> > is correct as a condition for struct timespec (in the public
> > header) *not* to have padding.  
> 
> Are you going to incorporate this into your series Lukasz?
> 
> I currently have this diff which fixes the build failures for me:
> 
> diff --git a/include/time.h b/include/time.h
> index 7ed3aa61d1d..91f6280eb4d 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -50,8 +50,7 @@ extern void __tzset_parse_tz (const char *tz)
> attribute_hidden;
>  extern void __tz_compute (__time64_t timer, struct tm *tm, int
> use_localtime) __THROW attribute_hidden;
> 
> -#if __WORDSIZE == 64 \
> -  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> +#if __TIMESIZE == 64

I've prepared v8 of __clock_settime64 conversion patches with the above
change:
https://patchwork.ozlabs.org/cover/1164209/

I've tested it with meta-y2038:
https://github.com/lmajewski/meta-y2038

 as well as
../src/scripts/build-many-glibcs.py

It seems to work as expected.	

>  # define __timespec64 timespec
>  #else
>  /* The glibc Y2038-proof struct __timespec64 structure for a time
> value. diff --git a/time/bits/types/struct_timespec.h
> b/time/bits/types/struct_timespec.h
> index 5b77c52b4f0..48405c4f08a 100644
> --- a/time/bits/types/struct_timespec.h
> +++ b/time/bits/types/struct_timespec.h
> @@ -3,13 +3,25 @@
>  #define _STRUCT_TIMESPEC 1
> 
>  #include <bits/types.h>
> +#include <endian.h>
> 
>  /* POSIX.1b structure for a time value.  This is like a `struct
> timeval' but has nanoseconds instead of microseconds.  */
>  struct timespec
>  {
>    __time_t tv_sec;             /* Seconds.  */
> +#if __WORDSIZE == 64 \
> +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
>    __syscall_slong_t tv_nsec;   /* Nanoseconds.  */
> +#else
> +# if BYTE_ORDER == BIG_ENDIAN
> +  __int32_t tv_pad;           /* Padding */
> +  __syscall_slong_t tv_nsec;  /* Nanoseconds */
> +# else
> +  __int32_t tv_nsec;          /* Nanoseconds */
> +  __syscall_slong_t tv_pad;   /* Padding */
> +# endif
> +#endif
>  };

I did not incorporated the above change to v8 of __clock_settime64 as
there are some issues raised by Joseph.

Last but not least - we can get away with the above change as the
implicit padding works for RV32, and ARM32 (which both are LE).

> 
>  #endif
> 
> As well as that the timeval struct has the same issue. I'll have to
> look into that and see what the solution is there.
> 
> Alistair
> 
> >
> > --
> > Joseph S. Myers
> > joseph@codesourcery.com  




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
  

Hi Joseph,

> On Wed, 18 Sep 2019, Alistair Francis wrote:
> 
> > +#include <endian.h>
> > 
> >  /* POSIX.1b structure for a time value.  This is like a `struct
> > timeval' but has nanoseconds instead of microseconds.  */
> >  struct timespec
> >  {
> >    __time_t tv_sec;             /* Seconds.  */
> > +#if __WORDSIZE == 64 \
> > +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> >    __syscall_slong_t tv_nsec;   /* Nanoseconds.  */
> > +#else
> > +# if BYTE_ORDER == BIG_ENDIAN
> > +  __int32_t tv_pad;           /* Padding */
> > +  __syscall_slong_t tv_nsec;  /* Nanoseconds */
> > +# else
> > +  __int32_t tv_nsec;          /* Nanoseconds */
> > +  __syscall_slong_t tv_pad;   /* Padding */
> > +# endif
> > +#endif  
> 

Just one more question - am I correct that the above code will increase
the overall sizeof(struct timespec) to 12 for machines with __WORDSIZE
== 32 ?

I guess that now such machine have sizeof(struct timespec) = 8 ?

However, this shouldn't be a problem (unless some user space SW uses
this data in an unusual way...).

> The padding must be an *unnamed bit-field* so that { tv_sec, tv_nsec
> } initializers (common in practice even if not officially supported
> by the standards) continue to work.  Also, I think you should just
> use "long int" for tv_nsec in the case where there is padding, as the
> standard-defined type (and then the padding can be "int: 32", so
> avoiding any dependence on whether compilers support non-int
> bit-fields).  Certainly the choice of types for tv_nsec and padding
> should not depend on the endianness (the patch above is using
> __int32_t for the first field and __syscall_slong_t for the second,
> regardless of which is tv_nsec and which is padding).
> 
> There are namespace issues when changing installed headers.  You
> can't use macros such as BYTE_ORDER or BIG_ENDIAN because they aren't
> in the standard-reserved namespaces.
> 
> Unfortunately the definitions of __LITTLE_ENDIAN and __BIG_ENDIAN are
> in <endian.h> (__BYTE_ORDER is in the architecture-specific
> <bits/endian.h>), and while the non-reserved names therein are all
> conditional on __USE_MISC, I don't think we really want to start
> exporting them from every header that uses struct timespec.  My
> inclination would be to have a separate bits/ header that only
> defines the __LITTLE_ENDIAN / __BIG_ENDIAN / __PDP_ENDIAN macros (or
> that defines those and includes the architecture-specific header for
> __BYTE_ORDER), so that other headers can test endianness without
> bringing in all the other __USE_MISC endian-related macros from
> <endian.h>, but Zack might advise on how such changes would fit into
> his header cleanups.
> 




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
  

On Wed, 18 Sep 2019, Lukasz Majewski wrote:

> Just one more question - am I correct that the above code will increase
> the overall sizeof(struct timespec) to 12 for machines with __WORDSIZE
> == 32 ?

It looks like that's a bug in the patch, indeed.  (The padding should 
*only* be added when __TIMESIZE == 64 in addition to the other conditions 
given, not when __TIMESIZE == 32.)
  

On Wed, Sep 18, 2019 at 2:28 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Alistair,
>
> > On Tue, Sep 17, 2019 at 9:51 AM Joseph Myers
> > <joseph@codesourcery.com> wrote:
> > >
> > > On Tue, 17 Sep 2019, Lukasz Majewski wrote:
> > >
> > > > - New 32 bits glibc ports (like RISC-V 32) will get __TIMESIZE ==
> > > >   64 (__WORDSIZE == 32) and no need to define the -D_TIME_BITS=64
> > > >   during the compilation. They will just get 64 bit time API
> > > > support from the outset.
> > >
> > > Yes, at least if such ports wish to use 64-bit time; I don't think
> > > we've really discussed if we want to *require* 64-bit time for
> > > future ports (e.g. the next revised resubmissions of the ARC and
> > > NDS32 ports). Certainly the work required right now for ARC or
> > > NDS32 to use 64-bit time would be significantly more than the work
> > > for RV32 (because they also support older kernel versions without
> > > the 64-bit-time syscalls, so all the Y2038 work for fallback at
> > > runtime to older syscalls becomes relevant), unless they decide on
> > > 5.1 or later as minimum kernel version.
> > > > - Already supported 32 bits architectures (like armv7-a with
> > > > __WORDSIZE == 32) will keep __TIMESIZE == 32 and require
> > > > -D_TIME_BITS=64 for compilation.
> > >
> > > Yes.
> > >
> > > >   After glibc sets the minimal supported kernel version to 5.1
> > > > and all conversions for syscalls to support 64 bit time API are
> > > > done the __TIMESIZE will be set to 64 and -D_TIME_BITS=64 will
> > > > not be required anymore for compilation.
> > >
> > > No.  __TIMESIZE means the size of time_t in the unsuffixed ABIs in
> > > glibc, not the _TIME_BITS-dependent size of time_t in the current
> > > compilation. We hope in future to make _TIME_BITS=64 the default
> > > and only API supported for new compilations (which is independent
> > > of what the minimum kernel version is), but __TIMESIZE would still
> > > be 32, because the unsuffixed ABIs would remain compatible with
> > > existing binaries using 32-bit time.
> > > > Ok. So then we shall keep the condition:
> > > >
> > > > #if __WORDSIZE == 64 \
> > > >   || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > > > # define __timespec64 timespec
> > > > #else
> > >
> > > No.  __timespec64 should be defined to timespec whenever __TIMESIZE
> > > == 64. The timespec to which it is defined, in the public header,
> > > would gain padding.
> > >
> > > The condition
> > >
> > > #if __WORDSIZE == 64 \
> > >   || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > >
> > > is correct as a condition for struct timespec (in the public
> > > header) *not* to have padding.
> >
> > Are you going to incorporate this into your series Lukasz?
> >
> > I currently have this diff which fixes the build failures for me:
> >
> > diff --git a/include/time.h b/include/time.h
> > index 7ed3aa61d1d..91f6280eb4d 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -50,8 +50,7 @@ extern void __tzset_parse_tz (const char *tz)
> > attribute_hidden;
> >  extern void __tz_compute (__time64_t timer, struct tm *tm, int
> > use_localtime) __THROW attribute_hidden;
> >
> > -#if __WORDSIZE == 64 \
> > -  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > +#if __TIMESIZE == 64
>
> I've prepared v8 of __clock_settime64 conversion patches with the above
> change:
> https://patchwork.ozlabs.org/cover/1164209/
>
> I've tested it with meta-y2038:
> https://github.com/lmajewski/meta-y2038
>
>  as well as
> ../src/scripts/build-many-glibcs.py
>
> It seems to work as expected.
>
> >  # define __timespec64 timespec
> >  #else
> >  /* The glibc Y2038-proof struct __timespec64 structure for a time
> > value. diff --git a/time/bits/types/struct_timespec.h
> > b/time/bits/types/struct_timespec.h
> > index 5b77c52b4f0..48405c4f08a 100644
> > --- a/time/bits/types/struct_timespec.h
> > +++ b/time/bits/types/struct_timespec.h
> > @@ -3,13 +3,25 @@
> >  #define _STRUCT_TIMESPEC 1
> >
> >  #include <bits/types.h>
> > +#include <endian.h>
> >
> >  /* POSIX.1b structure for a time value.  This is like a `struct
> > timeval' but has nanoseconds instead of microseconds.  */
> >  struct timespec
> >  {
> >    __time_t tv_sec;             /* Seconds.  */
> > +#if __WORDSIZE == 64 \
> > +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> >    __syscall_slong_t tv_nsec;   /* Nanoseconds.  */
> > +#else
> > +# if BYTE_ORDER == BIG_ENDIAN
> > +  __int32_t tv_pad;           /* Padding */
> > +  __syscall_slong_t tv_nsec;  /* Nanoseconds */
> > +# else
> > +  __int32_t tv_nsec;          /* Nanoseconds */
> > +  __syscall_slong_t tv_pad;   /* Padding */
> > +# endif
> > +#endif
> >  };
>
> I did not incorporated the above change to v8 of __clock_settime64 as
> there are some issues raised by Joseph.

That's fine, I can fix up his comments and include that in my series.

>
> Last but not least - we can get away with the above change as the
> implicit padding works for RV32, and ARM32 (which both are LE).

RV32 is actually both BE and LE. The spec allows it to be either. At
the moment there are only LE implementations, but we should try to
handle both.

Alistair

>
> >
> >  #endif
> >
> > As well as that the timeval struct has the same issue. I'll have to
> > look into that and see what the solution is there.
> >
> > Alistair
> >
> > >
> > > --
> > > Joseph S. Myers
> > > joseph@codesourcery.com
>
>
>
>
> 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
  

Hi Alistair,

> On Wed, Sep 18, 2019 at 2:28 PM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > Hi Alistair,
> >  
> > > On Tue, Sep 17, 2019 at 9:51 AM Joseph Myers
> > > <joseph@codesourcery.com> wrote:  
> > > >
> > > > On Tue, 17 Sep 2019, Lukasz Majewski wrote:
> > > >  
> > > > > - New 32 bits glibc ports (like RISC-V 32) will get
> > > > > __TIMESIZE == 64 (__WORDSIZE == 32) and no need to define the
> > > > > -D_TIME_BITS=64 during the compilation. They will just get 64
> > > > > bit time API support from the outset.  
> > > >
> > > > Yes, at least if such ports wish to use 64-bit time; I don't
> > > > think we've really discussed if we want to *require* 64-bit
> > > > time for future ports (e.g. the next revised resubmissions of
> > > > the ARC and NDS32 ports). Certainly the work required right now
> > > > for ARC or NDS32 to use 64-bit time would be significantly more
> > > > than the work for RV32 (because they also support older kernel
> > > > versions without the 64-bit-time syscalls, so all the Y2038
> > > > work for fallback at runtime to older syscalls becomes
> > > > relevant), unless they decide on 5.1 or later as minimum kernel
> > > > version.  
> > > > > - Already supported 32 bits architectures (like armv7-a with
> > > > > __WORDSIZE == 32) will keep __TIMESIZE == 32 and require
> > > > > -D_TIME_BITS=64 for compilation.  
> > > >
> > > > Yes.
> > > >  
> > > > >   After glibc sets the minimal supported kernel version to 5.1
> > > > > and all conversions for syscalls to support 64 bit time API
> > > > > are done the __TIMESIZE will be set to 64 and -D_TIME_BITS=64
> > > > > will not be required anymore for compilation.  
> > > >
> > > > No.  __TIMESIZE means the size of time_t in the unsuffixed ABIs
> > > > in glibc, not the _TIME_BITS-dependent size of time_t in the
> > > > current compilation. We hope in future to make _TIME_BITS=64
> > > > the default and only API supported for new compilations (which
> > > > is independent of what the minimum kernel version is), but
> > > > __TIMESIZE would still be 32, because the unsuffixed ABIs would
> > > > remain compatible with existing binaries using 32-bit time.  
> > > > > Ok. So then we shall keep the condition:
> > > > >
> > > > > #if __WORDSIZE == 64 \
> > > > >   || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > > > > # define __timespec64 timespec
> > > > > #else  
> > > >
> > > > No.  __timespec64 should be defined to timespec whenever
> > > > __TIMESIZE == 64. The timespec to which it is defined, in the
> > > > public header, would gain padding.
> > > >
> > > > The condition
> > > >
> > > > #if __WORDSIZE == 64 \
> > > >   || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > > >
> > > > is correct as a condition for struct timespec (in the public
> > > > header) *not* to have padding.  
> > >
> > > Are you going to incorporate this into your series Lukasz?
> > >
> > > I currently have this diff which fixes the build failures for me:
> > >
> > > diff --git a/include/time.h b/include/time.h
> > > index 7ed3aa61d1d..91f6280eb4d 100644
> > > --- a/include/time.h
> > > +++ b/include/time.h
> > > @@ -50,8 +50,7 @@ extern void __tzset_parse_tz (const char *tz)
> > > attribute_hidden;
> > >  extern void __tz_compute (__time64_t timer, struct tm *tm, int
> > > use_localtime) __THROW attribute_hidden;
> > >
> > > -#if __WORDSIZE == 64 \
> > > -  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > > +#if __TIMESIZE == 64  
> >
> > I've prepared v8 of __clock_settime64 conversion patches with the
> > above change:
> > https://patchwork.ozlabs.org/cover/1164209/
> >
> > I've tested it with meta-y2038:
> > https://github.com/lmajewski/meta-y2038
> >
> >  as well as
> > ../src/scripts/build-many-glibcs.py
> >
> > It seems to work as expected.
> >  
> > >  # define __timespec64 timespec
> > >  #else
> > >  /* The glibc Y2038-proof struct __timespec64 structure for a time
> > > value. diff --git a/time/bits/types/struct_timespec.h
> > > b/time/bits/types/struct_timespec.h
> > > index 5b77c52b4f0..48405c4f08a 100644
> > > --- a/time/bits/types/struct_timespec.h
> > > +++ b/time/bits/types/struct_timespec.h
> > > @@ -3,13 +3,25 @@
> > >  #define _STRUCT_TIMESPEC 1
> > >
> > >  #include <bits/types.h>
> > > +#include <endian.h>
> > >
> > >  /* POSIX.1b structure for a time value.  This is like a `struct
> > > timeval' but has nanoseconds instead of microseconds.  */
> > >  struct timespec
> > >  {
> > >    __time_t tv_sec;             /* Seconds.  */
> > > +#if __WORDSIZE == 64 \
> > > +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > >    __syscall_slong_t tv_nsec;   /* Nanoseconds.  */
> > > +#else
> > > +# if BYTE_ORDER == BIG_ENDIAN
> > > +  __int32_t tv_pad;           /* Padding */
> > > +  __syscall_slong_t tv_nsec;  /* Nanoseconds */
> > > +# else
> > > +  __int32_t tv_nsec;          /* Nanoseconds */
> > > +  __syscall_slong_t tv_pad;   /* Padding */
> > > +# endif
> > > +#endif
> > >  };  
> >
> > I did not incorporated the above change to v8 of __clock_settime64
> > as there are some issues raised by Joseph.  
> 
> That's fine, I can fix up his comments and include that in my series.
> 
> >
> > Last but not least - we can get away with the above change as the
> > implicit padding works for RV32, and ARM32 (which both are LE).  
> 
> RV32 is actually both BE and LE. The spec allows it to be either. 

Ok. I was not aware of this - and blindly assumed that it is LE.

> At
> the moment there are only LE implementations, but we should try to
> handle both.

Ok. Then if you don't mind, please add the above change to your series.

> 
> Alistair
> 
> >  
> > >
> > >  #endif
> > >
> > > As well as that the timeval struct has the same issue. I'll have
> > > to look into that and see what the solution is there.
> > >
> > > Alistair
> > >  
> > > >
> > > > --
> > > > Joseph S. Myers
> > > > joseph@codesourcery.com  
> >
> >
> >
> >
> > 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
  

On Wed, Sep 18, 2019 at 10:25 AM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Wed, 18 Sep 2019, Alistair Francis wrote:
>
> > +#include <endian.h>
> >
> >  /* POSIX.1b structure for a time value.  This is like a `struct timeval' but
> >     has nanoseconds instead of microseconds.  */
> >  struct timespec
> >  {
> >    __time_t tv_sec;             /* Seconds.  */
> > +#if __WORDSIZE == 64 \
> > +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> >    __syscall_slong_t tv_nsec;   /* Nanoseconds.  */
> > +#else
> > +# if BYTE_ORDER == BIG_ENDIAN
> > +  __int32_t tv_pad;           /* Padding */
> > +  __syscall_slong_t tv_nsec;  /* Nanoseconds */
> > +# else
> > +  __int32_t tv_nsec;          /* Nanoseconds */
> > +  __syscall_slong_t tv_pad;   /* Padding */
> > +# endif
> > +#endif
>
> The padding must be an *unnamed bit-field* so that { tv_sec, tv_nsec }
> initializers (common in practice even if not officially supported by the
> standards) continue to work.  Also, I think you should just use "long int"
> for tv_nsec in the case where there is padding, as the standard-defined
> type (and then the padding can be "int: 32", so avoiding any dependence on
> whether compilers support non-int bit-fields).  Certainly the choice of
> types for tv_nsec and padding should not depend on the endianness (the
> patch above is using __int32_t for the first field and __syscall_slong_t
> for the second, regardless of which is tv_nsec and which is padding).

Ok, I have fixed this up.

>
> There are namespace issues when changing installed headers.  You can't use
> macros such as BYTE_ORDER or BIG_ENDIAN because they aren't in the
> standard-reserved namespaces.
>
> Unfortunately the definitions of __LITTLE_ENDIAN and __BIG_ENDIAN are in
> <endian.h> (__BYTE_ORDER is in the architecture-specific <bits/endian.h>),
> and while the non-reserved names therein are all conditional on
> __USE_MISC, I don't think we really want to start exporting them from
> every header that uses struct timespec.  My inclination would be to have a
> separate bits/ header that only defines the __LITTLE_ENDIAN / __BIG_ENDIAN
> / __PDP_ENDIAN macros (or that defines those and includes the
> architecture-specific header for __BYTE_ORDER), so that other headers can
> test endianness without bringing in all the other __USE_MISC
> endian-related macros from <endian.h>, but Zack might advise on how such
> changes would fit into his header cleanups.

I think I understand what you mean, but it seems strange. I'm going to
send an RFC patch and we can discuss there.

Alistair

>
> --
> Joseph S. Myers
> joseph@codesourcery.com