[RFC,v6,04/23] linux: Use 32-bit time_t for itimerval

  The Linux kernel expects itimerval to use a 32-bit time_t, even on archs
with a 64-bit time_t (like RV32). To address this let's convert
itimerval to/from 32-bit and 64-bit to ensure the kernel always gets
a 32-bit time_t.

This means that all 32-bit architectures with a 64-bit time_t will be
able to use this generic implementation.

This code is bsaed on similar code in alpha, but adjusted to pass the
32-bit time_t to the kernel.

We can't directly call the __getitimer/__setitimer functions as they
expect a struct itimerval but we need to pass in a struct itimerval32.
---
 .../linux/generic/wordsize-32/getitimer.c     | 42 +++++++++++++++
 .../linux/generic/wordsize-32/setitimer.c     | 53 +++++++++++++++++++
 .../linux/generic/wordsize-32/tv32-compat.h   | 35 ++++++++++++
 3 files changed, 130 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
  

Hi Alistair,

> The Linux kernel expects itimerval to use a 32-bit time_t, even on
> archs with a 64-bit time_t (like RV32). To address this let's convert
> itimerval to/from 32-bit and 64-bit to ensure the kernel always gets
> a 32-bit time_t.
> 
> This means that all 32-bit architectures with a 64-bit time_t will be
> able to use this generic implementation.
> 
> This code is bsaed on similar code in alpha, but adjusted to pass the
> 32-bit time_t to the kernel.
> 
> We can't directly call the __getitimer/__setitimer functions as they
> expect a struct itimerval but we need to pass in a struct itimerval32.
> ---
>  .../linux/generic/wordsize-32/getitimer.c     | 42 +++++++++++++++
>  .../linux/generic/wordsize-32/setitimer.c     | 53
> +++++++++++++++++++ .../linux/generic/wordsize-32/tv32-compat.h   |
> 35 ++++++++++++ 3 files changed, 130 insertions(+)
>  create mode 100644
> sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c create mode
> 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c create
> mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> 
> diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c
> b/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c new file
> mode 100644 index 0000000000..b279acbc2e
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c
> @@ -0,0 +1,42 @@
> +/* getitimer -- Get the state of an interval timer.  Linux/RV32/tv32
> version.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be
> useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/time.h>
> +#include <sysdep.h>
> +#include <tv32-compat.h>
> +
> +int
> +__getitimer (__itimer_which_t which, struct itimerval *curr_value)
> +{
> +#if __TIMESIZE == 64
> +  struct itimerval32 curr_value_32;
> +  if (INLINE_SYSCALL_CALL (getitimer, which, &curr_value_32) == -1)
> +    return -1;
> +
> +  /* Write all fields of 'curr_value' regardless of overflow.  */
> +  curr_value->it_interval
> +    = valid_timeval_long_to_timeval64 (curr_value_32.it_interval);
> +  curr_value->it_value
> +    = valid_timeval_long_to_timeval64 (curr_value_32.it_value);
> +  return 0;
> +#else
> +  return INLINE_SYSCALL_CALL (getitimer, which, curr_value);
> +#endif
> +}
> +
> +weak_alias (__getitimer, getitimer)
> diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
> b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c new file
> mode 100644 index 0000000000..807683ccae
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
> @@ -0,0 +1,53 @@
> +/* getitimer -- Get the state of an interval timer.  Linux/RV32/tv32
> version.

Please update the header info. I suppose you refer to setitimer here.

> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be
> useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/time.h>
> +#include <sysdep.h>
> +#include <tv32-compat.h>
> +
> +int
> +__setitimer (__itimer_which_t which,
> +             const struct itimerval *restrict new_value,
> +             struct itimerval *restrict old_value)
> +{
> +#if __TIMESIZE == 64

I think that Y2038 safe systems with __WORDSIZE == 32 && __TIMESIZE ==
32 (for now?) also could reuse this work.

To allow it this syscall wrapper shall be converted (written) in the
same way as e.g:

./sysdeps/unix/sysv/linux/clock_settime.c

(the same issue is with getitimer).

Then only redirection when __USE_TIME_BITS64 is set are needed to use
__setitimer64/__getitimer64 on Y2038 safe systems.

> +  struct itimerval32 new_value_32;
> +  new_value_32.it_interval
> +    = valid_timeval64_to_timeval_long (new_value->it_interval);
> +  new_value_32.it_value
> +    = valid_timeval64_to_timeval_long (new_value->it_value);
> +
> +  if (old_value == NULL)
> +    return INLINE_SYSCALL_CALL (setitimer, which, &new_value_32,
> NULL); +
> +  struct itimerval32 old_value_32;
> +  if (INLINE_SYSCALL_CALL (setitimer, which, &new_value_32,
> &old_value_32) == -1)
> +    return -1;
> +
> +  /* Write all fields of 'old_value' regardless of overflow.  */
> +  old_value->it_interval
> +     = valid_timeval_long_to_timeval64 (old_value_32.it_interval);
> +  old_value->it_value
> +     = valid_timeval_long_to_timeval64 (old_value_32.it_value);
> +  return 0;
> +#else
> +  return INLINE_SYSCALL_CALL (setitimer, which, new_value,
> old_value); +#endif
> +}
> +
> +weak_alias (__setitimer, setitimer)
> diff --git
> a/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> b/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h new file
> mode 100644 index 0000000000..aa960a6632 --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> @@ -0,0 +1,35 @@
> +/* Compatibility definitions for `struct timeval' with 32-bit time_t.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be
> useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _TV32_COMPAT_H
> +#define _TV32_COMPAT_H 1
> +
> +#include <features.h>
> +
> +#include <bits/types.h>
> +#include <bits/types/time_t.h>
> +#include <bits/types/struct_timeval.h>
> +
> +/* Structures containing 'struct timeval' with 32-bit time_t.  */
> +struct itimerval32
> +{
> +  struct __timeval_long it_interval;
> +  struct __timeval_long it_value;
> +};
> +
> +#endif /* tv32-compat.h */


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 Tue, Jan 14, 2020 at 8:58 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Alistair,
>
> > The Linux kernel expects itimerval to use a 32-bit time_t, even on
> > archs with a 64-bit time_t (like RV32). To address this let's convert
> > itimerval to/from 32-bit and 64-bit to ensure the kernel always gets
> > a 32-bit time_t.
> >
> > This means that all 32-bit architectures with a 64-bit time_t will be
> > able to use this generic implementation.
> >
> > This code is bsaed on similar code in alpha, but adjusted to pass the
> > 32-bit time_t to the kernel.
> >
> > We can't directly call the __getitimer/__setitimer functions as they
> > expect a struct itimerval but we need to pass in a struct itimerval32.
> > ---
> >  .../linux/generic/wordsize-32/getitimer.c     | 42 +++++++++++++++
> >  .../linux/generic/wordsize-32/setitimer.c     | 53
> > +++++++++++++++++++ .../linux/generic/wordsize-32/tv32-compat.h   |
> > 35 ++++++++++++ 3 files changed, 130 insertions(+)
> >  create mode 100644
> > sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c create mode
> > 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c create
> > mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> >
> > diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c
> > b/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c new file
> > mode 100644 index 0000000000..b279acbc2e
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c
> > @@ -0,0 +1,42 @@
> > +/* getitimer -- Get the state of an interval timer.  Linux/RV32/tv32
> > version.
> > +   Copyright (C) 2020 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be
> > useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <http://www.gnu.org/licenses/>.  */
> > +
> > +#include <sys/time.h>
> > +#include <sysdep.h>
> > +#include <tv32-compat.h>
> > +
> > +int
> > +__getitimer (__itimer_which_t which, struct itimerval *curr_value)
> > +{
> > +#if __TIMESIZE == 64
> > +  struct itimerval32 curr_value_32;
> > +  if (INLINE_SYSCALL_CALL (getitimer, which, &curr_value_32) == -1)
> > +    return -1;
> > +
> > +  /* Write all fields of 'curr_value' regardless of overflow.  */
> > +  curr_value->it_interval
> > +    = valid_timeval_long_to_timeval64 (curr_value_32.it_interval);
> > +  curr_value->it_value
> > +    = valid_timeval_long_to_timeval64 (curr_value_32.it_value);
> > +  return 0;
> > +#else
> > +  return INLINE_SYSCALL_CALL (getitimer, which, curr_value);
> > +#endif
> > +}
> > +
> > +weak_alias (__getitimer, getitimer)
> > diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
> > b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c new file
> > mode 100644 index 0000000000..807683ccae
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
> > @@ -0,0 +1,53 @@
> > +/* getitimer -- Get the state of an interval timer.  Linux/RV32/tv32
> > version.
>
> Please update the header info. I suppose you refer to setitimer here.

Fixed!

>
> > +   Copyright (C) 2020 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be
> > useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <http://www.gnu.org/licenses/>.  */
> > +
> > +#include <sys/time.h>
> > +#include <sysdep.h>
> > +#include <tv32-compat.h>
> > +
> > +int
> > +__setitimer (__itimer_which_t which,
> > +             const struct itimerval *restrict new_value,
> > +             struct itimerval *restrict old_value)
> > +{
> > +#if __TIMESIZE == 64
>
> I think that Y2038 safe systems with __WORDSIZE == 32 && __TIMESIZE ==
> 32 (for now?) also could reuse this work.
>
> To allow it this syscall wrapper shall be converted (written) in the
> same way as e.g:
>
> ./sysdeps/unix/sysv/linux/clock_settime.c
>
> (the same issue is with getitimer).
>
> Then only redirection when __USE_TIME_BITS64 is set are needed to use
> __setitimer64/__getitimer64 on Y2038 safe systems.

I'm not sure what you mean here.

There is no setitimer64/getitimer64 syscall so we need to make sure we
pass a 32-bit time_t here. If __TIMESIZE == 32 then we don't need any
changes and can just directly make the syscall (as we do in this
patch). The conversion is only required if __TIMESIZE == 64 as we need
to convert to/from a 32-bit time_t.

Alistair

>
> > +  struct itimerval32 new_value_32;
> > +  new_value_32.it_interval
> > +    = valid_timeval64_to_timeval_long (new_value->it_interval);
> > +  new_value_32.it_value
> > +    = valid_timeval64_to_timeval_long (new_value->it_value);
> > +
> > +  if (old_value == NULL)
> > +    return INLINE_SYSCALL_CALL (setitimer, which, &new_value_32,
> > NULL); +
> > +  struct itimerval32 old_value_32;
> > +  if (INLINE_SYSCALL_CALL (setitimer, which, &new_value_32,
> > &old_value_32) == -1)
> > +    return -1;
> > +
> > +  /* Write all fields of 'old_value' regardless of overflow.  */
> > +  old_value->it_interval
> > +     = valid_timeval_long_to_timeval64 (old_value_32.it_interval);
> > +  old_value->it_value
> > +     = valid_timeval_long_to_timeval64 (old_value_32.it_value);
> > +  return 0;
> > +#else
> > +  return INLINE_SYSCALL_CALL (setitimer, which, new_value,
> > old_value); +#endif
> > +}
> > +
> > +weak_alias (__setitimer, setitimer)
> > diff --git
> > a/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> > b/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h new file
> > mode 100644 index 0000000000..aa960a6632 --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> > @@ -0,0 +1,35 @@
> > +/* Compatibility definitions for `struct timeval' with 32-bit time_t.
> > +   Copyright (C) 2020 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be
> > useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <http://www.gnu.org/licenses/>.  */
> > +
> > +#ifndef _TV32_COMPAT_H
> > +#define _TV32_COMPAT_H 1
> > +
> > +#include <features.h>
> > +
> > +#include <bits/types.h>
> > +#include <bits/types/time_t.h>
> > +#include <bits/types/struct_timeval.h>
> > +
> > +/* Structures containing 'struct timeval' with 32-bit time_t.  */
> > +struct itimerval32
> > +{
> > +  struct __timeval_long it_interval;
> > +  struct __timeval_long it_value;
> > +};
> > +
> > +#endif /* tv32-compat.h */
>
>
> 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 Thu, 16 Jan 2020, Alistair Francis wrote:

