Re: [RFC v6 06/23] sysdeps: Use long types with 64-bit time_t on 32-bit archs

[Alistair Francis <alistair23 at gmail dot com>]

On Mon, Jan 13, 2020 at 7:41 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sun, Jan 12, 2020 at 11:40 AM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > If we are using a 32-bit architecture with a 64-bit time_t let's
> > define __SEM_PAD_BEFORE_TIME as 1. This is because the kernel is
> > expecting a type __kernel_old_time_t which is 32-bit even with a
> > 64-bit time_t. Instead of adding another check, let's just set
> > __SEM_PAD_BEFORE_TIME as that will use a long type instead of
> > long long.
>
> I think you need to do this differently to avoid truncating the timestamp
> to 32 bit. The type in the generic kernel headers for 32-bit targets is not
> '__kernel_old_time_t sem_otime; __kernel_long_t pad' but it is
> 'unsigned long sem_otime; unsigned long sem_otime_high;'.

Argh! That is even harder.

I think I have updated both sem_otime and sem_ctime to have a high and
low 32-bit version. That looks actually pretty straightforward.

It does seem like we will also have to stich the two values together
as bits/sem.h expects them to just be a time_t. That seems self
contained in sysdeps/unix/sysv/linux/semctl.c though.

>
> Both need to be combined to read the 64-bit time_t value. It should
> be possible to do this in an architecture independent way, but note
> that the two halves are not always adjacent or in the correct order,
> and in one case (mips shmbuf) the high word is only 16 bit instead
> of 32.

That complicates things. I can probably get away with adding a new
define, but that seems like a pain. The best bet looks like allowing
arches to set __SEM_PAD_TIME and then having each arch specify their
own. Maybe with some sane default and then have MIPS set it's own.

Do glibc develepers have a preference here?

Alistair

>
>           Arnd
>
> > ---
> >  sysdeps/unix/sysv/linux/bits/sem-pad.h | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/sysdeps/unix/sysv/linux/bits/sem-pad.h b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > index 566ce039cc..4b419a6100 100644
> > --- a/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > @@ -30,4 +30,15 @@
> >     layout conversions for this structure.  */
> >
> >  #define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
> > -#define __SEM_PAD_BEFORE_TIME 0
> > +
> > +/* If we are using a 32-bit architecture with a 64-bit time_t let's
> > +   define __SEM_PAD_BEFORE_TIME as 1. This is because the kernel is
> > +   expecting a type __kernel_old_time_t which is 32-bit even with a
> > +   64-bit time_t. Instead of adding another check, let's just set
> > +   __SEM_PAD_BEFORE_TIME as that will use a long type instead of
> > +   long long.  */
> > +#if __WORDSIZE == 32 && __TIMESIZE == 64
> > +# define __SEM_PAD_BEFORE_TIME 1
> > +#else
> > +# define __SEM_PAD_BEFORE_TIME 0
> > +#endif
> > --
> > 2.24.1
> >