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

[Alistair Francis <alistair23 at gmail dot com>]

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