> > To allow it this syscall wrapper shall be converted (written) in the
> > same way as e.g:
> >
> > ./sysdeps/unix/sysv/linux/clock_settime.c
> >
> > (the same issue is with getitimer).
> >
> > Then only redirection when __USE_TIME_BITS64 is set are needed to use
> > __setitimer64/__getitimer64 on Y2038 safe systems.
> 
> I'm not sure what you mean here.
> 
> There is no setitimer64/getitimer64 syscall so we need to make sure we
> pass a 32-bit time_t here. If __TIMESIZE == 32 then we don't need any
> changes and can just directly make the syscall (as we do in this
> patch). The conversion is only required if __TIMESIZE == 64 as we need
> to convert to/from a 32-bit time_t.

This is a point about the general structure and goals of the Y2038 work.

We want to end up at a point where, on systems where time_t is currently 
32-bit, you can build with -D_TIME_BITS=64 and get 64-bit time_t instead.  
That means that all time-related functions, including getitimer and 
setitimer, will, on such systems, need to have two implementations in 
glibc, one using 64-bit times and one using 32-bit times.

The implementation structure generally being used is that the main 
implementation has an interface using 64-bit times (even if it ends up 
using syscalls with 32-bit times) and the one using 32-bit times is a thin 
wrapper around it (if time_t is already 64-bit, the second implementation 
does not exist and some macros ensure the first implementation keeps its 
existing name).  Once all functions have been moved to that structure, we 
can set things up so that all those 64-bit functions are exported from 
glibc and add _TIME_BITS support in the headers.
  

