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 Thu, Jan 16, 2020 at 6:36 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Jan 16, 2020 at 8:31 AM Alistair Francis <alistair23@gmail.com> wrote:
> > 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.
>
> There are three approaches that we have discussed in the past:
>
> 1. Convert the timestamps in place where possible, with special cases
>     for architectures that have the upper halves in non-consecutive
>     locations
> 2. Create a new set of portable sysvipc data structures for time64
>     user space, e.g. modeled after the generic 64-bit structures,
>     and convert all fields between the kernel version and the libc
>     version.
> 3. Keep the current layout but rename the existing time fields and
>     add new 64-bit fields at the end.

Ah ok, I prefer option 1 I think.

So I guess we will do something like:

 1. Define __semid_ds32 in sysdeps/unix/sysv/linux/bits/sem.h based on
macros in sem-pad.h
    - This allows us to have a generic implementation in
sysdeps/unix/sysv/linux/bits that can be overwritten by other
architectures
 2. If 32-bit with 64-bit time_t pass __semid_ds32 into the kernel in
sysdeps/unix/sysv/linux/semctl.c
    - Then reconstruct the user exposed version (unchanged). This is
the current struct semid_ds and uses time_t
    - The problem is not every architecture uses time_t here, that
only seems to apply if __TIMESIZE == 32 so we should be ok here.

Alistair

>
> You can have a look at the musl implementation for the third approach,
> the main advantage there is that it simplifies conversion between
> the old and new format for supporting the time32 interfaces on
> the existing 32-bit architectures as wrappers on top of the tim64
> interfaces.
>
> There is also a simplified form of 1) that will work on most of the
> architectures that glibc cares about: i386, armel, rv32le, csky, nds32,
> ppc32, microblaze-le, sparc, parisc, sh-le and nios2 all have the 'high'
> fields in the right place so you can simply treat them as 64-bit
> fields with no padding, while armeb, mips (all of them), s390,
> microblaze-be, sh-be and m68k get it wrong in one way or another
> and require would require some special case here.
>
>            Arnd