[PATCH v7 2/2] sysv: linux: Pass 64-bit version of semctl syscall

[Alistair Francis]

On Sun, May 10, 2020 at 7:52 AM Stepan Golosunov <stepan@golosunov.pp.ru> wrote:
>
> On Sat, May 09, 2020 at 11:08:23AM -0700, Alistair Francis wrote:
> > On Thu, May 7, 2020 at 12:44 PM Stepan Golosunov <stepan@golosunov.pp.ru> wrote:
> > >
> > > On Tue, May 05, 2020 at 08:47:36AM -0700, Alistair Francis wrote:
> > > > The semctl_syscall() function passes a union semun to the kernel. The
> > > > union includes struct semid_ds as a member. On 32-bit architectures the
> > > > Linux kernel provides a *_high version of the 32-bit sem_otime and
> > > > sem_ctime values. These can be combined to get a 64-bit version of the
> > > > time.
> > > >
> > > > This patch adjusts the struct semid_ds to support the *_high versions
> > > > of sem_otime and sem_ctime. For 32-bit systems with a 64-bit time_t
> > > > this can be used to get a 64-bit time from the two 32-bit values.
> > > >
> > > > This change handles issues that would arrise with architectures that
> > > > might select __TIMESIZE at build time (and result in different size
> > > > semid_ds structs).
> > > >
> > > > As described by Adhemerval:
> > > >   1. If an ABI issues 'semctl (semid, semnum, IPC_STAT, ...)' with
> > > >      IPC_STAT without _IPC_CMD_TIME_64, the provided semid_ds buffer
> > > >      will be used directly on the syscall.  This is the case for
> > > >      64-bit architectures and 32-bit architecture without
> > > >      __TIMESIZE==32 support (RV32 for instance).
> > > >
> > > >   2. If an ABI issues 'semctl (semid, semnum, IPC_STAT, ...)' with
> > > >      IPC_STAT with _IPC_CMD_TIME_64 then __TIMESIZE==64 is implied.
> > > >      A temporary __semid_ds32 will be used to issue the syscall and its
> > > >      contents will be copied to users semid_ds buffer (which is also
> > > >      implied to be the __TIMESIZE==64).
> > >
> > > As discussed elsewhere, __TIMESIZE description is incorrect here. It's
> > > part of the abi and cannot be selected at build time.
> >
> > Yep, but I'm a little confused what else to use.
> >
> > We should only be using the *_high variables on 32-bit systems with a
> > 64-bit time_t. So we need someway to check if that is the case.
>
> Both programs with 64-bit time_t and 32-bit time_t will need to work
> with the same glibc when __TIMESIZE==32.
>
> > I could change the macros to only work for _WORDSIZE == 32 and then
> > have a dynamic check on sizeof(time_t) == 8. Would that work?
>
> sizeof(time_t) will probably work in user-included headers.  It won't
> work inside glibc.
>
> Either two different functions or presence of __IPC_CMD_TIME64 would
> work inside glibc.  Neither of those is needed when __TIMESIZE==64.
>
> > >
> > > (And __IPC_CMD_TIME_64 is not actually required for __TIMESIZE==64
> > > architectures like rv32.  If it is used on them it's just for
> > > consistency.  Otherwise those architectures can be covered by
> > > ((cmd & __IPC_CMD_TIME64) || (__TIMESIZE == 64))
> > > conditions inside #if __IPC_TIME64 blocks.
> > >
> > > Or __IPC_CMD_TIME_64 could be not used at all. __TIMESIZE==32 support
> > > can be implemented with the usual thin wrapper around __semctl64.
> > > I suspect this would be preferable if not too costly.)
> >
> > This isn't really the same as the usual wrappers though as there is no
> > semctl64 syscall here.
>
> It does not matter which syscalls are used to implement __semctl64
> function.
>
> I would suggest the following:
>
> 1. Forget about __IPC_CMD_TIME64.
> 2. Implement __TIMESIZE==64 on __WORDSIZE==32, making sure that on
> __TIMESIZE==32 architectures nothing changes (previous versions of
> the patch were approaching that goal).  Do not do any conversions in
> semctl_syscall — it will be need in __semctl_mode16
>
> This would be good enough for rv32 and will not break anything.
> After that y2038 support can be added to __TIMESIZE==32 architectures
> with the following steps (there is a couple of alternative ways to do
> it; this one is the usual one):

Ok, this is what I have gone with. I am sending v8 now which just
supports a fixed __TIMESIZE==64 on __WORDSIZE==32 support (and doesn't
break anything else).

>
> 3. Make sure that struct __semid_ds64 exists on all architectures
> (being the struct semid_ds on __TIMESIZE==64 architectures). It should
> use __time64_t when __TIMESIZE==32.
> 4. Rename __new_semctl to __semctl (both in function definition and in
> versioned_symbol).
> 5. Rename __semctl to __semctl64 (leaving versioned_symbol
> untouched).  Make sure it accepts only __time64_t types (by changing
> types and removing __TIMESIZE condition from __IPC_TIME64
> definition).
> 6. When __TIMESIZE==64 #define __semctl64 __semctl before __semctl64
> function definition.
> 7. When __TIMESIZE==32 implement __semctl function by copying
> __semctl_mode16, changing semctl_syscall call to __semctl64 call and
> adding conversions from __semid_ds64 to semid_ds when necessary.
> Check whether libc_hidden_def (__semctl64) is needed for now.
>
> This would get y2038 support for semctl to the same level as done in
> other functions.  The next step will be done much later, as it depends
> on all the y2038-affected functions:

Ok, I understand more now. This is similar to what is being done with
all the other calls.

>
> 8. Expose all y2038-changes (including __semctl64 function) to users
> of the library when __TIMESIZE==32.
>
>
> Additional note:
> There will be 3 different types used for semid_ds:
> Kernel (and semctl_syscall function) accepts struct semid64_ds (from
> asm/sembuf.h).
> __semctl64 will accept struct __semid_ds64.
> __semctl will accept 32-bit struct semid_ds (which is different from
> semid64_ds in sem_perm field).
> __semctl_mode16 accepts semid64_ds (in cases where it's needed and
> neither __semctl nor __semctl64 can deal with it).
>
> (There is also 4th type, ancient struct semid_ds from linux/sem.h.
> __old_semctl deals with it.)

Thanks for going over all of this.

Alistair