On Thu, Jan 16, 2020 at 11:21 AM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 16 Jan 2020, Alistair Francis wrote:
>
> > > To allow it this syscall wrapper shall be converted (written) in the
> > > same way as e.g:
> > >
> > > ./sysdeps/unix/sysv/linux/clock_settime.c
> > >
> > > (the same issue is with getitimer).
> > >
> > > Then only redirection when __USE_TIME_BITS64 is set are needed to use
> > > __setitimer64/__getitimer64 on Y2038 safe systems.
> >
> > I'm not sure what you mean here.
> >
> > There is no setitimer64/getitimer64 syscall so we need to make sure we
> > pass a 32-bit time_t here. If __TIMESIZE == 32 then we don't need any
> > changes and can just directly make the syscall (as we do in this
> > patch). The conversion is only required if __TIMESIZE == 64 as we need
> > to convert to/from a 32-bit time_t.
>
> This is a point about the general structure and goals of the Y2038 work.
>
> We want to end up at a point where, on systems where time_t is currently
> 32-bit, you can build with -D_TIME_BITS=64 and get 64-bit time_t instead.
> That means that all time-related functions, including getitimer and
> setitimer, will, on such systems, need to have two implementations in
> glibc, one using 64-bit times and one using 32-bit times.

I'm still confused. If we build with -D_TIME_BITS=64 does that then
mean that __TIMESIZE == 64 ?

In which case the user exposed time_t is 64-bit and these calls will
take a 64-bit time_t and convert it to/from a 32-bit time_t to pass to
the kernel. The user will only ever use/see a 64-bit time_t.

>
> The implementation structure generally being used is that the main
> implementation has an interface using 64-bit times (even if it ends up
> using syscalls with 32-bit times) and the one using 32-bit times is a thin

Isn't that what is happening here when __TIMESIZE == 64 ?

> wrapper around it (if time_t is already 64-bit, the second implementation
> does not exist and some macros ensure the first implementation keeps its
> existing name).  Once all functions have been moved to that structure, we
> can set things up so that all those 64-bit functions are exported from
> glibc and add _TIME_BITS support in the headers.

Ah, do you mean that glibc should expose a 64-bit time_t version if
__TIMESIZE == 32? So even __TIMESIZE == 32 systems can call a 64-bit
time_t version (even if the syscall ends up being 32-bit).

Alistair

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

Hi Alistair, Joseph

> On Thu, Jan 16, 2020 at 11:21 AM Joseph Myers
> <joseph@codesourcery.com> wrote:
> >
> > On Thu, 16 Jan 2020, Alistair Francis wrote:
> >  
> > > > To allow it this syscall wrapper shall be converted (written)
> > > > in the same way as e.g:
> > > >
> > > > ./sysdeps/unix/sysv/linux/clock_settime.c
> > > >
> > > > (the same issue is with getitimer).
> > > >
> > > > Then only redirection when __USE_TIME_BITS64 is set are needed
> > > > to use __setitimer64/__getitimer64 on Y2038 safe systems.  
> > >
> > > I'm not sure what you mean here.
> > >
> > > There is no setitimer64/getitimer64 syscall so we need to make
> > > sure we pass a 32-bit time_t here. If __TIMESIZE == 32 then we
> > > don't need any changes and can just directly make the syscall (as
> > > we do in this patch). The conversion is only required if
> > > __TIMESIZE == 64 as we need to convert to/from a 32-bit time_t.  
> >
> > This is a point about the general structure and goals of the Y2038
> > work.
> >
> > We want to end up at a point where, on systems where time_t is
> > currently 32-bit, you can build with -D_TIME_BITS=64 and get 64-bit
> > time_t instead. That means that all time-related functions,
> > including getitimer and setitimer, will, on such systems, need to
> > have two implementations in glibc, one using 64-bit times and one
> > using 32-bit times.  
> 
> I'm still confused. If we build with -D_TIME_BITS=64 does that then
> mean that __TIMESIZE == 64 ?
> 
> In which case the user exposed time_t is 64-bit and these calls will
> take a 64-bit time_t and convert it to/from a 32-bit time_t to pass to
> the kernel. The user will only ever use/see a 64-bit time_t.
> 
> >
> > The implementation structure generally being used is that the main
> > implementation has an interface using 64-bit times (even if it ends
> > up using syscalls with 32-bit times) and the one using 32-bit times
> > is a thin  
> 
> Isn't that what is happening here when __TIMESIZE == 64 ?
> 
> > wrapper around it (if time_t is already 64-bit, the second
> > implementation does not exist and some macros ensure the first
> > implementation keeps its existing name).  Once all functions have
> > been moved to that structure, we can set things up so that all
> > those 64-bit functions are exported from glibc and add _TIME_BITS
> > support in the headers.  
> 
> Ah, do you mean that glibc should expose a 64-bit time_t version if
> __TIMESIZE == 32?

Yes, exactly.

Currently 32 bit ARM (__WORDSIZE == 32 && __TIMESIZE == 32) use 32 bit
time_t. However, after compiling the source code with -D_TIME_BITS=64
the time_t will become 64 bit.

Please find example setup for tests_y2038:
https://github.com/lmajewski/y2038-tests/blob/master/Makefile#L25
(Two binaries are build from the same sources - one is Y2038 safe and
other is not).

The notable difference between the new RV32 port and existing ARM32 is
that for new ports (i.e. RV32) the __TIMESIZE = 64 is set from the
outset.
For the latter (ARM32) the __TIMESIZE = 32 is kept and IIRC we will
NOT update it to __TIMESIZE = 64 anytime soon. Instead, in exported
headers _TIME_BITS=64 will be set by default (approach similar to the
one for _FILE_OFFSET_BITS=64).

Joseph, am I right?

> So even __TIMESIZE == 32 systems can call a 64-bit
> time_t version (even if the syscall ends up being 32-bit).

+1

Please find example conversion code for timespec_get conversion:
https://patchwork.ozlabs.org/patch/1224222/

it internally uses __clock_gettime64, which uses 64 bit time no matter
on which architecture it runs (even with __TIMESIZE==32).

Please also refer to the branch, which moves 32 bit ports closer to
being Y2038 safe:

https://github.com/lmajewski/y2038_glibc/commits/glibc_timespec_get-conversion-v1

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

On Thu, 16 Jan 2020, Alistair Francis wrote:

> > We want to end up at a point where, on systems where time_t is currently
> > 32-bit, you can build with -D_TIME_BITS=64 and get 64-bit time_t instead.
> > That means that all time-related functions, including getitimer and
> > setitimer, will, on such systems, need to have two implementations in
> > glibc, one using 64-bit times and one using 32-bit times.
> 
> I'm still confused. If we build with -D_TIME_BITS=64 does that then
> mean that __TIMESIZE == 64 ?

No.  The semantics of __TIMESIZE are defined in the comment on 
bits/timesize.h.  It refers to the *default ABI*, not to any different ABI 
resulting from a setting of _TIME_BITS.  -D_TIME_BITS=64 will result in 
64-bit time_t, but it will not change __TIMESIZE.

> > The implementation structure generally being used is that the main
> > implementation has an interface using 64-bit times (even if it ends up
> > using syscalls with 32-bit times) and the one using 32-bit times is a thin
> 
> Isn't that what is happening here when __TIMESIZE == 64 ?

