| Message ID | 20200505154736.1010242-3-alistair.francis@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support y2038 semctl_syscall() | expand |
Hi Alistair, > 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). Shall I provide any extra configuration (i.e. any extra flags) to have support for 64 bit time for this syscall on machines with __WORDSIZE == 32 && __TIMESIZE != 64 (i.e. ARM 32 bit)? Please correct me if I'm wrong, but from the below code (at bits/ipctypes.h) it seems like #define __IPC_CMD_TIME64 0x0 is for the above case (ARM 32 bit). > --- > bits/ipctypes.h | 10 ++ > sysdeps/mips/bits/ipctypes.h | 10 ++ > sysdeps/unix/sysv/linux/bits/ipc.h | 3 +- > sysdeps/unix/sysv/linux/bits/sem.h | 4 +- > sysdeps/unix/sysv/linux/ipc_priv.h | 4 + > sysdeps/unix/sysv/linux/semctl.c | 121 > ++++++++++++++++---- sysdeps/unix/sysv/linux/x86/bits/ipctypes.h | > 13 +++ 7 files changed, 141 insertions(+), 24 deletions(-) > > diff --git a/bits/ipctypes.h b/bits/ipctypes.h > index a955cdda7f..1b94122c32 100644 > --- a/bits/ipctypes.h > +++ b/bits/ipctypes.h > @@ -32,5 +32,15 @@ typedef unsigned short int __ipc_pid_t; > typedef int __ipc_pid_t; > # endif > > +#if __WORDSIZE == 64 \ > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > +# define __IPC_CMD_TIME64 0x0 > +#else > +# if __TIMESIZE == 64 > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > +# else > +# define __IPC_CMD_TIME64 0x0 > +# endif > +#endif > > #endif /* bits/ipctypes.h */ > diff --git a/sysdeps/mips/bits/ipctypes.h > b/sysdeps/mips/bits/ipctypes.h index 518f579b36..9a7218bf7d 100644 > --- a/sysdeps/mips/bits/ipctypes.h > +++ b/sysdeps/mips/bits/ipctypes.h > @@ -27,5 +27,15 @@ > > typedef __SLONG32_TYPE __ipc_pid_t; > > +#if __WORDSIZE == 64 \ > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > +# define __IPC_CMD_TIME64 0x0 > +#else > +# if __TIMESIZE == 64 > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > +# else > +# define __IPC_CMD_TIME64 0x0 > +# endif > +#endif > > #endif /* bits/ipctypes.h */ > diff --git a/sysdeps/unix/sysv/linux/bits/ipc.h > b/sysdeps/unix/sysv/linux/bits/ipc.h index 085dd628ac..2519322d16 > 100644 --- a/sysdeps/unix/sysv/linux/bits/ipc.h > +++ b/sysdeps/unix/sysv/linux/bits/ipc.h > @@ -20,6 +20,7 @@ > #endif > > #include <bits/types.h> > +#include <bits/ipctypes.h> > > /* Mode bits for `msgget', `semget', and `shmget'. */ > #define IPC_CREAT 01000 /* Create key if key > does not exist. */ @@ -29,7 +30,7 @@ > /* Control commands for `msgctl', `semctl', and `shmctl'. */ > #define IPC_RMID 0 /* Remove identifier. */ > #define IPC_SET 1 /* Set `ipc_perm' > options. */ -#define IPC_STAT 2 /* Get > `ipc_perm' options. */ +#define IPC_STAT (2 | > __IPC_CMD_TIME64) /* Get `ipc_perm' options. */ > #ifdef __USE_GNU # define IPC_INFO 3 /* See > ipcs. */ #endif > diff --git a/sysdeps/unix/sysv/linux/bits/sem.h > b/sysdeps/unix/sysv/linux/bits/sem.h index ba1169fdb3..8d0b88f5e0 > 100644 --- a/sysdeps/unix/sysv/linux/bits/sem.h > +++ b/sysdeps/unix/sysv/linux/bits/sem.h > @@ -54,9 +54,9 @@ > #ifdef __USE_MISC > > /* ipcs ctl cmds */ > -# define SEM_STAT 18 > +# define SEM_STAT (18 | __IPC_CMD_TIME64) > # define SEM_INFO 19 > -# define SEM_STAT_ANY 20 > +# define SEM_STAT_ANY (20 | __IPC_CMD_TIME64) > > struct seminfo > { > diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h > b/sysdeps/unix/sysv/linux/ipc_priv.h index 15a6e683a4..a1a7cacd17 > 100644 --- a/sysdeps/unix/sysv/linux/ipc_priv.h > +++ b/sysdeps/unix/sysv/linux/ipc_priv.h > @@ -43,6 +43,10 @@ struct __old_ipc_perm > unsigned short int __seq; /* Sequence number. */ > }; > > +#define __IPC_TIME64 \ > + (__WORDSIZE == 32 && __TIMESIZE == 64 \ > + && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) > + > #define SEMCTL_ARG_ADDRESS(__arg) &__arg.array > > #define MSGRCV_ARGS(__msgp, __msgtyp) \ > diff --git a/sysdeps/unix/sysv/linux/semctl.c > b/sysdeps/unix/sysv/linux/semctl.c index 30571af49f..99ad686194 100644 > --- a/sysdeps/unix/sysv/linux/semctl.c > +++ b/sysdeps/unix/sysv/linux/semctl.c > @@ -22,12 +22,16 @@ > #include <sysdep.h> > #include <shlib-compat.h> > #include <errno.h> > +#include <struct__semid_ds32.h> > #include <linux/posix_types.h> /* For __kernel_mode_t. */ > > /* Define a `union semun' suitable for Linux here. */ > union semun > { > int val; /* value for SETVAL */ > +#if __IPC_TIME64 > + struct __semid_ds32 *buf32; > +#endif > struct semid_ds *buf; /* buffer for IPC_STAT & > IPC_SET */ unsigned short int *array; /* array for GETALL & > SETALL */ struct seminfo *__buf; /* buffer for IPC_INFO */ > @@ -44,13 +48,54 @@ union semun > static int > semctl_syscall (int semid, int semnum, int cmd, union semun arg) > { > + int ret; > +#if __IPC_TIME64 > + /* A temporary buffer is used to avoid both an issue where the > export > + semid_ds might not follow the kernel's expected layout (due > + to {o,c}time{_high} alignment in 64-bit time case) and the > issue where > + some kernel versions might not clear the high bits when > returning > + then {o,c}time{_high} information. */ > + struct __semid_ds32 tmp; > + struct semid_ds *orig; > + bool restore = false; > + if (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY) > + { > + tmp = (struct __semid_ds32) { > + .sem_perm = arg.buf->sem_perm, > + .sem_otime = arg.buf->sem_otime, > + .sem_otime_high = arg.buf->sem_otime >> 32, > + .sem_ctime = arg.buf->sem_ctime, > + .sem_ctime_high = arg.buf->sem_ctime >> 32, > + .sem_nsems = arg.buf->sem_nsems, > + }; > + orig = arg.buf; > + arg.buf = (struct semid_ds*) &tmp; > + restore = true; > + } > +#endif > + > #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS > - return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64, > - arg.array); > + ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64, > + arg.array); > #else > - return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd > | __IPC_64, > - SEMCTL_ARG_ADDRESS (arg)); > + ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | > __IPC_64, > + SEMCTL_ARG_ADDRESS (arg)); > #endif > + > +#if __IPC_TIME64 > + if (ret >= 0 && restore) > + { > + arg.buf = orig; > + arg.buf->sem_perm = tmp.sem_perm; > + arg.buf->sem_otime = tmp.sem_otime > + | ((__time64_t) tmp.sem_otime_high << 32); > + arg.buf->sem_ctime = tmp.sem_ctime > + | ((__time64_t) tmp.sem_ctime_high << 32); > + arg.buf->sem_nsems = tmp.sem_nsems; > + } > +#endif > + > + return ret; > } > > int > @@ -64,15 +109,15 @@ __new_semctl (int semid, int semnum, int cmd, > ...) union semun arg = { 0 }; > va_list ap; > > - /* Get the argument only if required. */ > - switch (cmd) > + /* Get the argument only if required. Ignore the > __IPC_CMD_TIME64. */ > + switch (cmd & ~__IPC_CMD_TIME64) > { > case SETVAL: /* arg.val */ > case GETALL: /* arg.array */ > case SETALL: > - case IPC_STAT: /* arg.buf */ > + case IPC_STAT & ~__IPC_CMD_TIME64: /* arg.buf */ > case IPC_SET: > - case SEM_STAT: > + case SEM_STAT & ~__IPC_CMD_TIME64: > case IPC_INFO: /* arg.__buf */ > case SEM_INFO: > va_start (ap, cmd); > @@ -81,6 +126,20 @@ __new_semctl (int semid, int semnum, int cmd, ...) > break; > } > > +#ifdef __IPC_TIME64 > + struct __semid_ds32 tmp32; > + struct semid_ds *orig64; > + bool restore = false; > + > + if (cmd & __IPC_CMD_TIME64) > + { > + tmp32 = (struct __semid_ds32) { 0 }; > + orig64 = (struct semid_ds *) arg.buf; > + arg.buf = (struct semid_ds *) &tmp32; > + restore = true; > + } > +#endif > + > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > struct semid_ds tmpds; > if (cmd == IPC_SET) > @@ -91,26 +150,46 @@ __new_semctl (int semid, int semnum, int cmd, > ...) } > #endif > > - int ret = semctl_syscall (semid, semnum, cmd, arg); > + int ret = semctl_syscall (semid, semnum, cmd & ~__IPC_CMD_TIME64, > arg); > + if (ret < 0) > + return ret; > > - if (ret >= 0) > + switch (cmd & ~__IPC_CMD_TIME64) > { > - switch (cmd) > - { > - case IPC_STAT: > - case SEM_STAT: > - case SEM_STAT_ANY: > + case IPC_STAT: > + case SEM_STAT: > + case SEM_STAT_ANY: > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > - arg.buf->sem_perm.mode >>= 16; > +# if __IPC_TIME64 > + arg.buf32->sem_perm.mode >>= 16; > +# else > + arg.buf->sem_perm.mode >>= 16; > +# endif > #else > - /* Old Linux kernel versions might not clear the mode > padding. */ > - if (sizeof ((struct semid_ds){0}.sem_perm.mode) > - != sizeof (__kernel_mode_t)) > - arg.buf->sem_perm.mode &= 0xFFFF; > + /* Old Linux kernel versions might not clear the mode padding. > */ > + if (sizeof ((struct semid_ds){0}.sem_perm.mode) > + != sizeof (__kernel_mode_t)) > +# if __IPC_TIME64 > + arg.buf32->sem_perm.mode &= 0xFFFF; > +# else > + arg.buf->sem_perm.mode &= 0xFFFF; > +# endif > #endif > - } > } > > +#ifdef __IPC_TIME64 > + if (restore) > + { > + orig64->sem_perm = tmp32.sem_perm; > + orig64->sem_otime = tmp32.sem_otime > + | ((__time64_t) tmp32.sem_otime_high << > 32); > + orig64->sem_ctime = tmp32.sem_ctime > + | ((__time64_t) tmp32.sem_ctime_high << > 32); > + orig64->sem_nsems = tmp32.sem_nsems; > + arg.buf = (struct semid_ds *) orig64; > + } > +#endif > + > return ret; > } > versioned_symbol (libc, __new_semctl, semctl, DEFAULT_VERSION); > diff --git a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h > b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h index > 12e4cd7682..aa56da6cd5 100644 --- > a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h +++ > b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h @@ -23,6 +23,8 @@ > #ifndef _BITS_IPCTYPES_H > #define _BITS_IPCTYPES_H 1 > > +#include <bits/types.h> > + > /* Used in `struct shmid_ds'. */ > # ifdef __x86_64__ > typedef int __ipc_pid_t; > @@ -30,4 +32,15 @@ typedef int __ipc_pid_t; > typedef unsigned short int __ipc_pid_t; > # endif > > +#if __WORDSIZE == 64 \ > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > +# define __IPC_CMD_TIME64 0x0 > +#else > +# if __TIMESIZE == 64 > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > +# else > +# define __IPC_CMD_TIME64 0x0 > +# endif > +#endif > + > #endif /* bits/ipctypes.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, May 5, 2020 at 10:29 AM Lukasz Majewski <lukma@denx.de> wrote: > > Hi Alistair, > > > 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). > > Shall I provide any extra configuration (i.e. any extra flags) to have > support for 64 bit time for this syscall on machines with __WORDSIZE == > 32 && __TIMESIZE != 64 (i.e. ARM 32 bit)? I don't think so. This should "just work" with __TIMESIZE == 64 for ARM. I have a change for RV32 (ipctypes.h) to support RV32 as __TIMESIZE is always 64. > > Please correct me if I'm wrong, but from the below code (at > bits/ipctypes.h) it seems like #define __IPC_CMD_TIME64 0x0 is for the > above case (ARM 32 bit). It should be 0 by default and 0x100 for __TIMESIZE == 64. Alistair > > > --- > > bits/ipctypes.h | 10 ++ > > sysdeps/mips/bits/ipctypes.h | 10 ++ > > sysdeps/unix/sysv/linux/bits/ipc.h | 3 +- > > sysdeps/unix/sysv/linux/bits/sem.h | 4 +- > > sysdeps/unix/sysv/linux/ipc_priv.h | 4 + > > sysdeps/unix/sysv/linux/semctl.c | 121 > > ++++++++++++++++---- sysdeps/unix/sysv/linux/x86/bits/ipctypes.h | > > 13 +++ 7 files changed, 141 insertions(+), 24 deletions(-) > > > > diff --git a/bits/ipctypes.h b/bits/ipctypes.h > > index a955cdda7f..1b94122c32 100644 > > --- a/bits/ipctypes.h > > +++ b/bits/ipctypes.h > > @@ -32,5 +32,15 @@ typedef unsigned short int __ipc_pid_t; > > typedef int __ipc_pid_t; > > # endif > > > > +#if __WORDSIZE == 64 \ > > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > > +# define __IPC_CMD_TIME64 0x0 > > +#else > > +# if __TIMESIZE == 64 > > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > > +# else > > +# define __IPC_CMD_TIME64 0x0 > > +# endif > > +#endif > > > > #endif /* bits/ipctypes.h */ > > diff --git a/sysdeps/mips/bits/ipctypes.h > > b/sysdeps/mips/bits/ipctypes.h index 518f579b36..9a7218bf7d 100644 > > --- a/sysdeps/mips/bits/ipctypes.h > > +++ b/sysdeps/mips/bits/ipctypes.h > > @@ -27,5 +27,15 @@ > > > > typedef __SLONG32_TYPE __ipc_pid_t; > > > > +#if __WORDSIZE == 64 \ > > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > > +# define __IPC_CMD_TIME64 0x0 > > +#else > > +# if __TIMESIZE == 64 > > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > > +# else > > +# define __IPC_CMD_TIME64 0x0 > > +# endif > > +#endif > > > > #endif /* bits/ipctypes.h */ > > diff --git a/sysdeps/unix/sysv/linux/bits/ipc.h > > b/sysdeps/unix/sysv/linux/bits/ipc.h index 085dd628ac..2519322d16 > > 100644 --- a/sysdeps/unix/sysv/linux/bits/ipc.h > > +++ b/sysdeps/unix/sysv/linux/bits/ipc.h > > @@ -20,6 +20,7 @@ > > #endif > > > > #include <bits/types.h> > > +#include <bits/ipctypes.h> > > > > /* Mode bits for `msgget', `semget', and `shmget'. */ > > #define IPC_CREAT 01000 /* Create key if key > > does not exist. */ @@ -29,7 +30,7 @@ > > /* Control commands for `msgctl', `semctl', and `shmctl'. */ > > #define IPC_RMID 0 /* Remove identifier. */ > > #define IPC_SET 1 /* Set `ipc_perm' > > options. */ -#define IPC_STAT 2 /* Get > > `ipc_perm' options. */ +#define IPC_STAT (2 | > > __IPC_CMD_TIME64) /* Get `ipc_perm' options. */ > > #ifdef __USE_GNU # define IPC_INFO 3 /* See > > ipcs. */ #endif > > diff --git a/sysdeps/unix/sysv/linux/bits/sem.h > > b/sysdeps/unix/sysv/linux/bits/sem.h index ba1169fdb3..8d0b88f5e0 > > 100644 --- a/sysdeps/unix/sysv/linux/bits/sem.h > > +++ b/sysdeps/unix/sysv/linux/bits/sem.h > > @@ -54,9 +54,9 @@ > > #ifdef __USE_MISC > > > > /* ipcs ctl cmds */ > > -# define SEM_STAT 18 > > +# define SEM_STAT (18 | __IPC_CMD_TIME64) > > # define SEM_INFO 19 > > -# define SEM_STAT_ANY 20 > > +# define SEM_STAT_ANY (20 | __IPC_CMD_TIME64) > > > > struct seminfo > > { > > diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h > > b/sysdeps/unix/sysv/linux/ipc_priv.h index 15a6e683a4..a1a7cacd17 > > 100644 --- a/sysdeps/unix/sysv/linux/ipc_priv.h > > +++ b/sysdeps/unix/sysv/linux/ipc_priv.h > > @@ -43,6 +43,10 @@ struct __old_ipc_perm > > unsigned short int __seq; /* Sequence number. */ > > }; > > > > +#define __IPC_TIME64 \ > > + (__WORDSIZE == 32 && __TIMESIZE == 64 \ > > + && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) > > + > > #define SEMCTL_ARG_ADDRESS(__arg) &__arg.array > > > > #define MSGRCV_ARGS(__msgp, __msgtyp) \ > > diff --git a/sysdeps/unix/sysv/linux/semctl.c > > b/sysdeps/unix/sysv/linux/semctl.c index 30571af49f..99ad686194 100644 > > --- a/sysdeps/unix/sysv/linux/semctl.c > > +++ b/sysdeps/unix/sysv/linux/semctl.c > > @@ -22,12 +22,16 @@ > > #include <sysdep.h> > > #include <shlib-compat.h> > > #include <errno.h> > > +#include <struct__semid_ds32.h> > > #include <linux/posix_types.h> /* For __kernel_mode_t. */ > > > > /* Define a `union semun' suitable for Linux here. */ > > union semun > > { > > int val; /* value for SETVAL */ > > +#if __IPC_TIME64 > > + struct __semid_ds32 *buf32; > > +#endif > > struct semid_ds *buf; /* buffer for IPC_STAT & > > IPC_SET */ unsigned short int *array; /* array for GETALL & > > SETALL */ struct seminfo *__buf; /* buffer for IPC_INFO */ > > @@ -44,13 +48,54 @@ union semun > > static int > > semctl_syscall (int semid, int semnum, int cmd, union semun arg) > > { > > + int ret; > > +#if __IPC_TIME64 > > + /* A temporary buffer is used to avoid both an issue where the > > export > > + semid_ds might not follow the kernel's expected layout (due > > + to {o,c}time{_high} alignment in 64-bit time case) and the > > issue where > > + some kernel versions might not clear the high bits when > > returning > > + then {o,c}time{_high} information. */ > > + struct __semid_ds32 tmp; > > + struct semid_ds *orig; > > + bool restore = false; > > + if (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY) > > + { > > + tmp = (struct __semid_ds32) { > > + .sem_perm = arg.buf->sem_perm, > > + .sem_otime = arg.buf->sem_otime, > > + .sem_otime_high = arg.buf->sem_otime >> 32, > > + .sem_ctime = arg.buf->sem_ctime, > > + .sem_ctime_high = arg.buf->sem_ctime >> 32, > > + .sem_nsems = arg.buf->sem_nsems, > > + }; > > + orig = arg.buf; > > + arg.buf = (struct semid_ds*) &tmp; > > + restore = true; > > + } > > +#endif > > + > > #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS > > - return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64, > > - arg.array); > > + ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64, > > + arg.array); > > #else > > - return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd > > | __IPC_64, > > - SEMCTL_ARG_ADDRESS (arg)); > > + ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | > > __IPC_64, > > + SEMCTL_ARG_ADDRESS (arg)); > > #endif > > + > > +#if __IPC_TIME64 > > + if (ret >= 0 && restore) > > + { > > + arg.buf = orig; > > + arg.buf->sem_perm = tmp.sem_perm; > > + arg.buf->sem_otime = tmp.sem_otime > > + | ((__time64_t) tmp.sem_otime_high << 32); > > + arg.buf->sem_ctime = tmp.sem_ctime > > + | ((__time64_t) tmp.sem_ctime_high << 32); > > + arg.buf->sem_nsems = tmp.sem_nsems; > > + } > > +#endif > > + > > + return ret; > > } > > > > int > > @@ -64,15 +109,15 @@ __new_semctl (int semid, int semnum, int cmd, > > ...) union semun arg = { 0 }; > > va_list ap; > > > > - /* Get the argument only if required. */ > > - switch (cmd) > > + /* Get the argument only if required. Ignore the > > __IPC_CMD_TIME64. */ > > + switch (cmd & ~__IPC_CMD_TIME64) > > { > > case SETVAL: /* arg.val */ > > case GETALL: /* arg.array */ > > case SETALL: > > - case IPC_STAT: /* arg.buf */ > > + case IPC_STAT & ~__IPC_CMD_TIME64: /* arg.buf */ > > case IPC_SET: > > - case SEM_STAT: > > + case SEM_STAT & ~__IPC_CMD_TIME64: > > case IPC_INFO: /* arg.__buf */ > > case SEM_INFO: > > va_start (ap, cmd); > > @@ -81,6 +126,20 @@ __new_semctl (int semid, int semnum, int cmd, ...) > > break; > > } > > > > +#ifdef __IPC_TIME64 > > + struct __semid_ds32 tmp32; > > + struct semid_ds *orig64; > > + bool restore = false; > > + > > + if (cmd & __IPC_CMD_TIME64) > > + { > > + tmp32 = (struct __semid_ds32) { 0 }; > > + orig64 = (struct semid_ds *) arg.buf; > > + arg.buf = (struct semid_ds *) &tmp32; > > + restore = true; > > + } > > +#endif > > + > > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > > struct semid_ds tmpds; > > if (cmd == IPC_SET) > > @@ -91,26 +150,46 @@ __new_semctl (int semid, int semnum, int cmd, > > ...) } > > #endif > > > > - int ret = semctl_syscall (semid, semnum, cmd, arg); > > + int ret = semctl_syscall (semid, semnum, cmd & ~__IPC_CMD_TIME64, > > arg); > > + if (ret < 0) > > + return ret; > > > > - if (ret >= 0) > > + switch (cmd & ~__IPC_CMD_TIME64) > > { > > - switch (cmd) > > - { > > - case IPC_STAT: > > - case SEM_STAT: > > - case SEM_STAT_ANY: > > + case IPC_STAT: > > + case SEM_STAT: > > + case SEM_STAT_ANY: > > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > > - arg.buf->sem_perm.mode >>= 16; > > +# if __IPC_TIME64 > > + arg.buf32->sem_perm.mode >>= 16; > > +# else > > + arg.buf->sem_perm.mode >>= 16; > > +# endif > > #else > > - /* Old Linux kernel versions might not clear the mode > > padding. */ > > - if (sizeof ((struct semid_ds){0}.sem_perm.mode) > > - != sizeof (__kernel_mode_t)) > > - arg.buf->sem_perm.mode &= 0xFFFF; > > + /* Old Linux kernel versions might not clear the mode padding. > > */ > > + if (sizeof ((struct semid_ds){0}.sem_perm.mode) > > + != sizeof (__kernel_mode_t)) > > +# if __IPC_TIME64 > > + arg.buf32->sem_perm.mode &= 0xFFFF; > > +# else > > + arg.buf->sem_perm.mode &= 0xFFFF; > > +# endif > > #endif > > - } > > } > > > > +#ifdef __IPC_TIME64 > > + if (restore) > > + { > > + orig64->sem_perm = tmp32.sem_perm; > > + orig64->sem_otime = tmp32.sem_otime > > + | ((__time64_t) tmp32.sem_otime_high << > > 32); > > + orig64->sem_ctime = tmp32.sem_ctime > > + | ((__time64_t) tmp32.sem_ctime_high << > > 32); > > + orig64->sem_nsems = tmp32.sem_nsems; > > + arg.buf = (struct semid_ds *) orig64; > > + } > > +#endif > > + > > return ret; > > } > > versioned_symbol (libc, __new_semctl, semctl, DEFAULT_VERSION); > > diff --git a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h > > b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h index > > 12e4cd7682..aa56da6cd5 100644 --- > > a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h +++ > > b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h @@ -23,6 +23,8 @@ > > #ifndef _BITS_IPCTYPES_H > > #define _BITS_IPCTYPES_H 1 > > > > +#include <bits/types.h> > > + > > /* Used in `struct shmid_ds'. */ > > # ifdef __x86_64__ > > typedef int __ipc_pid_t; > > @@ -30,4 +32,15 @@ typedef int __ipc_pid_t; > > typedef unsigned short int __ipc_pid_t; > > # endif > > > > +#if __WORDSIZE == 64 \ > > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > > +# define __IPC_CMD_TIME64 0x0 > > +#else > > +# if __TIMESIZE == 64 > > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > > +# else > > +# define __IPC_CMD_TIME64 0x0 > > +# endif > > +#endif > > + > > #endif /* bits/ipctypes.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, 5 May 2020 11:22:05 -0700 Alistair Francis <alistair23@gmail.com> wrote: > On Tue, May 5, 2020 at 10:29 AM Lukasz Majewski <lukma@denx.de> wrote: > > > > Hi Alistair, > > > > > 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). > > > > Shall I provide any extra configuration (i.e. any extra flags) to > > have support for 64 bit time for this syscall on machines with > > __WORDSIZE == 32 && __TIMESIZE != 64 (i.e. ARM 32 bit)? > > I don't think so. This should "just work" with __TIMESIZE == 64 for > ARM. But for ARM 32 the __TIMESIZE == 32, so will it be possible to have 64 bit time for this case? (To be in sync with other work regarding being Y2038 safe on machines with __WORDSIZE == 32 && __TIMESIZE != 64)? > > I have a change for RV32 (ipctypes.h) to support RV32 as __TIMESIZE is > always 64. > > > > > Please correct me if I'm wrong, but from the below code (at > > bits/ipctypes.h) it seems like #define __IPC_CMD_TIME64 0x0 is for > > the above case (ARM 32 bit). > > It should be 0 by default and 0x100 for __TIMESIZE == 64. > > Alistair > > > > > > --- > > > bits/ipctypes.h | 10 ++ > > > sysdeps/mips/bits/ipctypes.h | 10 ++ > > > sysdeps/unix/sysv/linux/bits/ipc.h | 3 +- > > > sysdeps/unix/sysv/linux/bits/sem.h | 4 +- > > > sysdeps/unix/sysv/linux/ipc_priv.h | 4 + > > > sysdeps/unix/sysv/linux/semctl.c | 121 > > > ++++++++++++++++---- sysdeps/unix/sysv/linux/x86/bits/ipctypes.h | > > > 13 +++ 7 files changed, 141 insertions(+), 24 deletions(-) > > > > > > diff --git a/bits/ipctypes.h b/bits/ipctypes.h > > > index a955cdda7f..1b94122c32 100644 > > > --- a/bits/ipctypes.h > > > +++ b/bits/ipctypes.h > > > @@ -32,5 +32,15 @@ typedef unsigned short int __ipc_pid_t; > > > typedef int __ipc_pid_t; > > > # endif > > > > > > +#if __WORDSIZE == 64 \ > > > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > > > +# define __IPC_CMD_TIME64 0x0 > > > +#else > > > +# if __TIMESIZE == 64 > > > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > > > +# else > > > +# define __IPC_CMD_TIME64 0x0 > > > +# endif > > > +#endif > > > > > > #endif /* bits/ipctypes.h */ > > > diff --git a/sysdeps/mips/bits/ipctypes.h > > > b/sysdeps/mips/bits/ipctypes.h index 518f579b36..9a7218bf7d 100644 > > > --- a/sysdeps/mips/bits/ipctypes.h > > > +++ b/sysdeps/mips/bits/ipctypes.h > > > @@ -27,5 +27,15 @@ > > > > > > typedef __SLONG32_TYPE __ipc_pid_t; > > > > > > +#if __WORDSIZE == 64 \ > > > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > > > +# define __IPC_CMD_TIME64 0x0 > > > +#else > > > +# if __TIMESIZE == 64 > > > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > > > +# else > > > +# define __IPC_CMD_TIME64 0x0 > > > +# endif > > > +#endif > > > > > > #endif /* bits/ipctypes.h */ > > > diff --git a/sysdeps/unix/sysv/linux/bits/ipc.h > > > b/sysdeps/unix/sysv/linux/bits/ipc.h index 085dd628ac..2519322d16 > > > 100644 --- a/sysdeps/unix/sysv/linux/bits/ipc.h > > > +++ b/sysdeps/unix/sysv/linux/bits/ipc.h > > > @@ -20,6 +20,7 @@ > > > #endif > > > > > > #include <bits/types.h> > > > +#include <bits/ipctypes.h> > > > > > > /* Mode bits for `msgget', `semget', and `shmget'. */ > > > #define IPC_CREAT 01000 /* Create key if key > > > does not exist. */ @@ -29,7 +30,7 @@ > > > /* Control commands for `msgctl', `semctl', and `shmctl'. */ > > > #define IPC_RMID 0 /* Remove identifier. */ > > > #define IPC_SET 1 /* Set `ipc_perm' > > > options. */ -#define IPC_STAT 2 /* Get > > > `ipc_perm' options. */ +#define IPC_STAT (2 | > > > __IPC_CMD_TIME64) /* Get `ipc_perm' options. */ > > > #ifdef __USE_GNU # define IPC_INFO 3 /* See > > > ipcs. */ #endif > > > diff --git a/sysdeps/unix/sysv/linux/bits/sem.h > > > b/sysdeps/unix/sysv/linux/bits/sem.h index ba1169fdb3..8d0b88f5e0 > > > 100644 --- a/sysdeps/unix/sysv/linux/bits/sem.h > > > +++ b/sysdeps/unix/sysv/linux/bits/sem.h > > > @@ -54,9 +54,9 @@ > > > #ifdef __USE_MISC > > > > > > /* ipcs ctl cmds */ > > > -# define SEM_STAT 18 > > > +# define SEM_STAT (18 | __IPC_CMD_TIME64) > > > # define SEM_INFO 19 > > > -# define SEM_STAT_ANY 20 > > > +# define SEM_STAT_ANY (20 | __IPC_CMD_TIME64) > > > > > > struct seminfo > > > { > > > diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h > > > b/sysdeps/unix/sysv/linux/ipc_priv.h index 15a6e683a4..a1a7cacd17 > > > 100644 --- a/sysdeps/unix/sysv/linux/ipc_priv.h > > > +++ b/sysdeps/unix/sysv/linux/ipc_priv.h > > > @@ -43,6 +43,10 @@ struct __old_ipc_perm > > > unsigned short int __seq; /* Sequence number. */ > > > }; > > > > > > +#define __IPC_TIME64 \ > > > + (__WORDSIZE == 32 && __TIMESIZE == 64 \ > > > + && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == > > > 32)) + > > > #define SEMCTL_ARG_ADDRESS(__arg) &__arg.array > > > > > > #define MSGRCV_ARGS(__msgp, __msgtyp) \ > > > diff --git a/sysdeps/unix/sysv/linux/semctl.c > > > b/sysdeps/unix/sysv/linux/semctl.c index 30571af49f..99ad686194 > > > 100644 --- a/sysdeps/unix/sysv/linux/semctl.c > > > +++ b/sysdeps/unix/sysv/linux/semctl.c > > > @@ -22,12 +22,16 @@ > > > #include <sysdep.h> > > > #include <shlib-compat.h> > > > #include <errno.h> > > > +#include <struct__semid_ds32.h> > > > #include <linux/posix_types.h> /* For __kernel_mode_t. */ > > > > > > /* Define a `union semun' suitable for Linux here. */ > > > union semun > > > { > > > int val; /* value for SETVAL */ > > > +#if __IPC_TIME64 > > > + struct __semid_ds32 *buf32; > > > +#endif > > > struct semid_ds *buf; /* buffer for IPC_STAT & > > > IPC_SET */ unsigned short int *array; /* array for GETALL & > > > SETALL */ struct seminfo *__buf; /* buffer for IPC_INFO */ > > > @@ -44,13 +48,54 @@ union semun > > > static int > > > semctl_syscall (int semid, int semnum, int cmd, union semun arg) > > > { > > > + int ret; > > > +#if __IPC_TIME64 > > > + /* A temporary buffer is used to avoid both an issue where the > > > export > > > + semid_ds might not follow the kernel's expected layout (due > > > + to {o,c}time{_high} alignment in 64-bit time case) and the > > > issue where > > > + some kernel versions might not clear the high bits when > > > returning > > > + then {o,c}time{_high} information. */ > > > + struct __semid_ds32 tmp; > > > + struct semid_ds *orig; > > > + bool restore = false; > > > + if (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY) > > > + { > > > + tmp = (struct __semid_ds32) { > > > + .sem_perm = arg.buf->sem_perm, > > > + .sem_otime = arg.buf->sem_otime, > > > + .sem_otime_high = arg.buf->sem_otime >> 32, > > > + .sem_ctime = arg.buf->sem_ctime, > > > + .sem_ctime_high = arg.buf->sem_ctime >> 32, > > > + .sem_nsems = arg.buf->sem_nsems, > > > + }; > > > + orig = arg.buf; > > > + arg.buf = (struct semid_ds*) &tmp; > > > + restore = true; > > > + } > > > +#endif > > > + > > > #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS > > > - return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | > > > __IPC_64, > > > - arg.array); > > > + ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | > > > __IPC_64, > > > + arg.array); > > > #else > > > - return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, > > > cmd | __IPC_64, > > > - SEMCTL_ARG_ADDRESS (arg)); > > > + ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, > > > cmd | __IPC_64, > > > + SEMCTL_ARG_ADDRESS (arg)); > > > #endif > > > + > > > +#if __IPC_TIME64 > > > + if (ret >= 0 && restore) > > > + { > > > + arg.buf = orig; > > > + arg.buf->sem_perm = tmp.sem_perm; > > > + arg.buf->sem_otime = tmp.sem_otime > > > + | ((__time64_t) tmp.sem_otime_high << > > > 32); > > > + arg.buf->sem_ctime = tmp.sem_ctime > > > + | ((__time64_t) tmp.sem_ctime_high << > > > 32); > > > + arg.buf->sem_nsems = tmp.sem_nsems; > > > + } > > > +#endif > > > + > > > + return ret; > > > } > > > > > > int > > > @@ -64,15 +109,15 @@ __new_semctl (int semid, int semnum, int cmd, > > > ...) union semun arg = { 0 }; > > > va_list ap; > > > > > > - /* Get the argument only if required. */ > > > - switch (cmd) > > > + /* Get the argument only if required. Ignore the > > > __IPC_CMD_TIME64. */ > > > + switch (cmd & ~__IPC_CMD_TIME64) > > > { > > > case SETVAL: /* arg.val */ > > > case GETALL: /* arg.array */ > > > case SETALL: > > > - case IPC_STAT: /* arg.buf */ > > > + case IPC_STAT & ~__IPC_CMD_TIME64: /* arg.buf */ > > > case IPC_SET: > > > - case SEM_STAT: > > > + case SEM_STAT & ~__IPC_CMD_TIME64: > > > case IPC_INFO: /* arg.__buf */ > > > case SEM_INFO: > > > va_start (ap, cmd); > > > @@ -81,6 +126,20 @@ __new_semctl (int semid, int semnum, int cmd, > > > ...) break; > > > } > > > > > > +#ifdef __IPC_TIME64 > > > + struct __semid_ds32 tmp32; > > > + struct semid_ds *orig64; > > > + bool restore = false; > > > + > > > + if (cmd & __IPC_CMD_TIME64) > > > + { > > > + tmp32 = (struct __semid_ds32) { 0 }; > > > + orig64 = (struct semid_ds *) arg.buf; > > > + arg.buf = (struct semid_ds *) &tmp32; > > > + restore = true; > > > + } > > > +#endif > > > + > > > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > > > struct semid_ds tmpds; > > > if (cmd == IPC_SET) > > > @@ -91,26 +150,46 @@ __new_semctl (int semid, int semnum, int cmd, > > > ...) } > > > #endif > > > > > > - int ret = semctl_syscall (semid, semnum, cmd, arg); > > > + int ret = semctl_syscall (semid, semnum, cmd & > > > ~__IPC_CMD_TIME64, arg); > > > + if (ret < 0) > > > + return ret; > > > > > > - if (ret >= 0) > > > + switch (cmd & ~__IPC_CMD_TIME64) > > > { > > > - switch (cmd) > > > - { > > > - case IPC_STAT: > > > - case SEM_STAT: > > > - case SEM_STAT_ANY: > > > + case IPC_STAT: > > > + case SEM_STAT: > > > + case SEM_STAT_ANY: > > > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > > > - arg.buf->sem_perm.mode >>= 16; > > > +# if __IPC_TIME64 > > > + arg.buf32->sem_perm.mode >>= 16; > > > +# else > > > + arg.buf->sem_perm.mode >>= 16; > > > +# endif > > > #else > > > - /* Old Linux kernel versions might not clear the mode > > > padding. */ > > > - if (sizeof ((struct semid_ds){0}.sem_perm.mode) > > > - != sizeof (__kernel_mode_t)) > > > - arg.buf->sem_perm.mode &= 0xFFFF; > > > + /* Old Linux kernel versions might not clear the mode > > > padding. */ > > > + if (sizeof ((struct semid_ds){0}.sem_perm.mode) > > > + != sizeof (__kernel_mode_t)) > > > +# if __IPC_TIME64 > > > + arg.buf32->sem_perm.mode &= 0xFFFF; > > > +# else > > > + arg.buf->sem_perm.mode &= 0xFFFF; > > > +# endif > > > #endif > > > - } > > > } > > > > > > +#ifdef __IPC_TIME64 > > > + if (restore) > > > + { > > > + orig64->sem_perm = tmp32.sem_perm; > > > + orig64->sem_otime = tmp32.sem_otime > > > + | ((__time64_t) tmp32.sem_otime_high << > > > 32); > > > + orig64->sem_ctime = tmp32.sem_ctime > > > + | ((__time64_t) tmp32.sem_ctime_high << > > > 32); > > > + orig64->sem_nsems = tmp32.sem_nsems; > > > + arg.buf = (struct semid_ds *) orig64; > > > + } > > > +#endif > > > + > > > return ret; > > > } > > > versioned_symbol (libc, __new_semctl, semctl, DEFAULT_VERSION); > > > diff --git a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h > > > b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h index > > > 12e4cd7682..aa56da6cd5 100644 --- > > > a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h +++ > > > b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h @@ -23,6 +23,8 @@ > > > #ifndef _BITS_IPCTYPES_H > > > #define _BITS_IPCTYPES_H 1 > > > > > > +#include <bits/types.h> > > > + > > > /* Used in `struct shmid_ds'. */ > > > # ifdef __x86_64__ > > > typedef int __ipc_pid_t; > > > @@ -30,4 +32,15 @@ typedef int __ipc_pid_t; > > > typedef unsigned short int __ipc_pid_t; > > > # endif > > > > > > +#if __WORDSIZE == 64 \ > > > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > > > +# define __IPC_CMD_TIME64 0x0 > > > +#else > > > +# if __TIMESIZE == 64 > > > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > > > +# else > > > +# define __IPC_CMD_TIME64 0x0 > > > +# endif > > > +#endif > > > + > > > #endif /* bits/ipctypes.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 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, May 5, 2020 at 3:34 PM Lukasz Majewski <lukma@denx.de> wrote: > > On Tue, 5 May 2020 11:22:05 -0700 > Alistair Francis <alistair23@gmail.com> wrote: > > > On Tue, May 5, 2020 at 10:29 AM Lukasz Majewski <lukma@denx.de> wrote: > > > > > > Hi Alistair, > > > > > > > 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). > > > > > > Shall I provide any extra configuration (i.e. any extra flags) to > > > have support for 64 bit time for this syscall on machines with > > > __WORDSIZE == 32 && __TIMESIZE != 64 (i.e. ARM 32 bit)? > > > > I don't think so. This should "just work" with __TIMESIZE == 64 for > > ARM. > > But for ARM 32 the __TIMESIZE == 32, so will it be possible to have 64 > bit time for this case? I thought you would override the __TIMESIZE. Alistair > > (To be in sync with other work regarding being Y2038 safe on machines > with __WORDSIZE == 32 && __TIMESIZE != 64)? > > > > > I have a change for RV32 (ipctypes.h) to support RV32 as __TIMESIZE is > > always 64. > > > > > > > > Please correct me if I'm wrong, but from the below code (at > > > bits/ipctypes.h) it seems like #define __IPC_CMD_TIME64 0x0 is for > > > the above case (ARM 32 bit). > > > > It should be 0 by default and 0x100 for __TIMESIZE == 64. > > > > Alistair > > > > > > > > > --- > > > > bits/ipctypes.h | 10 ++ > > > > sysdeps/mips/bits/ipctypes.h | 10 ++ > > > > sysdeps/unix/sysv/linux/bits/ipc.h | 3 +- > > > > sysdeps/unix/sysv/linux/bits/sem.h | 4 +- > > > > sysdeps/unix/sysv/linux/ipc_priv.h | 4 + > > > > sysdeps/unix/sysv/linux/semctl.c | 121 > > > > ++++++++++++++++---- sysdeps/unix/sysv/linux/x86/bits/ipctypes.h | > > > > 13 +++ 7 files changed, 141 insertions(+), 24 deletions(-) > > > > > > > > diff --git a/bits/ipctypes.h b/bits/ipctypes.h > > > > index a955cdda7f..1b94122c32 100644 > > > > --- a/bits/ipctypes.h > > > > +++ b/bits/ipctypes.h > > > > @@ -32,5 +32,15 @@ typedef unsigned short int __ipc_pid_t; > > > > typedef int __ipc_pid_t; > > > > # endif > > > > > > > > +#if __WORDSIZE == 64 \ > > > > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > > > > +# define __IPC_CMD_TIME64 0x0 > > > > +#else > > > > +# if __TIMESIZE == 64 > > > > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > > > > +# else > > > > +# define __IPC_CMD_TIME64 0x0 > > > > +# endif > > > > +#endif > > > > > > > > #endif /* bits/ipctypes.h */ > > > > diff --git a/sysdeps/mips/bits/ipctypes.h > > > > b/sysdeps/mips/bits/ipctypes.h index 518f579b36..9a7218bf7d 100644 > > > > --- a/sysdeps/mips/bits/ipctypes.h > > > > +++ b/sysdeps/mips/bits/ipctypes.h > > > > @@ -27,5 +27,15 @@ > > > > > > > > typedef __SLONG32_TYPE __ipc_pid_t; > > > > > > > > +#if __WORDSIZE == 64 \ > > > > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > > > > +# define __IPC_CMD_TIME64 0x0 > > > > +#else > > > > +# if __TIMESIZE == 64 > > > > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > > > > +# else > > > > +# define __IPC_CMD_TIME64 0x0 > > > > +# endif > > > > +#endif > > > > > > > > #endif /* bits/ipctypes.h */ > > > > diff --git a/sysdeps/unix/sysv/linux/bits/ipc.h > > > > b/sysdeps/unix/sysv/linux/bits/ipc.h index 085dd628ac..2519322d16 > > > > 100644 --- a/sysdeps/unix/sysv/linux/bits/ipc.h > > > > +++ b/sysdeps/unix/sysv/linux/bits/ipc.h > > > > @@ -20,6 +20,7 @@ > > > > #endif > > > > > > > > #include <bits/types.h> > > > > +#include <bits/ipctypes.h> > > > > > > > > /* Mode bits for `msgget', `semget', and `shmget'. */ > > > > #define IPC_CREAT 01000 /* Create key if key > > > > does not exist. */ @@ -29,7 +30,7 @@ > > > > /* Control commands for `msgctl', `semctl', and `shmctl'. */ > > > > #define IPC_RMID 0 /* Remove identifier. */ > > > > #define IPC_SET 1 /* Set `ipc_perm' > > > > options. */ -#define IPC_STAT 2 /* Get > > > > `ipc_perm' options. */ +#define IPC_STAT (2 | > > > > __IPC_CMD_TIME64) /* Get `ipc_perm' options. */ > > > > #ifdef __USE_GNU # define IPC_INFO 3 /* See > > > > ipcs. */ #endif > > > > diff --git a/sysdeps/unix/sysv/linux/bits/sem.h > > > > b/sysdeps/unix/sysv/linux/bits/sem.h index ba1169fdb3..8d0b88f5e0 > > > > 100644 --- a/sysdeps/unix/sysv/linux/bits/sem.h > > > > +++ b/sysdeps/unix/sysv/linux/bits/sem.h > > > > @@ -54,9 +54,9 @@ > > > > #ifdef __USE_MISC > > > > > > > > /* ipcs ctl cmds */ > > > > -# define SEM_STAT 18 > > > > +# define SEM_STAT (18 | __IPC_CMD_TIME64) > > > > # define SEM_INFO 19 > > > > -# define SEM_STAT_ANY 20 > > > > +# define SEM_STAT_ANY (20 | __IPC_CMD_TIME64) > > > > > > > > struct seminfo > > > > { > > > > diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h > > > > b/sysdeps/unix/sysv/linux/ipc_priv.h index 15a6e683a4..a1a7cacd17 > > > > 100644 --- a/sysdeps/unix/sysv/linux/ipc_priv.h > > > > +++ b/sysdeps/unix/sysv/linux/ipc_priv.h > > > > @@ -43,6 +43,10 @@ struct __old_ipc_perm > > > > unsigned short int __seq; /* Sequence number. */ > > > > }; > > > > > > > > +#define __IPC_TIME64 \ > > > > + (__WORDSIZE == 32 && __TIMESIZE == 64 \ > > > > + && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == > > > > 32)) + > > > > #define SEMCTL_ARG_ADDRESS(__arg) &__arg.array > > > > > > > > #define MSGRCV_ARGS(__msgp, __msgtyp) \ > > > > diff --git a/sysdeps/unix/sysv/linux/semctl.c > > > > b/sysdeps/unix/sysv/linux/semctl.c index 30571af49f..99ad686194 > > > > 100644 --- a/sysdeps/unix/sysv/linux/semctl.c > > > > +++ b/sysdeps/unix/sysv/linux/semctl.c > > > > @@ -22,12 +22,16 @@ > > > > #include <sysdep.h> > > > > #include <shlib-compat.h> > > > > #include <errno.h> > > > > +#include <struct__semid_ds32.h> > > > > #include <linux/posix_types.h> /* For __kernel_mode_t. */ > > > > > > > > /* Define a `union semun' suitable for Linux here. */ > > > > union semun > > > > { > > > > int val; /* value for SETVAL */ > > > > +#if __IPC_TIME64 > > > > + struct __semid_ds32 *buf32; > > > > +#endif > > > > struct semid_ds *buf; /* buffer for IPC_STAT & > > > > IPC_SET */ unsigned short int *array; /* array for GETALL & > > > > SETALL */ struct seminfo *__buf; /* buffer for IPC_INFO */ > > > > @@ -44,13 +48,54 @@ union semun > > > > static int > > > > semctl_syscall (int semid, int semnum, int cmd, union semun arg) > > > > { > > > > + int ret; > > > > +#if __IPC_TIME64 > > > > + /* A temporary buffer is used to avoid both an issue where the > > > > export > > > > + semid_ds might not follow the kernel's expected layout (due > > > > + to {o,c}time{_high} alignment in 64-bit time case) and the > > > > issue where > > > > + some kernel versions might not clear the high bits when > > > > returning > > > > + then {o,c}time{_high} information. */ > > > > + struct __semid_ds32 tmp; > > > > + struct semid_ds *orig; > > > > + bool restore = false; > > > > + if (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY) > > > > + { > > > > + tmp = (struct __semid_ds32) { > > > > + .sem_perm = arg.buf->sem_perm, > > > > + .sem_otime = arg.buf->sem_otime, > > > > + .sem_otime_high = arg.buf->sem_otime >> 32, > > > > + .sem_ctime = arg.buf->sem_ctime, > > > > + .sem_ctime_high = arg.buf->sem_ctime >> 32, > > > > + .sem_nsems = arg.buf->sem_nsems, > > > > + }; > > > > + orig = arg.buf; > > > > + arg.buf = (struct semid_ds*) &tmp; > > > > + restore = true; > > > > + } > > > > +#endif > > > > + > > > > #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS > > > > - return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | > > > > __IPC_64, > > > > - arg.array); > > > > + ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | > > > > __IPC_64, > > > > + arg.array); > > > > #else > > > > - return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, > > > > cmd | __IPC_64, > > > > - SEMCTL_ARG_ADDRESS (arg)); > > > > + ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, > > > > cmd | __IPC_64, > > > > + SEMCTL_ARG_ADDRESS (arg)); > > > > #endif > > > > + > > > > +#if __IPC_TIME64 > > > > + if (ret >= 0 && restore) > > > > + { > > > > + arg.buf = orig; > > > > + arg.buf->sem_perm = tmp.sem_perm; > > > > + arg.buf->sem_otime = tmp.sem_otime > > > > + | ((__time64_t) tmp.sem_otime_high << > > > > 32); > > > > + arg.buf->sem_ctime = tmp.sem_ctime > > > > + | ((__time64_t) tmp.sem_ctime_high << > > > > 32); > > > > + arg.buf->sem_nsems = tmp.sem_nsems; > > > > + } > > > > +#endif > > > > + > > > > + return ret; > > > > } > > > > > > > > int > > > > @@ -64,15 +109,15 @@ __new_semctl (int semid, int semnum, int cmd, > > > > ...) union semun arg = { 0 }; > > > > va_list ap; > > > > > > > > - /* Get the argument only if required. */ > > > > - switch (cmd) > > > > + /* Get the argument only if required. Ignore the > > > > __IPC_CMD_TIME64. */ > > > > + switch (cmd & ~__IPC_CMD_TIME64) > > > > { > > > > case SETVAL: /* arg.val */ > > > > case GETALL: /* arg.array */ > > > > case SETALL: > > > > - case IPC_STAT: /* arg.buf */ > > > > + case IPC_STAT & ~__IPC_CMD_TIME64: /* arg.buf */ > > > > case IPC_SET: > > > > - case SEM_STAT: > > > > + case SEM_STAT & ~__IPC_CMD_TIME64: > > > > case IPC_INFO: /* arg.__buf */ > > > > case SEM_INFO: > > > > va_start (ap, cmd); > > > > @@ -81,6 +126,20 @@ __new_semctl (int semid, int semnum, int cmd, > > > > ...) break; > > > > } > > > > > > > > +#ifdef __IPC_TIME64 > > > > + struct __semid_ds32 tmp32; > > > > + struct semid_ds *orig64; > > > > + bool restore = false; > > > > + > > > > + if (cmd & __IPC_CMD_TIME64) > > > > + { > > > > + tmp32 = (struct __semid_ds32) { 0 }; > > > > + orig64 = (struct semid_ds *) arg.buf; > > > > + arg.buf = (struct semid_ds *) &tmp32; > > > > + restore = true; > > > > + } > > > > +#endif > > > > + > > > > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > > > > struct semid_ds tmpds; > > > > if (cmd == IPC_SET) > > > > @@ -91,26 +150,46 @@ __new_semctl (int semid, int semnum, int cmd, > > > > ...) } > > > > #endif > > > > > > > > - int ret = semctl_syscall (semid, semnum, cmd, arg); > > > > + int ret = semctl_syscall (semid, semnum, cmd & > > > > ~__IPC_CMD_TIME64, arg); > > > > + if (ret < 0) > > > > + return ret; > > > > > > > > - if (ret >= 0) > > > > + switch (cmd & ~__IPC_CMD_TIME64) > > > > { > > > > - switch (cmd) > > > > - { > > > > - case IPC_STAT: > > > > - case SEM_STAT: > > > > - case SEM_STAT_ANY: > > > > + case IPC_STAT: > > > > + case SEM_STAT: > > > > + case SEM_STAT_ANY: > > > > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > > > > - arg.buf->sem_perm.mode >>= 16; > > > > +# if __IPC_TIME64 > > > > + arg.buf32->sem_perm.mode >>= 16; > > > > +# else > > > > + arg.buf->sem_perm.mode >>= 16; > > > > +# endif > > > > #else > > > > - /* Old Linux kernel versions might not clear the mode > > > > padding. */ > > > > - if (sizeof ((struct semid_ds){0}.sem_perm.mode) > > > > - != sizeof (__kernel_mode_t)) > > > > - arg.buf->sem_perm.mode &= 0xFFFF; > > > > + /* Old Linux kernel versions might not clear the mode > > > > padding. */ > > > > + if (sizeof ((struct semid_ds){0}.sem_perm.mode) > > > > + != sizeof (__kernel_mode_t)) > > > > +# if __IPC_TIME64 > > > > + arg.buf32->sem_perm.mode &= 0xFFFF; > > > > +# else > > > > + arg.buf->sem_perm.mode &= 0xFFFF; > > > > +# endif > > > > #endif > > > > - } > > > > } > > > > > > > > +#ifdef __IPC_TIME64 > > > > + if (restore) > > > > + { > > > > + orig64->sem_perm = tmp32.sem_perm; > > > > + orig64->sem_otime = tmp32.sem_otime > > > > + | ((__time64_t) tmp32.sem_otime_high << > > > > 32); > > > > + orig64->sem_ctime = tmp32.sem_ctime > > > > + | ((__time64_t) tmp32.sem_ctime_high << > > > > 32); > > > > + orig64->sem_nsems = tmp32.sem_nsems; > > > > + arg.buf = (struct semid_ds *) orig64; > > > > + } > > > > +#endif > > > > + > > > > return ret; > > > > } > > > > versioned_symbol (libc, __new_semctl, semctl, DEFAULT_VERSION); > > > > diff --git a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h > > > > b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h index > > > > 12e4cd7682..aa56da6cd5 100644 --- > > > > a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h +++ > > > > b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h @@ -23,6 +23,8 @@ > > > > #ifndef _BITS_IPCTYPES_H > > > > #define _BITS_IPCTYPES_H 1 > > > > > > > > +#include <bits/types.h> > > > > + > > > > /* Used in `struct shmid_ds'. */ > > > > # ifdef __x86_64__ > > > > typedef int __ipc_pid_t; > > > > @@ -30,4 +32,15 @@ typedef int __ipc_pid_t; > > > > typedef unsigned short int __ipc_pid_t; > > > > # endif > > > > > > > > +#if __WORDSIZE == 64 \ > > > > + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) > > > > +# define __IPC_CMD_TIME64 0x0 > > > > +#else > > > > +# if __TIMESIZE == 64 > > > > +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ > > > > +# else > > > > +# define __IPC_CMD_TIME64 0x0 > > > > +# endif > > > > +#endif > > > > + > > > > #endif /* bits/ipctypes.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 > > > > > 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 Wed, 6 May 2020, Alistair Francis via Libc-alpha wrote: > > But for ARM 32 the __TIMESIZE == 32, so will it be possible to have 64 > > bit time for this case? > > I thought you would override the __TIMESIZE. __TIMESIZE is always the size of time_t in the default ABI, which is not affected by _TIME_BITS. When we support defining _TIME_BITS to change the size of time_t for a particular compilation, there will need to be another macro reflecting the actual size of time_t for the current compilation, and some installed headers will need conditionals on that macro.
Hi Joseph, Alistair, > On Wed, 6 May 2020, Alistair Francis via Libc-alpha wrote: > > > > But for ARM 32 the __TIMESIZE == 32, so will it be possible to > > > have 64 bit time for this case? > > > > I thought you would override the __TIMESIZE. > > __TIMESIZE is always the size of time_t in the default ABI, which is > not affected by _TIME_BITS. This means that for ARM32 bit architecture (__WORDSIZE == 32 && __TIMESIZE != 64)) the __TIMESIZE == 32 with and without enabled support for 64 bit time. > When we support defining _TIME_BITS to > change the size of time_t for a particular compilation, there will > need to be another macro reflecting the actual size of time_t In my development/testing code there is: # define __USE_TIME_BITS64 1 as in [1]. > for the > current compilation, and some installed headers will need > conditionals on that macro. > Please refer to [2] Link: [1] - https://github.com/lmajewski/y2038_glibc/commit/0f65e19bb9b9bd55007d042a05daee61e461eef9#diff-a5ab6c74681eaf0569ed54f6bf0d7978R391 [2] - https://github.com/lmajewski/y2038_glibc/commit/0f65e19bb9b9bd55007d042a05daee61e461eef9 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 Wed, May 6, 2020 at 11:31 AM Lukasz Majewski <lukma@denx.de> wrote: > > Hi Joseph, Alistair, > > > On Wed, 6 May 2020, Alistair Francis via Libc-alpha wrote: > > > > > > But for ARM 32 the __TIMESIZE == 32, so will it be possible to > > > > have 64 bit time for this case? > > > > > > I thought you would override the __TIMESIZE. > > > > __TIMESIZE is always the size of time_t in the default ABI, which is > > not affected by _TIME_BITS. > > This means that for ARM32 bit architecture (__WORDSIZE == 32 && > __TIMESIZE != 64)) the __TIMESIZE == 32 with and without enabled support > for 64 bit time. > > > When we support defining _TIME_BITS to > > change the size of time_t for a particular compilation, there will > > need to be another macro reflecting the actual size of time_t > > In my development/testing code there is: > # define __USE_TIME_BITS64 1 > > as in [1]. Ah ok. It should just be as simple as changing the define for __IPC_TIME64 and __IPC_CMD_TIME64 to be based on _TIME_BITS == 64 or __USE_TIME_BITS64 being defined. Alistair > > > for the > > current compilation, and some installed headers will need > > conditionals on that macro. > > > > Please refer to [2] > > Link: > [1] - > https://github.com/lmajewski/y2038_glibc/commit/0f65e19bb9b9bd55007d042a05daee61e461eef9#diff-a5ab6c74681eaf0569ed54f6bf0d7978R391 > > [2] - > https://github.com/lmajewski/y2038_glibc/commit/0f65e19bb9b9bd55007d042a05daee61e461eef9 > > 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 Wed, May 6, 2020 at 11:31 AM Lukasz Majewski <lukma@denx.de> wrote: > > > > Hi Joseph, Alistair, > > > > > On Wed, 6 May 2020, Alistair Francis via Libc-alpha wrote: > > > > > > > > But for ARM 32 the __TIMESIZE == 32, so will it be possible to > > > > > have 64 bit time for this case? > > > > > > > > I thought you would override the __TIMESIZE. > > > > > > __TIMESIZE is always the size of time_t in the default ABI, which > > > is not affected by _TIME_BITS. > > > > This means that for ARM32 bit architecture (__WORDSIZE == 32 && > > __TIMESIZE != 64)) the __TIMESIZE == 32 with and without enabled > > support for 64 bit time. > > > > > When we support defining _TIME_BITS to > > > change the size of time_t for a particular compilation, there will > > > need to be another macro reflecting the actual size of time_t > > > > In my development/testing code there is: > > # define __USE_TIME_BITS64 1 > > > > as in [1]. > > Ah ok. > > It should just be as simple as changing the define for __IPC_TIME64 > and __IPC_CMD_TIME64 to be based on _TIME_BITS == 64 or > __USE_TIME_BITS64 being defined. I see. Thanks for the info. > > Alistair > > > > > > for the > > > current compilation, and some installed headers will need > > > conditionals on that macro. > > > > > > > Please refer to [2] > > > > Link: > > [1] - > > https://github.com/lmajewski/y2038_glibc/commit/0f65e19bb9b9bd55007d042a05daee61e461eef9#diff-a5ab6c74681eaf0569ed54f6bf0d7978R391 > > > > [2] - > > https://github.com/lmajewski/y2038_glibc/commit/0f65e19bb9b9bd55007d042a05daee61e461eef9 > > > > 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 Wed, May 06, 2020 at 04:44:21PM -0700, Alistair Francis wrote: > On Wed, May 6, 2020 at 11:31 AM Lukasz Majewski <lukma@denx.de> wrote: > > > > Hi Joseph, Alistair, > > > > > On Wed, 6 May 2020, Alistair Francis via Libc-alpha wrote: > > > > > > > > But for ARM 32 the __TIMESIZE == 32, so will it be possible to > > > > > have 64 bit time for this case? > > > > > > > > I thought you would override the __TIMESIZE. > > > > > > __TIMESIZE is always the size of time_t in the default ABI, which is > > > not affected by _TIME_BITS. > > > > This means that for ARM32 bit architecture (__WORDSIZE == 32 && > > __TIMESIZE != 64)) the __TIMESIZE == 32 with and without enabled support > > for 64 bit time. > > > > > When we support defining _TIME_BITS to > > > change the size of time_t for a particular compilation, there will > > > need to be another macro reflecting the actual size of time_t > > > > In my development/testing code there is: > > # define __USE_TIME_BITS64 1 > > > > as in [1]. > > Ah ok. > > It should just be as simple as changing the define for __IPC_TIME64 > and __IPC_CMD_TIME64 to be based on _TIME_BITS == 64 or > __USE_TIME_BITS64 being defined. __IPC_TIME64 cannot be based on _TIME_BITS or __USE_TIME_BITS64 as those won't be known at glibc compile time. __IPC_TIME64 needs to be true on all __TIME_SIZE==32 architectetures. And should semctl have new symbol version if it accepts __IPC_CMD_TIME64?
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. (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.) > --- > bits/ipctypes.h | 10 ++ > sysdeps/mips/bits/ipctypes.h | 10 ++ > sysdeps/unix/sysv/linux/bits/ipc.h | 3 +- > sysdeps/unix/sysv/linux/bits/sem.h | 4 +- > sysdeps/unix/sysv/linux/ipc_priv.h | 4 + > sysdeps/unix/sysv/linux/semctl.c | 121 ++++++++++++++++---- > sysdeps/unix/sysv/linux/x86/bits/ipctypes.h | 13 +++ > 7 files changed, 141 insertions(+), 24 deletions(-) > diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c > index 30571af49f..99ad686194 100644 > --- a/sysdeps/unix/sysv/linux/semctl.c > +++ b/sysdeps/unix/sysv/linux/semctl.c > @@ -22,12 +22,16 @@ > #include <sysdep.h> > #include <shlib-compat.h> > #include <errno.h> > +#include <struct__semid_ds32.h> > #include <linux/posix_types.h> /* For __kernel_mode_t. */ > > /* Define a `union semun' suitable for Linux here. */ > union semun > { > int val; /* value for SETVAL */ > +#if __IPC_TIME64 > + struct __semid_ds32 *buf32; > +#endif > struct semid_ds *buf; /* buffer for IPC_STAT & IPC_SET */ > unsigned short int *array; /* array for GETALL & SETALL */ > struct seminfo *__buf; /* buffer for IPC_INFO */ > @@ -44,13 +48,54 @@ union semun > static int > semctl_syscall (int semid, int semnum, int cmd, union semun arg) > { > + int ret; > +#if __IPC_TIME64 > + /* A temporary buffer is used to avoid both an issue where the export > + semid_ds might not follow the kernel's expected layout (due > + ? to {o,c}time{_high} alignment in 64-bit time case) and the issue where > + ? ?some kernel versions might not clear the high bits when returning > + ? ?then {o,c}time{_high} information. */ > + struct __semid_ds32 tmp; > + struct semid_ds *orig; > + bool restore = false; > + if (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY) > + { > + tmp = (struct __semid_ds32) { > + .sem_perm = arg.buf->sem_perm, > + .sem_otime = arg.buf->sem_otime, > + .sem_otime_high = arg.buf->sem_otime >> 32, > + .sem_ctime = arg.buf->sem_ctime, > + .sem_ctime_high = arg.buf->sem_ctime >> 32, > + .sem_nsems = arg.buf->sem_nsems, > + }; sem_otime_high and sem_ctime_high need to be 0 for compatibility with older kernels. And it seems this struct is write-only in kernel, so it does not not matter what's in the other fields. But it looks like all the changes to semctl_syscall function are junk from old patch versions as necessary conversions are added to the caller. > + orig = arg.buf; > + arg.buf = (struct semid_ds*) &tmp; > + restore = true; > + } > +#endif > + > #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS > - return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64, > - arg.array); > + ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64, > + arg.array); > #else > - return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64, > - SEMCTL_ARG_ADDRESS (arg)); > + ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64, > + SEMCTL_ARG_ADDRESS (arg)); > #endif > + > +#if __IPC_TIME64 > + if (ret >= 0 && restore) > + { > + arg.buf = orig; > + arg.buf->sem_perm = tmp.sem_perm; > + arg.buf->sem_otime = tmp.sem_otime > + | ((__time64_t) tmp.sem_otime_high << 32); > + arg.buf->sem_ctime = tmp.sem_ctime > + | ((__time64_t) tmp.sem_ctime_high << 32); > + arg.buf->sem_nsems = tmp.sem_nsems; > + } > +#endif > + > + return ret; > } > > int > @@ -64,15 +109,15 @@ __new_semctl (int semid, int semnum, int cmd, ...) > union semun arg = { 0 }; > va_list ap; > > - /* Get the argument only if required. */ > - switch (cmd) > + /* Get the argument only if required. Ignore the __IPC_CMD_TIME64. */ > + switch (cmd & ~__IPC_CMD_TIME64) > { > case SETVAL: /* arg.val */ > case GETALL: /* arg.array */ > case SETALL: > - case IPC_STAT: /* arg.buf */ > + case IPC_STAT & ~__IPC_CMD_TIME64: /* arg.buf */ > case IPC_SET: > - case SEM_STAT: > + case SEM_STAT & ~__IPC_CMD_TIME64: > case IPC_INFO: /* arg.__buf */ > case SEM_INFO: > va_start (ap, cmd); > @@ -81,6 +126,20 @@ __new_semctl (int semid, int semnum, int cmd, ...) > break; > } > > +#ifdef __IPC_TIME64 s/ifdef/if/ here and later. > + struct __semid_ds32 tmp32; > + struct semid_ds *orig64; > + bool restore = false; > + > + if (cmd & __IPC_CMD_TIME64) > + { > + tmp32 = (struct __semid_ds32) { 0 }; > + orig64 = (struct semid_ds *) arg.buf; > + arg.buf = (struct semid_ds *) &tmp32; > + restore = true; > + } > +#endif > + > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > struct semid_ds tmpds; > if (cmd == IPC_SET) > @@ -91,26 +150,46 @@ __new_semctl (int semid, int semnum, int cmd, ...) > } > #endif > > - int ret = semctl_syscall (semid, semnum, cmd, arg); > + int ret = semctl_syscall (semid, semnum, cmd & ~__IPC_CMD_TIME64, arg); > + if (ret < 0) > + return ret; > > - if (ret >= 0) > + switch (cmd & ~__IPC_CMD_TIME64) > { > - switch (cmd) > - { > - case IPC_STAT: > - case SEM_STAT: > - case SEM_STAT_ANY: > + case IPC_STAT: > + case SEM_STAT: > + case SEM_STAT_ANY: > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > - arg.buf->sem_perm.mode >>= 16; > +# if __IPC_TIME64 > + arg.buf32->sem_perm.mode >>= 16; > +# else > + arg.buf->sem_perm.mode >>= 16; > +# endif > #else > - /* Old Linux kernel versions might not clear the mode padding. */ > - if (sizeof ((struct semid_ds){0}.sem_perm.mode) > - != sizeof (__kernel_mode_t)) > - arg.buf->sem_perm.mode &= 0xFFFF; > + /* Old Linux kernel versions might not clear the mode padding. */ > + if (sizeof ((struct semid_ds){0}.sem_perm.mode) > + != sizeof (__kernel_mode_t)) > +# if __IPC_TIME64 > + arg.buf32->sem_perm.mode &= 0xFFFF; > +# else > + arg.buf->sem_perm.mode &= 0xFFFF; > +# endif > #endif > - } > } > > +#ifdef __IPC_TIME64 > + if (restore) > + { > + orig64->sem_perm = tmp32.sem_perm; > + orig64->sem_otime = tmp32.sem_otime > + | ((__time64_t) tmp32.sem_otime_high << 32); > + orig64->sem_ctime = tmp32.sem_ctime > + | ((__time64_t) tmp32.sem_ctime_high << 32); > + orig64->sem_nsems = tmp32.sem_nsems; > + arg.buf = (struct semid_ds *) orig64; > + } > +#endif > + > return ret; > } > versioned_symbol (libc, __new_semctl, semctl, DEFAULT_VERSION);
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. 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? > > (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. > > > --- > > bits/ipctypes.h | 10 ++ > > sysdeps/mips/bits/ipctypes.h | 10 ++ > > sysdeps/unix/sysv/linux/bits/ipc.h | 3 +- > > sysdeps/unix/sysv/linux/bits/sem.h | 4 +- > > sysdeps/unix/sysv/linux/ipc_priv.h | 4 + > > sysdeps/unix/sysv/linux/semctl.c | 121 ++++++++++++++++---- > > sysdeps/unix/sysv/linux/x86/bits/ipctypes.h | 13 +++ > > 7 files changed, 141 insertions(+), 24 deletions(-) > > > diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c > > index 30571af49f..99ad686194 100644 > > --- a/sysdeps/unix/sysv/linux/semctl.c > > +++ b/sysdeps/unix/sysv/linux/semctl.c > > @@ -22,12 +22,16 @@ > > #include <sysdep.h> > > #include <shlib-compat.h> > > #include <errno.h> > > +#include <struct__semid_ds32.h> > > #include <linux/posix_types.h> /* For __kernel_mode_t. */ > > > > /* Define a `union semun' suitable for Linux here. */ > > union semun > > { > > int val; /* value for SETVAL */ > > +#if __IPC_TIME64 > > + struct __semid_ds32 *buf32; > > +#endif > > struct semid_ds *buf; /* buffer for IPC_STAT & IPC_SET */ > > unsigned short int *array; /* array for GETALL & SETALL */ > > struct seminfo *__buf; /* buffer for IPC_INFO */ > > @@ -44,13 +48,54 @@ union semun > > static int > > semctl_syscall (int semid, int semnum, int cmd, union semun arg) > > { > > + int ret; > > +#if __IPC_TIME64 > > + /* A temporary buffer is used to avoid both an issue where the export > > + semid_ds might not follow the kernel's expected layout (due > > + ? to {o,c}time{_high} alignment in 64-bit time case) and the issue where > > + ? ?some kernel versions might not clear the high bits when returning > > + ? ?then {o,c}time{_high} information. */ > > + struct __semid_ds32 tmp; > > + struct semid_ds *orig; > > + bool restore = false; > > + if (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY) > > + { > > + tmp = (struct __semid_ds32) { > > + .sem_perm = arg.buf->sem_perm, > > + .sem_otime = arg.buf->sem_otime, > > + .sem_otime_high = arg.buf->sem_otime >> 32, > > + .sem_ctime = arg.buf->sem_ctime, > > + .sem_ctime_high = arg.buf->sem_ctime >> 32, > > + .sem_nsems = arg.buf->sem_nsems, > > + }; > > sem_otime_high and sem_ctime_high need to be 0 for compatibility with > older kernels. And it seems this struct is write-only in kernel, so > it does not not matter what's in the other fields. > > But it looks like all the changes to semctl_syscall function are junk > from old patch versions as necessary conversions are added to the > caller. Yep, I have remove this. > > > + orig = arg.buf; > > + arg.buf = (struct semid_ds*) &tmp; > > + restore = true; > > + } > > +#endif > > + > > #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS > > - return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64, > > - arg.array); > > + ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64, > > + arg.array); > > #else > > - return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64, > > - SEMCTL_ARG_ADDRESS (arg)); > > + ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64, > > + SEMCTL_ARG_ADDRESS (arg)); > > #endif > > + > > +#if __IPC_TIME64 > > + if (ret >= 0 && restore) > > + { > > + arg.buf = orig; > > + arg.buf->sem_perm = tmp.sem_perm; > > + arg.buf->sem_otime = tmp.sem_otime > > + | ((__time64_t) tmp.sem_otime_high << 32); > > + arg.buf->sem_ctime = tmp.sem_ctime > > + | ((__time64_t) tmp.sem_ctime_high << 32); > > + arg.buf->sem_nsems = tmp.sem_nsems; > > + } > > +#endif > > + > > + return ret; > > } > > > > int > > @@ -64,15 +109,15 @@ __new_semctl (int semid, int semnum, int cmd, ...) > > union semun arg = { 0 }; > > va_list ap; > > > > - /* Get the argument only if required. */ > > - switch (cmd) > > + /* Get the argument only if required. Ignore the __IPC_CMD_TIME64. */ > > + switch (cmd & ~__IPC_CMD_TIME64) > > { > > case SETVAL: /* arg.val */ > > case GETALL: /* arg.array */ > > case SETALL: > > - case IPC_STAT: /* arg.buf */ > > + case IPC_STAT & ~__IPC_CMD_TIME64: /* arg.buf */ > > case IPC_SET: > > - case SEM_STAT: > > + case SEM_STAT & ~__IPC_CMD_TIME64: > > case IPC_INFO: /* arg.__buf */ > > case SEM_INFO: > > va_start (ap, cmd); > > @@ -81,6 +126,20 @@ __new_semctl (int semid, int semnum, int cmd, ...) > > break; > > } > > > > +#ifdef __IPC_TIME64 > > s/ifdef/if/ here and later. Good catch, fixed. Alistair > > > > + struct __semid_ds32 tmp32; > > + struct semid_ds *orig64; > > + bool restore = false; > > + > > + if (cmd & __IPC_CMD_TIME64) > > + { > > + tmp32 = (struct __semid_ds32) { 0 }; > > + orig64 = (struct semid_ds *) arg.buf; > > + arg.buf = (struct semid_ds *) &tmp32; > > + restore = true; > > + } > > +#endif > > + > > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > > struct semid_ds tmpds; > > if (cmd == IPC_SET) > > @@ -91,26 +150,46 @@ __new_semctl (int semid, int semnum, int cmd, ...) > > } > > #endif > > > > - int ret = semctl_syscall (semid, semnum, cmd, arg); > > + int ret = semctl_syscall (semid, semnum, cmd & ~__IPC_CMD_TIME64, arg); > > + if (ret < 0) > > + return ret; > > > > - if (ret >= 0) > > + switch (cmd & ~__IPC_CMD_TIME64) > > { > > - switch (cmd) > > - { > > - case IPC_STAT: > > - case SEM_STAT: > > - case SEM_STAT_ANY: > > + case IPC_STAT: > > + case SEM_STAT: > > + case SEM_STAT_ANY: > > #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T > > - arg.buf->sem_perm.mode >>= 16; > > +# if __IPC_TIME64 > > + arg.buf32->sem_perm.mode >>= 16; > > +# else > > + arg.buf->sem_perm.mode >>= 16; > > +# endif > > #else > > - /* Old Linux kernel versions might not clear the mode padding. */ > > - if (sizeof ((struct semid_ds){0}.sem_perm.mode) > > - != sizeof (__kernel_mode_t)) > > - arg.buf->sem_perm.mode &= 0xFFFF; > > + /* Old Linux kernel versions might not clear the mode padding. */ > > + if (sizeof ((struct semid_ds){0}.sem_perm.mode) > > + != sizeof (__kernel_mode_t)) > > +# if __IPC_TIME64 > > + arg.buf32->sem_perm.mode &= 0xFFFF; > > +# else > > + arg.buf->sem_perm.mode &= 0xFFFF; > > +# endif > > #endif > > - } > > } > > > > +#ifdef __IPC_TIME64 > > + if (restore) > > + { > > + orig64->sem_perm = tmp32.sem_perm; > > + orig64->sem_otime = tmp32.sem_otime > > + | ((__time64_t) tmp32.sem_otime_high << 32); > > + orig64->sem_ctime = tmp32.sem_ctime > > + | ((__time64_t) tmp32.sem_ctime_high << 32); > > + orig64->sem_nsems = tmp32.sem_nsems; > > + arg.buf = (struct semid_ds *) orig64; > > + } > > +#endif > > + > > return ret; > > } > > versioned_symbol (libc, __new_semctl, semctl, DEFAULT_VERSION);
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): 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: 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.)
On 10/05/2020 11:52, Stepan Golosunov 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): This is the suggestion of adding new time64 SysVIPC symbols I outlined as an options in a previous review. I don't a a strong opinion which options would be better, but the one you outlined here seems to align better with other time64 symbols. > > 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. In fact we should follow other time64 and do on __semid_ds64: #if __TIMESIZE == 64 # define __semid_ds64 semid_ds #else struct __semid_ds64 { [...] __time64_t sem_otime; __time64_t sem_ctime; [...] }; #endif This will require to remove some of the __TIMESIZE adjustments in the architecture defined struct_semid.h and add arch-specific semid64.h headers. > 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). The __semctl64 will still need to handle the semid64_ds alignment issue if it differs from kernel one. With semid64_ds being always defined we should use the temporary buffer for stat operations iff: offsetof (struct semid_ds, sem_otime) != offsetof (struct semid64_ds, sem_otime) || offsetof (struct semid_ds, sem_ctime) != offsetof (struct semid64_ds, sem_ctime) > 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: Indeed this approach seems more streamline with current y2038 work. > > 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.) >
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
diff --git a/bits/ipctypes.h b/bits/ipctypes.h index a955cdda7f..1b94122c32 100644 --- a/bits/ipctypes.h +++ b/bits/ipctypes.h @@ -32,5 +32,15 @@ typedef unsigned short int __ipc_pid_t; typedef int __ipc_pid_t; # endif +#if __WORDSIZE == 64 \ + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) +# define __IPC_CMD_TIME64 0x0 +#else +# if __TIMESIZE == 64 +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ +# else +# define __IPC_CMD_TIME64 0x0 +# endif +#endif #endif /* bits/ipctypes.h */ diff --git a/sysdeps/mips/bits/ipctypes.h b/sysdeps/mips/bits/ipctypes.h index 518f579b36..9a7218bf7d 100644 --- a/sysdeps/mips/bits/ipctypes.h +++ b/sysdeps/mips/bits/ipctypes.h @@ -27,5 +27,15 @@ typedef __SLONG32_TYPE __ipc_pid_t; +#if __WORDSIZE == 64 \ + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) +# define __IPC_CMD_TIME64 0x0 +#else +# if __TIMESIZE == 64 +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ +# else +# define __IPC_CMD_TIME64 0x0 +# endif +#endif #endif /* bits/ipctypes.h */ diff --git a/sysdeps/unix/sysv/linux/bits/ipc.h b/sysdeps/unix/sysv/linux/bits/ipc.h index 085dd628ac..2519322d16 100644 --- a/sysdeps/unix/sysv/linux/bits/ipc.h +++ b/sysdeps/unix/sysv/linux/bits/ipc.h @@ -20,6 +20,7 @@ #endif #include <bits/types.h> +#include <bits/ipctypes.h> /* Mode bits for `msgget', `semget', and `shmget'. */ #define IPC_CREAT 01000 /* Create key if key does not exist. */ @@ -29,7 +30,7 @@ /* Control commands for `msgctl', `semctl', and `shmctl'. */ #define IPC_RMID 0 /* Remove identifier. */ #define IPC_SET 1 /* Set `ipc_perm' options. */ -#define IPC_STAT 2 /* Get `ipc_perm' options. */ +#define IPC_STAT (2 | __IPC_CMD_TIME64) /* Get `ipc_perm' options. */ #ifdef __USE_GNU # define IPC_INFO 3 /* See ipcs. */ #endif diff --git a/sysdeps/unix/sysv/linux/bits/sem.h b/sysdeps/unix/sysv/linux/bits/sem.h index ba1169fdb3..8d0b88f5e0 100644 --- a/sysdeps/unix/sysv/linux/bits/sem.h +++ b/sysdeps/unix/sysv/linux/bits/sem.h @@ -54,9 +54,9 @@ #ifdef __USE_MISC /* ipcs ctl cmds */ -# define SEM_STAT 18 +# define SEM_STAT (18 | __IPC_CMD_TIME64) # define SEM_INFO 19 -# define SEM_STAT_ANY 20 +# define SEM_STAT_ANY (20 | __IPC_CMD_TIME64) struct seminfo { diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h index 15a6e683a4..a1a7cacd17 100644 --- a/sysdeps/unix/sysv/linux/ipc_priv.h +++ b/sysdeps/unix/sysv/linux/ipc_priv.h @@ -43,6 +43,10 @@ struct __old_ipc_perm unsigned short int __seq; /* Sequence number. */ }; +#define __IPC_TIME64 \ + (__WORDSIZE == 32 && __TIMESIZE == 64 \ + && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) + #define SEMCTL_ARG_ADDRESS(__arg) &__arg.array #define MSGRCV_ARGS(__msgp, __msgtyp) \ diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c index 30571af49f..99ad686194 100644 --- a/sysdeps/unix/sysv/linux/semctl.c +++ b/sysdeps/unix/sysv/linux/semctl.c @@ -22,12 +22,16 @@ #include <sysdep.h> #include <shlib-compat.h> #include <errno.h> +#include <struct__semid_ds32.h> #include <linux/posix_types.h> /* For __kernel_mode_t. */ /* Define a `union semun' suitable for Linux here. */ union semun { int val; /* value for SETVAL */ +#if __IPC_TIME64 + struct __semid_ds32 *buf32; +#endif struct semid_ds *buf; /* buffer for IPC_STAT & IPC_SET */ unsigned short int *array; /* array for GETALL & SETALL */ struct seminfo *__buf; /* buffer for IPC_INFO */ @@ -44,13 +48,54 @@ union semun static int semctl_syscall (int semid, int semnum, int cmd, union semun arg) { + int ret; +#if __IPC_TIME64 + /* A temporary buffer is used to avoid both an issue where the export + semid_ds might not follow the kernel's expected layout (due + to {o,c}time{_high} alignment in 64-bit time case) and the issue where + some kernel versions might not clear the high bits when returning + then {o,c}time{_high} information. */ + struct __semid_ds32 tmp; + struct semid_ds *orig; + bool restore = false; + if (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY) + { + tmp = (struct __semid_ds32) { + .sem_perm = arg.buf->sem_perm, + .sem_otime = arg.buf->sem_otime, + .sem_otime_high = arg.buf->sem_otime >> 32, + .sem_ctime = arg.buf->sem_ctime, + .sem_ctime_high = arg.buf->sem_ctime >> 32, + .sem_nsems = arg.buf->sem_nsems, + }; + orig = arg.buf; + arg.buf = (struct semid_ds*) &tmp; + restore = true; + } +#endif + #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS - return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64, - arg.array); + ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64, + arg.array); #else - return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64, - SEMCTL_ARG_ADDRESS (arg)); + ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64, + SEMCTL_ARG_ADDRESS (arg)); #endif + +#if __IPC_TIME64 + if (ret >= 0 && restore) + { + arg.buf = orig; + arg.buf->sem_perm = tmp.sem_perm; + arg.buf->sem_otime = tmp.sem_otime + | ((__time64_t) tmp.sem_otime_high << 32); + arg.buf->sem_ctime = tmp.sem_ctime + | ((__time64_t) tmp.sem_ctime_high << 32); + arg.buf->sem_nsems = tmp.sem_nsems; + } +#endif + + return ret; } int @@ -64,15 +109,15 @@ __new_semctl (int semid, int semnum, int cmd, ...) union semun arg = { 0 }; va_list ap; - /* Get the argument only if required. */ - switch (cmd) + /* Get the argument only if required. Ignore the __IPC_CMD_TIME64. */ + switch (cmd & ~__IPC_CMD_TIME64) { case SETVAL: /* arg.val */ case GETALL: /* arg.array */ case SETALL: - case IPC_STAT: /* arg.buf */ + case IPC_STAT & ~__IPC_CMD_TIME64: /* arg.buf */ case IPC_SET: - case SEM_STAT: + case SEM_STAT & ~__IPC_CMD_TIME64: case IPC_INFO: /* arg.__buf */ case SEM_INFO: va_start (ap, cmd); @@ -81,6 +126,20 @@ __new_semctl (int semid, int semnum, int cmd, ...) break; } +#ifdef __IPC_TIME64 + struct __semid_ds32 tmp32; + struct semid_ds *orig64; + bool restore = false; + + if (cmd & __IPC_CMD_TIME64) + { + tmp32 = (struct __semid_ds32) { 0 }; + orig64 = (struct semid_ds *) arg.buf; + arg.buf = (struct semid_ds *) &tmp32; + restore = true; + } +#endif + #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T struct semid_ds tmpds; if (cmd == IPC_SET) @@ -91,26 +150,46 @@ __new_semctl (int semid, int semnum, int cmd, ...) } #endif - int ret = semctl_syscall (semid, semnum, cmd, arg); + int ret = semctl_syscall (semid, semnum, cmd & ~__IPC_CMD_TIME64, arg); + if (ret < 0) + return ret; - if (ret >= 0) + switch (cmd & ~__IPC_CMD_TIME64) { - switch (cmd) - { - case IPC_STAT: - case SEM_STAT: - case SEM_STAT_ANY: + case IPC_STAT: + case SEM_STAT: + case SEM_STAT_ANY: #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T - arg.buf->sem_perm.mode >>= 16; +# if __IPC_TIME64 + arg.buf32->sem_perm.mode >>= 16; +# else + arg.buf->sem_perm.mode >>= 16; +# endif #else - /* Old Linux kernel versions might not clear the mode padding. */ - if (sizeof ((struct semid_ds){0}.sem_perm.mode) - != sizeof (__kernel_mode_t)) - arg.buf->sem_perm.mode &= 0xFFFF; + /* Old Linux kernel versions might not clear the mode padding. */ + if (sizeof ((struct semid_ds){0}.sem_perm.mode) + != sizeof (__kernel_mode_t)) +# if __IPC_TIME64 + arg.buf32->sem_perm.mode &= 0xFFFF; +# else + arg.buf->sem_perm.mode &= 0xFFFF; +# endif #endif - } } +#ifdef __IPC_TIME64 + if (restore) + { + orig64->sem_perm = tmp32.sem_perm; + orig64->sem_otime = tmp32.sem_otime + | ((__time64_t) tmp32.sem_otime_high << 32); + orig64->sem_ctime = tmp32.sem_ctime + | ((__time64_t) tmp32.sem_ctime_high << 32); + orig64->sem_nsems = tmp32.sem_nsems; + arg.buf = (struct semid_ds *) orig64; + } +#endif + return ret; } versioned_symbol (libc, __new_semctl, semctl, DEFAULT_VERSION); diff --git a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h index 12e4cd7682..aa56da6cd5 100644 --- a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h +++ b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h @@ -23,6 +23,8 @@ #ifndef _BITS_IPCTYPES_H #define _BITS_IPCTYPES_H 1 +#include <bits/types.h> + /* Used in `struct shmid_ds'. */ # ifdef __x86_64__ typedef int __ipc_pid_t; @@ -30,4 +32,15 @@ typedef int __ipc_pid_t; typedef unsigned short int __ipc_pid_t; # endif +#if __WORDSIZE == 64 \ + || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) +# define __IPC_CMD_TIME64 0x0 +#else +# if __TIMESIZE == 64 +# define __IPC_CMD_TIME64 0x100 /* Same as __IPC_64. */ +# else +# define __IPC_CMD_TIME64 0x0 +# endif +#endif + #endif /* bits/ipctypes.h */
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). --- bits/ipctypes.h | 10 ++ sysdeps/mips/bits/ipctypes.h | 10 ++ sysdeps/unix/sysv/linux/bits/ipc.h | 3 +- sysdeps/unix/sysv/linux/bits/sem.h | 4 +- sysdeps/unix/sysv/linux/ipc_priv.h | 4 + sysdeps/unix/sysv/linux/semctl.c | 121 ++++++++++++++++---- sysdeps/unix/sysv/linux/x86/bits/ipctypes.h | 13 +++ 7 files changed, 141 insertions(+), 24 deletions(-)