If __TIMESIZE == 64, the thin wrapper does not exist, because there's a 
"#if __TIMESIZE != 64" conditional around it (see e.g. 
sysdeps/unix/sysv/linux/clock_settime.c for the desired structure).

> > wrapper around it (if time_t is already 64-bit, the second implementation
> > does not exist and some macros ensure the first implementation keeps its
> > existing name).  Once all functions have been moved to that structure, we
> > can set things up so that all those 64-bit functions are exported from
> > glibc and add _TIME_BITS support in the headers.
> 
> Ah, do you mean that glibc should expose a 64-bit time_t version if
> __TIMESIZE == 32? So even __TIMESIZE == 32 systems can call a 64-bit
> time_t version (even if the syscall ends up being 32-bit).

Eventually glibc should expose all those 64-bit time_t versions.

Right now, when __TIMESIZE == 32 those versions should be defined, and 
called by the thin wrappers for 32-bit time_t, but not exported at public 
symbol version (until all of them exist and all the header changes are 
ready).
  

On Thu, Jan 16, 2020 at 11:34 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Alistair, Joseph
>
> > On Thu, Jan 16, 2020 at 11:21 AM Joseph Myers
> > <joseph@codesourcery.com> wrote:
> > >
> > > On Thu, 16 Jan 2020, Alistair Francis wrote:
> > >
> > > > > To allow it this syscall wrapper shall be converted (written)
> > > > > in the same way as e.g:
> > > > >
> > > > > ./sysdeps/unix/sysv/linux/clock_settime.c
> > > > >
> > > > > (the same issue is with getitimer).
> > > > >
> > > > > Then only redirection when __USE_TIME_BITS64 is set are needed
> > > > > to use __setitimer64/__getitimer64 on Y2038 safe systems.
> > > >
> > > > I'm not sure what you mean here.
> > > >
> > > > There is no setitimer64/getitimer64 syscall so we need to make
> > > > sure we pass a 32-bit time_t here. If __TIMESIZE == 32 then we
> > > > don't need any changes and can just directly make the syscall (as
> > > > we do in this patch). The conversion is only required if
> > > > __TIMESIZE == 64 as we need to convert to/from a 32-bit time_t.
> > >
> > > This is a point about the general structure and goals of the Y2038
> > > work.
> > >
> > > We want to end up at a point where, on systems where time_t is
> > > currently 32-bit, you can build with -D_TIME_BITS=64 and get 64-bit
> > > time_t instead. That means that all time-related functions,
> > > including getitimer and setitimer, will, on such systems, need to
> > > have two implementations in glibc, one using 64-bit times and one
> > > using 32-bit times.
> >
> > I'm still confused. If we build with -D_TIME_BITS=64 does that then
> > mean that __TIMESIZE == 64 ?
> >
> > In which case the user exposed time_t is 64-bit and these calls will
> > take a 64-bit time_t and convert it to/from a 32-bit time_t to pass to
> > the kernel. The user will only ever use/see a 64-bit time_t.
> >
> > >
> > > The implementation structure generally being used is that the main
> > > implementation has an interface using 64-bit times (even if it ends
> > > up using syscalls with 32-bit times) and the one using 32-bit times
> > > is a thin
> >
> > Isn't that what is happening here when __TIMESIZE == 64 ?
> >
> > > wrapper around it (if time_t is already 64-bit, the second
> > > implementation does not exist and some macros ensure the first
> > > implementation keeps its existing name).  Once all functions have
> > > been moved to that structure, we can set things up so that all
> > > those 64-bit functions are exported from glibc and add _TIME_BITS
> > > support in the headers.
> >
> > Ah, do you mean that glibc should expose a 64-bit time_t version if
> > __TIMESIZE == 32?
>
> Yes, exactly.
>
> Currently 32 bit ARM (__WORDSIZE == 32 && __TIMESIZE == 32) use 32 bit
> time_t. However, after compiling the source code with -D_TIME_BITS=64
> the time_t will become 64 bit.
>
> Please find example setup for tests_y2038:
> https://github.com/lmajewski/y2038-tests/blob/master/Makefile#L25
> (Two binaries are build from the same sources - one is Y2038 safe and
> other is not).
>
> The notable difference between the new RV32 port and existing ARM32 is
> that for new ports (i.e. RV32) the __TIMESIZE = 64 is set from the
> outset.
> For the latter (ARM32) the __TIMESIZE = 32 is kept and IIRC we will
> NOT update it to __TIMESIZE = 64 anytime soon. Instead, in exported
> headers _TIME_BITS=64 will be set by default (approach similar to the
> one for _FILE_OFFSET_BITS=64).

Doesn't that mean that I can fix this all by just using if _TIME_BITS
== 64 instead of __TIMESIZE?

>
> Joseph, am I right?
>
> > So even __TIMESIZE == 32 systems can call a 64-bit
> > time_t version (even if the syscall ends up being 32-bit).
>
> +1

Even if time_t is 64-bit you can't pass a value larger then 32-bits to
these functions as they get converted back to a 32-bit value for the
kernel syscall.

>
> Please find example conversion code for timespec_get conversion:
> https://patchwork.ozlabs.org/patch/1224222/
>
> it internally uses __clock_gettime64, which uses 64 bit time no matter
> on which architecture it runs (even with __TIMESIZE==32).

In this case though the syscall is always 32-bit, so I'm not sure if
there is a huge advantage here.

I don't see a point of converting this function to take a 64-bit value
if time_t is 32-bit as that doesn't give you anything. It does seem
worth changing the #if to check _TIME_BITS == 64 instead of __TIMESIZE
though.

Alistair

>
> Please also refer to the branch, which moves 32 bit ports closer to
> being Y2038 safe:
>
> https://github.com/lmajewski/y2038_glibc/commits/glibc_timespec_get-conversion-v1
>
> >
> > 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
  

On Fri, Jan 17, 2020 at 2:19 AM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 16 Jan 2020, Alistair Francis wrote:
>
> > > We want to end up at a point where, on systems where time_t is currently
> > > 32-bit, you can build with -D_TIME_BITS=64 and get 64-bit time_t instead.
> > > That means that all time-related functions, including getitimer and
> > > setitimer, will, on such systems, need to have two implementations in
> > > glibc, one using 64-bit times and one using 32-bit times.
> >
> > I'm still confused. If we build with -D_TIME_BITS=64 does that then
> > mean that __TIMESIZE == 64 ?
>
> No.  The semantics of __TIMESIZE are defined in the comment on
> bits/timesize.h.  It refers to the *default ABI*, not to any different ABI
> resulting from a setting of _TIME_BITS.  -D_TIME_BITS=64 will result in
> 64-bit time_t, but it will not change __TIMESIZE.
>
> > > The implementation structure generally being used is that the main
> > > implementation has an interface using 64-bit times (even if it ends up
> > > using syscalls with 32-bit times) and the one using 32-bit times is a thin
> >
> > Isn't that what is happening here when __TIMESIZE == 64 ?
>
> If __TIMESIZE == 64, the thin wrapper does not exist, because there's a
> "#if __TIMESIZE != 64" conditional around it (see e.g.
> sysdeps/unix/sysv/linux/clock_settime.c for the desired structure).
>
> > > wrapper around it (if time_t is already 64-bit, the second implementation
> > > does not exist and some macros ensure the first implementation keeps its
> > > existing name).  Once all functions have been moved to that structure, we
> > > can set things up so that all those 64-bit functions are exported from
> > > glibc and add _TIME_BITS support in the headers.
> >
> > Ah, do you mean that glibc should expose a 64-bit time_t version if
> > __TIMESIZE == 32? So even __TIMESIZE == 32 systems can call a 64-bit
> > time_t version (even if the syscall ends up being 32-bit).
>
> Eventually glibc should expose all those 64-bit time_t versions.

In this case won't that just happen automatically when _TIME_BITS is
64 (64-bit time_t)?

>
> Right now, when __TIMESIZE == 32 those versions should be defined, and
> called by the thin wrappers for 32-bit time_t, but not exported at public
> symbol version (until all of them exist and all the header changes are
> ready).

In this case though there is no 64-bit time_t syscall (we only have
32-bit even with 64-bit time_t) so why expose a 64-bit version?

Alistair

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

Hi Alistair,

> On Thu, Jan 16, 2020 at 11:34 PM Lukasz Majewski <lukma@denx.de>
> wrote:
> >
> > Hi Alistair, Joseph
> >  
> > > On Thu, Jan 16, 2020 at 11:21 AM Joseph Myers
> > > <joseph@codesourcery.com> wrote:  
> > > >
> > > > On Thu, 16 Jan 2020, Alistair Francis wrote:
> > > >  
> > > > > > To allow it this syscall wrapper shall be converted
> > > > > > (written) in the same way as e.g:
> > > > > >
> > > > > > ./sysdeps/unix/sysv/linux/clock_settime.c
> > > > > >
> > > > > > (the same issue is with getitimer).
> > > > > >
> > > > > > Then only redirection when __USE_TIME_BITS64 is set are
> > > > > > needed to use __setitimer64/__getitimer64 on Y2038 safe
> > > > > > systems.  
> > > > >
> > > > > I'm not sure what you mean here.
> > > > >
> > > > > There is no setitimer64/getitimer64 syscall so we need to make
> > > > > sure we pass a 32-bit time_t here. If __TIMESIZE == 32 then we
> > > > > don't need any changes and can just directly make the syscall
> > > > > (as we do in this patch). The conversion is only required if
> > > > > __TIMESIZE == 64 as we need to convert to/from a 32-bit
> > > > > time_t.  
> > > >
> > > > This is a point about the general structure and goals of the
> > > > Y2038 work.
> > > >
> > > > We want to end up at a point where, on systems where time_t is
> > > > currently 32-bit, you can build with -D_TIME_BITS=64 and get
> > > > 64-bit time_t instead. That means that all time-related
> > > > functions, including getitimer and setitimer, will, on such
> > > > systems, need to have two implementations in glibc, one using
> > > > 64-bit times and one using 32-bit times.  
> > >
> > > I'm still confused. If we build with -D_TIME_BITS=64 does that
> > > then mean that __TIMESIZE == 64 ?
> > >
> > > In which case the user exposed time_t is 64-bit and these calls
> > > will take a 64-bit time_t and convert it to/from a 32-bit time_t
> > > to pass to the kernel. The user will only ever use/see a 64-bit
> > > time_t. 
> > > >
> > > > The implementation structure generally being used is that the
> > > > main implementation has an interface using 64-bit times (even
> > > > if it ends up using syscalls with 32-bit times) and the one
> > > > using 32-bit times is a thin  
> > >
> > > Isn't that what is happening here when __TIMESIZE == 64 ?
> > >  
> > > > wrapper around it (if time_t is already 64-bit, the second
> > > > implementation does not exist and some macros ensure the first
> > > > implementation keeps its existing name).  Once all functions
> > > > have been moved to that structure, we can set things up so that
> > > > all those 64-bit functions are exported from glibc and add
> > > > _TIME_BITS support in the headers.  
> > >
> > > Ah, do you mean that glibc should expose a 64-bit time_t version
> > > if __TIMESIZE == 32?  
> >
> > Yes, exactly.
> >
> > Currently 32 bit ARM (__WORDSIZE == 32 && __TIMESIZE == 32) use 32
> > bit time_t. However, after compiling the source code with
> > -D_TIME_BITS=64 the time_t will become 64 bit.
> >
> > Please find example setup for tests_y2038:
> > https://github.com/lmajewski/y2038-tests/blob/master/Makefile#L25
> > (Two binaries are build from the same sources - one is Y2038 safe
> > and other is not).
> >
> > The notable difference between the new RV32 port and existing ARM32
> > is that for new ports (i.e. RV32) the __TIMESIZE = 64 is set from
> > the outset.
> > For the latter (ARM32) the __TIMESIZE = 32 is kept and IIRC we will
> > NOT update it to __TIMESIZE = 64 anytime soon. Instead, in exported
> > headers _TIME_BITS=64 will be set by default (approach similar to
> > the one for _FILE_OFFSET_BITS=64).  
> 
> Doesn't that mean that I can fix this all by just using if _TIME_BITS
> == 64 instead of __TIMESIZE?

Unfortunately, not. The _TIME_BITS define affects _exported_ glibc
headers. It sets __USE_TIME_BITS64, which shall NOT be used in glibc
internals:
https://github.com/lmajewski/y2038_glibc/commit/319c24507974fecdffe5182d310289b2a4cb9122#diff-a5ab6c74681eaf0569ed54f6bf0d7978R391


Instead, the glibc shall be prepared for handling those syscalls
correctly in both cases - when _TIME_BITS == 64 (Y2038 safe) is defined
or not during the compilation.

> 
> >
> > Joseph, am I right?
> >  
> > > So even __TIMESIZE == 32 systems can call a 64-bit
> > > time_t version (even if the syscall ends up being 32-bit).  
> >
> > +1  
> 
> Even if time_t is 64-bit you can't pass a value larger then 32-bits to
> these functions as they get converted back to a 32-bit value for the
> kernel syscall.

The problem is with consistency:

1. Internally glibc is going to replace time_t (which may be 32 bit)
with __time64_t (as it is done indirectly with introduction of struct
__timespec64, etc). In that way glibc will be itself Y2038 safe no
matter of __TIMESIZE value (32, 64 bit).

2. The functions with 64 suffix (i.e. __getitimer64, __settime64) 

e.g.:

int
__getitimer64 (__itimer_which_t which, struct __itimerval64 *curr_value)

Will use struct __itimerval64, which will always support 64 bit time
(the same situation as with struct __itimerspec64).


Afterwards, I will define redirection for it in the exported headers -
like for example here (for timerspec_get):
https://github.com/lmajewski/y2038_glibc/commit/9844cb7beb854e21bbdc595398627777b6155c27#diff-07934c1fe09f0e6357413e138856c786R320

This redirection will force usage of __gettimer64 on ARM32 bit
(__TIMESIZE==32 && __WORDSIZE==32) when -D_TIME_BITS=64 is set during
compilation.

(I'm now finishing patches to introduce struct __timeval64 to glibc)


> 
> >
> > Please find example conversion code for timespec_get conversion:
> > https://patchwork.ozlabs.org/patch/1224222/
> >
> > it internally uses __clock_gettime64, which uses 64 bit time no
> > matter on which architecture it runs (even with __TIMESIZE==32).  
> 
> In this case though the syscall is always 32-bit, so I'm not sure if
> there is a huge advantage here.

The advantage is that the consistency of function conversion will be
preserved. The __gettimer64 function will be used, even though we will
use 32 bit syscalls and when kernel (maybe?) introduce 64 bit syscalls
we will be ready.

> 
> I don't see a point of converting this function to take a 64-bit value
> if time_t is 32-bit as that doesn't give you anything.

time_t may be 32 bit or 64 on the Y2038 safe systems. It depends on
adding -D_TIME_BITS=64 during compilation.

> It does seem
> worth changing the #if to check _TIME_BITS == 64 instead of __TIMESIZE
> though.

As I've stated above - the _TIME_BITS (and __USE_TIME_BITS64) are
supposed to be "visible" in the glibc exported headers.
The __USE_TIME_BITS64 flag enables redirection for the syscalls:
https://github.com/lmajewski/y2038_glibc/commit/319c24507974fecdffe5182d310289b2a4cb9122#diff-8bdb75c23cf534160b06f2dc2183e971R67

On the glibc side one shall have time functions converted to:
https://github.com/lmajewski/y2038_glibc/commit/6367f6e061c5184b39cf7fff07ffcf8b569d765e#diff-a8bb3b17dcb4b2b40deba97323563301R33


> 
> Alistair
> 
> >
> > Please also refer to the branch, which moves 32 bit ports closer to
> > being Y2038 safe:
> >
> > https://github.com/lmajewski/y2038_glibc/commits/glibc_timespec_get-conversion-v1
> >  
> > >
> > > 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 Fri, Jan 17, 2020 at 9:14 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Alistair,
>
> > On Thu, Jan 16, 2020 at 11:34 PM Lukasz Majewski <lukma@denx.de>
> > wrote:
> > >
> > > Hi Alistair, Joseph
> > >
> > > > On Thu, Jan 16, 2020 at 11:21 AM Joseph Myers
> > > > <joseph@codesourcery.com> wrote:
> > > > >
> > > > > On Thu, 16 Jan 2020, Alistair Francis wrote:
> > > > >
> > > > > > > To allow it this syscall wrapper shall be converted
> > > > > > > (written) in the same way as e.g:
> > > > > > >
> > > > > > > ./sysdeps/unix/sysv/linux/clock_settime.c
> > > > > > >
> > > > > > > (the same issue is with getitimer).
> > > > > > >
> > > > > > > Then only redirection when __USE_TIME_BITS64 is set are
> > > > > > > needed to use __setitimer64/__getitimer64 on Y2038 safe
> > > > > > > systems.
> > > > > >
> > > > > > I'm not sure what you mean here.
> > > > > >
> > > > > > There is no setitimer64/getitimer64 syscall so we need to make
> > > > > > sure we pass a 32-bit time_t here. If __TIMESIZE == 32 then we
> > > > > > don't need any changes and can just directly make the syscall
> > > > > > (as we do in this patch). The conversion is only required if
> > > > > > __TIMESIZE == 64 as we need to convert to/from a 32-bit
> > > > > > time_t.
> > > > >
> > > > > This is a point about the general structure and goals of the
> > > > > Y2038 work.
> > > > >
> > > > > We want to end up at a point where, on systems where time_t is
> > > > > currently 32-bit, you can build with -D_TIME_BITS=64 and get
> > > > > 64-bit time_t instead. That means that all time-related
> > > > > functions, including getitimer and setitimer, will, on such
> > > > > systems, need to have two implementations in glibc, one using
> > > > > 64-bit times and one using 32-bit times.
> > > >
> > > > I'm still confused. If we build with -D_TIME_BITS=64 does that
> > > > then mean that __TIMESIZE == 64 ?
> > > >
> > > > In which case the user exposed time_t is 64-bit and these calls
> > > > will take a 64-bit time_t and convert it to/from a 32-bit time_t
> > > > to pass to the kernel. The user will only ever use/see a 64-bit
> > > > time_t.
> > > > >
> > > > > The implementation structure generally being used is that the
> > > > > main implementation has an interface using 64-bit times (even
> > > > > if it ends up using syscalls with 32-bit times) and the one
> > > > > using 32-bit times is a thin
> > > >
> > > > Isn't that what is happening here when __TIMESIZE == 64 ?
> > > >
> > > > > wrapper around it (if time_t is already 64-bit, the second
> > > > > implementation does not exist and some macros ensure the first
> > > > > implementation keeps its existing name).  Once all functions
> > > > > have been moved to that structure, we can set things up so that
> > > > > all those 64-bit functions are exported from glibc and add
> > > > > _TIME_BITS support in the headers.
> > > >
> > > > Ah, do you mean that glibc should expose a 64-bit time_t version
> > > > if __TIMESIZE == 32?
> > >
> > > Yes, exactly.
> > >
> > > Currently 32 bit ARM (__WORDSIZE == 32 && __TIMESIZE == 32) use 32
> > > bit time_t. However, after compiling the source code with
> > > -D_TIME_BITS=64 the time_t will become 64 bit.
> > >
> > > Please find example setup for tests_y2038:
> > > https://github.com/lmajewski/y2038-tests/blob/master/Makefile#L25
> > > (Two binaries are build from the same sources - one is Y2038 safe
> > > and other is not).
> > >
> > > The notable difference between the new RV32 port and existing ARM32
> > > is that for new ports (i.e. RV32) the __TIMESIZE = 64 is set from
> > > the outset.
> > > For the latter (ARM32) the __TIMESIZE = 32 is kept and IIRC we will
> > > NOT update it to __TIMESIZE = 64 anytime soon. Instead, in exported
> > > headers _TIME_BITS=64 will be set by default (approach similar to
> > > the one for _FILE_OFFSET_BITS=64).
> >
> > Doesn't that mean that I can fix this all by just using if _TIME_BITS
> > == 64 instead of __TIMESIZE?
>
> Unfortunately, not. The _TIME_BITS define affects _exported_ glibc
> headers. It sets __USE_TIME_BITS64, which shall NOT be used in glibc
> internals:
> https://github.com/lmajewski/y2038_glibc/commit/319c24507974fecdffe5182d310289b2a4cb9122#diff-a5ab6c74681eaf0569ed54f6bf0d7978R391
>
>
> Instead, the glibc shall be prepared for handling those syscalls
> correctly in both cases - when _TIME_BITS == 64 (Y2038 safe) is defined
> or not during the compilation.
>
> >
> > >
> > > Joseph, am I right?
> > >
> > > > So even __TIMESIZE == 32 systems can call a 64-bit
> > > > time_t version (even if the syscall ends up being 32-bit).
> > >
> > > +1
> >
> > Even if time_t is 64-bit you can't pass a value larger then 32-bits to
> > these functions as they get converted back to a 32-bit value for the
> > kernel syscall.
>
> The problem is with consistency:
>
> 1. Internally glibc is going to replace time_t (which may be 32 bit)
> with __time64_t (as it is done indirectly with introduction of struct
> __timespec64, etc). In that way glibc will be itself Y2038 safe no
> matter of __TIMESIZE value (32, 64 bit).
>
> 2. The functions with 64 suffix (i.e. __getitimer64, __settime64)
>
> e.g.:
>
> int
> __getitimer64 (__itimer_which_t which, struct __itimerval64 *curr_value)
>
> Will use struct __itimerval64, which will always support 64 bit time
> (the same situation as with struct __itimerspec64).
>
>
> Afterwards, I will define redirection for it in the exported headers -
> like for example here (for timerspec_get):
> https://github.com/lmajewski/y2038_glibc/commit/9844cb7beb854e21bbdc595398627777b6155c27#diff-07934c1fe09f0e6357413e138856c786R320
>
> This redirection will force usage of __gettimer64 on ARM32 bit
> (__TIMESIZE==32 && __WORDSIZE==32) when -D_TIME_BITS=64 is set during
> compilation.
>
> (I'm now finishing patches to introduce struct __timeval64 to glibc)
>
>
> >
> > >
> > > Please find example conversion code for timespec_get conversion:
> > > https://patchwork.ozlabs.org/patch/1224222/
> > >
> > > it internally uses __clock_gettime64, which uses 64 bit time no
> > > matter on which architecture it runs (even with __TIMESIZE==32).
> >
> > In this case though the syscall is always 32-bit, so I'm not sure if
> > there is a huge advantage here.
>
> The advantage is that the consistency of function conversion will be
> preserved. The __gettimer64 function will be used, even though we will
> use 32 bit syscalls and when kernel (maybe?) introduce 64 bit syscalls
> we will be ready.
>
> >
> > I don't see a point of converting this function to take a 64-bit value
> > if time_t is 32-bit as that doesn't give you anything.
>
> time_t may be 32 bit or 64 on the Y2038 safe systems. It depends on
> adding -D_TIME_BITS=64 during compilation.
>
> > It does seem
> > worth changing the #if to check _TIME_BITS == 64 instead of __TIMESIZE
> > though.
>
> As I've stated above - the _TIME_BITS (and __USE_TIME_BITS64) are
> supposed to be "visible" in the glibc exported headers.
> The __USE_TIME_BITS64 flag enables redirection for the syscalls:
> https://github.com/lmajewski/y2038_glibc/commit/319c24507974fecdffe5182d310289b2a4cb9122#diff-8bdb75c23cf534160b06f2dc2183e971R67
>
> On the glibc side one shall have time functions converted to:
> https://github.com/lmajewski/y2038_glibc/commit/6367f6e061c5184b39cf7fff07ffcf8b569d765e#diff-a8bb3b17dcb4b2b40deba97323563301R33

Thanks for clarifying that. I think I understand now.

So basically follow the other implementations (or the link directly
above) and add a __getitimer64/__setitimer64 function and a wrapper
__getitimer/__setitimer for the current 32-bit time_t.

Alistair

>
>
> >
> > Alistair
> >
> > >
> > > Please also refer to the branch, which moves 32 bit ports closer to
> > > being Y2038 safe:
> > >
> > > https://github.com/lmajewski/y2038_glibc/commits/glibc_timespec_get-conversion-v1
> > >
> > > >
> > > > 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
  

Hi Alistair,

> On Fri, Jan 17, 2020 at 9:14 PM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > Hi Alistair,
> >  
> > > On Thu, Jan 16, 2020 at 11:34 PM Lukasz Majewski <lukma@denx.de>
> > > wrote:  
> > > >
> > > > Hi Alistair, Joseph
> > > >  
> > > > > On Thu, Jan 16, 2020 at 11:21 AM Joseph Myers
> > > > > <joseph@codesourcery.com> wrote:  
> > > > > >
> > > > > > On Thu, 16 Jan 2020, Alistair Francis wrote:
> > > > > >  
> > > > > > > > To allow it this syscall wrapper shall be converted
> > > > > > > > (written) in the same way as e.g:
> > > > > > > >
> > > > > > > > ./sysdeps/unix/sysv/linux/clock_settime.c
> > > > > > > >
> > > > > > > > (the same issue is with getitimer).
> > > > > > > >
> > > > > > > > Then only redirection when __USE_TIME_BITS64 is set are
> > > > > > > > needed to use __setitimer64/__getitimer64 on Y2038 safe
> > > > > > > > systems.  
> > > > > > >
> > > > > > > I'm not sure what you mean here.
> > > > > > >
> > > > > > > There is no setitimer64/getitimer64 syscall so we need to
> > > > > > > make sure we pass a 32-bit time_t here. If __TIMESIZE ==
> > > > > > > 32 then we don't need any changes and can just directly
> > > > > > > make the syscall (as we do in this patch). The conversion
> > > > > > > is only required if __TIMESIZE == 64 as we need to
> > > > > > > convert to/from a 32-bit time_t.  
> > > > > >
> > > > > > This is a point about the general structure and goals of the
> > > > > > Y2038 work.
> > > > > >
> > > > > > We want to end up at a point where, on systems where time_t
> > > > > > is currently 32-bit, you can build with -D_TIME_BITS=64 and
> > > > > > get 64-bit time_t instead. That means that all time-related
> > > > > > functions, including getitimer and setitimer, will, on such
> > > > > > systems, need to have two implementations in glibc, one
> > > > > > using 64-bit times and one using 32-bit times.  
> > > > >
> > > > > I'm still confused. If we build with -D_TIME_BITS=64 does that
> > > > > then mean that __TIMESIZE == 64 ?
> > > > >
> > > > > In which case the user exposed time_t is 64-bit and these
> > > > > calls will take a 64-bit time_t and convert it to/from a
> > > > > 32-bit time_t to pass to the kernel. The user will only ever
> > > > > use/see a 64-bit time_t.  
> > > > > >
> > > > > > The implementation structure generally being used is that
> > > > > > the main implementation has an interface using 64-bit times
> > > > > > (even if it ends up using syscalls with 32-bit times) and
> > > > > > the one using 32-bit times is a thin  
> > > > >
> > > > > Isn't that what is happening here when __TIMESIZE == 64 ?
> > > > >  
> > > > > > wrapper around it (if time_t is already 64-bit, the second
> > > > > > implementation does not exist and some macros ensure the
> > > > > > first implementation keeps its existing name).  Once all
> > > > > > functions have been moved to that structure, we can set
> > > > > > things up so that all those 64-bit functions are exported
> > > > > > from glibc and add _TIME_BITS support in the headers.  
> > > > >
> > > > > Ah, do you mean that glibc should expose a 64-bit time_t
> > > > > version if __TIMESIZE == 32?  
> > > >
> > > > Yes, exactly.
> > > >
> > > > Currently 32 bit ARM (__WORDSIZE == 32 && __TIMESIZE == 32) use
> > > > 32 bit time_t. However, after compiling the source code with
> > > > -D_TIME_BITS=64 the time_t will become 64 bit.
> > > >
> > > > Please find example setup for tests_y2038:
> > > > https://github.com/lmajewski/y2038-tests/blob/master/Makefile#L25
> > > > (Two binaries are build from the same sources - one is Y2038
> > > > safe and other is not).
> > > >
> > > > The notable difference between the new RV32 port and existing
> > > > ARM32 is that for new ports (i.e. RV32) the __TIMESIZE = 64 is
> > > > set from the outset.
> > > > For the latter (ARM32) the __TIMESIZE = 32 is kept and IIRC we
> > > > will NOT update it to __TIMESIZE = 64 anytime soon. Instead, in
> > > > exported headers _TIME_BITS=64 will be set by default (approach
> > > > similar to the one for _FILE_OFFSET_BITS=64).  
> > >
> > > Doesn't that mean that I can fix this all by just using if
> > > _TIME_BITS == 64 instead of __TIMESIZE?  
> >
> > Unfortunately, not. The _TIME_BITS define affects _exported_ glibc
> > headers. It sets __USE_TIME_BITS64, which shall NOT be used in glibc
> > internals:
> > https://github.com/lmajewski/y2038_glibc/commit/319c24507974fecdffe5182d310289b2a4cb9122#diff-a5ab6c74681eaf0569ed54f6bf0d7978R391
> >
> >
> > Instead, the glibc shall be prepared for handling those syscalls
> > correctly in both cases - when _TIME_BITS == 64 (Y2038 safe) is
> > defined or not during the compilation.
> >  
> > >  
> > > >
> > > > Joseph, am I right?
> > > >  
> > > > > So even __TIMESIZE == 32 systems can call a 64-bit
> > > > > time_t version (even if the syscall ends up being 32-bit).  
> > > >
> > > > +1  
> > >
> > > Even if time_t is 64-bit you can't pass a value larger then
> > > 32-bits to these functions as they get converted back to a 32-bit
> > > value for the kernel syscall.  
> >
> > The problem is with consistency:
> >
> > 1. Internally glibc is going to replace time_t (which may be 32 bit)
> > with __time64_t (as it is done indirectly with introduction of
> > struct __timespec64, etc). In that way glibc will be itself Y2038
> > safe no matter of __TIMESIZE value (32, 64 bit).
> >
> > 2. The functions with 64 suffix (i.e. __getitimer64, __settime64)
> >
> > e.g.:
> >
> > int
> > __getitimer64 (__itimer_which_t which, struct __itimerval64
> > *curr_value)
> >
> > Will use struct __itimerval64, which will always support 64 bit time
> > (the same situation as with struct __itimerspec64).
> >
> >
> > Afterwards, I will define redirection for it in the exported
> > headers - like for example here (for timerspec_get):
> > https://github.com/lmajewski/y2038_glibc/commit/9844cb7beb854e21bbdc595398627777b6155c27#diff-07934c1fe09f0e6357413e138856c786R320
> >
> > This redirection will force usage of __gettimer64 on ARM32 bit
> > (__TIMESIZE==32 && __WORDSIZE==32) when -D_TIME_BITS=64 is set
> > during compilation.
> >
> > (I'm now finishing patches to introduce struct __timeval64 to glibc)
> >
> >  
> > >  
> > > >
> > > > Please find example conversion code for timespec_get conversion:
> > > > https://patchwork.ozlabs.org/patch/1224222/
> > > >
> > > > it internally uses __clock_gettime64, which uses 64 bit time no
> > > > matter on which architecture it runs (even with
> > > > __TIMESIZE==32).  
> > >
> > > In this case though the syscall is always 32-bit, so I'm not sure
> > > if there is a huge advantage here.  
> >
> > The advantage is that the consistency of function conversion will be
> > preserved. The __gettimer64 function will be used, even though we
> > will use 32 bit syscalls and when kernel (maybe?) introduce 64 bit
> > syscalls we will be ready.
> >  
> > >
> > > I don't see a point of converting this function to take a 64-bit
> > > value if time_t is 32-bit as that doesn't give you anything.  
> >
> > time_t may be 32 bit or 64 on the Y2038 safe systems. It depends on
> > adding -D_TIME_BITS=64 during compilation.
> >  
> > > It does seem
> > > worth changing the #if to check _TIME_BITS == 64 instead of
> > > __TIMESIZE though.  
> >
> > As I've stated above - the _TIME_BITS (and __USE_TIME_BITS64) are
> > supposed to be "visible" in the glibc exported headers.
> > The __USE_TIME_BITS64 flag enables redirection for the syscalls:
> > https://github.com/lmajewski/y2038_glibc/commit/319c24507974fecdffe5182d310289b2a4cb9122#diff-8bdb75c23cf534160b06f2dc2183e971R67
> >
> > On the glibc side one shall have time functions converted to:
> > https://github.com/lmajewski/y2038_glibc/commit/6367f6e061c5184b39cf7fff07ffcf8b569d765e#diff-a8bb3b17dcb4b2b40deba97323563301R33
> >  
> 
> Thanks for clarifying that. I think I understand now.
> 
> So basically follow the other implementations (or the link directly
> above) and add a __getitimer64/__setitimer64 function and a wrapper
> __getitimer/__setitimer for the current 32-bit time_t.

Yes, exactly. :-)

> 
> Alistair
> 
> >
> >  
> > >
> > > Alistair
> > >  
> > > >
> > > > Please also refer to the branch, which moves 32 bit ports
> > > > closer to being Y2038 safe:
> > > >
> > > > https://github.com/lmajewski/y2038_glibc/commits/glibc_timespec_get-conversion-v1
> > > >  
> > > > >
> > > > > 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  




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 Fri, 17 Jan 2020, Alistair Francis wrote:

> So basically follow the other implementations (or the link directly
> above) and add a __getitimer64/__setitimer64 function and a wrapper
> __getitimer/__setitimer for the current 32-bit time_t.

Yes.

We are in the middle of a very long series of patches - maybe ending up as 
hundreds of patches - that *prepare* for adding support for _TIME_BITS=64.  
None of those patches actually contain any _TIME_BITS or __USE_TIME_BITS64 
conditionals, because it's important that users of glibc get a consistent 
API (we don't want an intermediate state where defining _TIME_BITS=64 
gives users a broken interface where different functions have different 
notions of time_t, for example).  But new patches relating to 64-bit times 
should be consistent with the design of support for 64-bit time on systems 
that currently use 32-bit time.  And that means having a main function 
that uses 64-bit times in its interface, and on systems with 32-bit time, 
a thin wrapper that converts 32-bit times to 64-bit.

This design becomes particularly important when we get to functions that 
involve hundreds of lines of glibc code rather than thin wrappers round 
syscalls - for example, some pthreads functions.