unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] sysv: linux: Pass 64-bit version of semctl syscall
@ 2020-02-05  0:56 Alistair Francis
  2020-02-14 16:39 ` Alistair Francis
  2020-02-21 14:03 ` Adhemerval Zanella
  0 siblings, 2 replies; 5+ messages in thread
From: Alistair Francis @ 2020-02-05  0:56 UTC (permalink / raw)
  To: libc-alpha; +Cc: alistair23, Alistair Francis

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 union semun to include a struct __semid_ds32
which supports 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.
---
v2:
 - Fix the HPPA error
 - Simplfy the code changes

 include/sys/sem.h                             | 35 +++++++++++++++++++
 sysdeps/unix/sysv/linux/bits/sem-pad.h        |  1 +
 sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h   |  2 ++
 sysdeps/unix/sysv/linux/mips/bits/sem-pad.h   |  2 ++
 .../unix/sysv/linux/powerpc/bits/sem-pad.h    |  2 ++
 sysdeps/unix/sysv/linux/semctl.c              | 23 +++++++++---
 sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h  |  2 ++
 sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    |  1 +
 8 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/include/sys/sem.h b/include/sys/sem.h
index 69fdf1f752..70b83127cb 100644
--- a/include/sys/sem.h
+++ b/include/sys/sem.h
@@ -5,5 +5,40 @@
 
 __typeof__ (semtimedop) __semtimedop attribute_hidden;
 
+# endif
+
+# ifdef __SEMID_DS_HIGH
+#  if defined (__SEMID_DS_HIGH_END)
+struct __semid_ds32 {
+  struct ipc_perm sem_perm;             /* permissions .. see ipc.h */
+  __syscall_ulong_t   sem_otime;          /* last semop time */
+  __syscall_ulong_t   sem_ctime;          /* last change time */
+  __syscall_ulong_t   sem_nsems;          /* no. of semaphores in array */
+  __syscall_ulong_t   sem_otime_high;
+  __syscall_ulong_t   sem_ctime_high;
+};
+#  elif defined (__SEMID_DS_HIGH_SWAP)
+struct __semid_ds32 {
+  struct ipc_perm sem_perm;              /* operation permission struct */
+  __syscall_ulong_t   sem_otime_high;    /* last semop() time high */
+  __syscall_ulong_t   sem_otime;         /* last semop() time */
+  __syscall_ulong_t   sem_ctime_high;    /* last time changed by semctl() high */
+  __syscall_ulong_t   sem_ctime;         /* last time changed by semctl() */
+  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
+  __syscall_ulong_t   __glibc_reserved3;
+  __syscall_ulong_t   __glibc_reserved4;
+};
+#  else
+struct __semid_ds32 {
+  struct ipc_perm sem_perm;              /* operation permission struct */
+  __syscall_ulong_t   sem_otime;         /* last semop() time */
+  __syscall_ulong_t   sem_otime_high;    /* last semop() time high */
+  __syscall_ulong_t   sem_ctime;         /* last time changed by semctl() */
+  __syscall_ulong_t   sem_ctime_high;    /* last time changed by semctl() high */
+  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
+  __syscall_ulong_t   __glibc_reserved3;
+  __syscall_ulong_t   __glibc_reserved4;
+};
+#  endif
 # endif
 #endif
diff --git a/sysdeps/unix/sysv/linux/bits/sem-pad.h b/sysdeps/unix/sysv/linux/bits/sem-pad.h
index 566ce039cc..18235e56df 100644
--- a/sysdeps/unix/sysv/linux/bits/sem-pad.h
+++ b/sysdeps/unix/sysv/linux/bits/sem-pad.h
@@ -31,3 +31,4 @@
 
 #define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
 #define __SEM_PAD_BEFORE_TIME 0
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
diff --git a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
index ee0332325b..62580792b3 100644
--- a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
+++ b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
@@ -24,3 +24,5 @@
 
 #define __SEM_PAD_AFTER_TIME 0
 #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
+#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
diff --git a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
index 4c581f7694..12f2bd9c62 100644
--- a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
+++ b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
@@ -22,3 +22,5 @@
 
 #define __SEM_PAD_AFTER_TIME 0
 #define __SEM_PAD_BEFORE_TIME 0
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
+#define __SEMID_DS_HIGH_END (__WORDSIZE == 32)
diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
index 42d8827906..a13ca5f6bd 100644
--- a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
+++ b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
@@ -24,3 +24,5 @@
 
 #define __SEM_PAD_AFTER_TIME 0
 #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
+#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
index 0c3eb0932f..8dcb012b0b 100644
--- a/sysdeps/unix/sysv/linux/semctl.c
+++ b/sysdeps/unix/sysv/linux/semctl.c
@@ -28,6 +28,9 @@ union semun
 {
   int val;			/* value for SETVAL */
   struct semid_ds *buf;		/* buffer for IPC_STAT & IPC_SET */
+#ifdef __SEMID_DS_HIGH
+  struct __semid_ds32 *buf32;  /* buffer for IPC_STAT & IPC_SET on 32-bit systems */
+#endif
   unsigned short int *array;	/* array for GETALL & SETALL */
   struct seminfo *__buf;	/* buffer for IPC_INFO */
 };
@@ -43,13 +46,25 @@ union semun
 static int
 semctl_syscall (int semid, int semnum, int cmd, union semun arg)
 {
+  int ret;
 #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
+#ifdef __SEMID_DS_HIGH
+/* If we aren't going to overflow the time_t sem_ctime/sem_otime set it  */
+# if __TIMESIZE == 64 && __WORDSIZE == 32
+  if (ret == 0)
+    {
+      arg.buf->sem_ctime = arg.buf32->sem_ctime | ((time_t) arg.buf32->sem_ctime_high << 32);
+      arg.buf->sem_otime = arg.buf32->sem_otime | ((time_t) arg.buf32->sem_otime_high << 32);
+    }
+# endif
+#endif
+  return ret;
 }
 
 int
diff --git a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
index 5f4e214d12..79655b8149 100644
--- a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
+++ b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
@@ -24,3 +24,5 @@
 
 #define __SEM_PAD_AFTER_TIME 0
 #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
+#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
diff --git a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
index 102e226997..db397857e7 100644
--- a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
+++ b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
@@ -22,3 +22,4 @@
 
 #define __SEM_PAD_AFTER_TIME 1
 #define __SEM_PAD_BEFORE_TIME 0
+#define __SEMID_DS_HIGH 1
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] sysv: linux: Pass 64-bit version of semctl syscall
  2020-02-05  0:56 [PATCH v2] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
@ 2020-02-14 16:39 ` Alistair Francis
  2020-02-20 17:58   ` Alistair Francis
  2020-02-21 14:03 ` Adhemerval Zanella
  1 sibling, 1 reply; 5+ messages in thread
From: Alistair Francis @ 2020-02-14 16:39 UTC (permalink / raw)
  To: Alistair Francis; +Cc: GNU C Library

On Tue, Feb 4, 2020 at 5:02 PM Alistair Francis
<alistair.francis@wdc.com> 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 union semun to include a struct __semid_ds32
> which supports 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.

Ping!

> ---
> v2:
>  - Fix the HPPA error
>  - Simplfy the code changes
>
>  include/sys/sem.h                             | 35 +++++++++++++++++++
>  sysdeps/unix/sysv/linux/bits/sem-pad.h        |  1 +
>  sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h   |  2 ++
>  sysdeps/unix/sysv/linux/mips/bits/sem-pad.h   |  2 ++
>  .../unix/sysv/linux/powerpc/bits/sem-pad.h    |  2 ++
>  sysdeps/unix/sysv/linux/semctl.c              | 23 +++++++++---
>  sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h  |  2 ++
>  sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    |  1 +
>  8 files changed, 64 insertions(+), 4 deletions(-)
>
> diff --git a/include/sys/sem.h b/include/sys/sem.h
> index 69fdf1f752..70b83127cb 100644
> --- a/include/sys/sem.h
> +++ b/include/sys/sem.h
> @@ -5,5 +5,40 @@
>
>  __typeof__ (semtimedop) __semtimedop attribute_hidden;
>
> +# endif
> +
> +# ifdef __SEMID_DS_HIGH
> +#  if defined (__SEMID_DS_HIGH_END)
> +struct __semid_ds32 {
> +  struct ipc_perm sem_perm;             /* permissions .. see ipc.h */
> +  __syscall_ulong_t   sem_otime;          /* last semop time */
> +  __syscall_ulong_t   sem_ctime;          /* last change time */
> +  __syscall_ulong_t   sem_nsems;          /* no. of semaphores in array */
> +  __syscall_ulong_t   sem_otime_high;
> +  __syscall_ulong_t   sem_ctime_high;
> +};
> +#  elif defined (__SEMID_DS_HIGH_SWAP)
> +struct __semid_ds32 {
> +  struct ipc_perm sem_perm;              /* operation permission struct */
> +  __syscall_ulong_t   sem_otime_high;    /* last semop() time high */
> +  __syscall_ulong_t   sem_otime;         /* last semop() time */
> +  __syscall_ulong_t   sem_ctime_high;    /* last time changed by semctl() high */
> +  __syscall_ulong_t   sem_ctime;         /* last time changed by semctl() */
> +  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
> +  __syscall_ulong_t   __glibc_reserved3;
> +  __syscall_ulong_t   __glibc_reserved4;
> +};
> +#  else
> +struct __semid_ds32 {
> +  struct ipc_perm sem_perm;              /* operation permission struct */
> +  __syscall_ulong_t   sem_otime;         /* last semop() time */
> +  __syscall_ulong_t   sem_otime_high;    /* last semop() time high */
> +  __syscall_ulong_t   sem_ctime;         /* last time changed by semctl() */
> +  __syscall_ulong_t   sem_ctime_high;    /* last time changed by semctl() high */
> +  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
> +  __syscall_ulong_t   __glibc_reserved3;
> +  __syscall_ulong_t   __glibc_reserved4;
> +};
> +#  endif
>  # endif
>  #endif
> diff --git a/sysdeps/unix/sysv/linux/bits/sem-pad.h b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> index 566ce039cc..18235e56df 100644
> --- a/sysdeps/unix/sysv/linux/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> @@ -31,3 +31,4 @@
>
>  #define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
>  #define __SEM_PAD_BEFORE_TIME 0
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> index ee0332325b..62580792b3 100644
> --- a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> @@ -24,3 +24,5 @@
>
>  #define __SEM_PAD_AFTER_TIME 0
>  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> index 4c581f7694..12f2bd9c62 100644
> --- a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> @@ -22,3 +22,5 @@
>
>  #define __SEM_PAD_AFTER_TIME 0
>  #define __SEM_PAD_BEFORE_TIME 0
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_END (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> index 42d8827906..a13ca5f6bd 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> @@ -24,3 +24,5 @@
>
>  #define __SEM_PAD_AFTER_TIME 0
>  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
> index 0c3eb0932f..8dcb012b0b 100644
> --- a/sysdeps/unix/sysv/linux/semctl.c
> +++ b/sysdeps/unix/sysv/linux/semctl.c
> @@ -28,6 +28,9 @@ union semun
>  {
>    int val;                     /* value for SETVAL */
>    struct semid_ds *buf;                /* buffer for IPC_STAT & IPC_SET */
> +#ifdef __SEMID_DS_HIGH
> +  struct __semid_ds32 *buf32;  /* buffer for IPC_STAT & IPC_SET on 32-bit systems */
> +#endif
>    unsigned short int *array;   /* array for GETALL & SETALL */
>    struct seminfo *__buf;       /* buffer for IPC_INFO */
>  };
> @@ -43,13 +46,25 @@ union semun
>  static int
>  semctl_syscall (int semid, int semnum, int cmd, union semun arg)
>  {
> +  int ret;
>  #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
> +#ifdef __SEMID_DS_HIGH
> +/* If we aren't going to overflow the time_t sem_ctime/sem_otime set it  */
> +# if __TIMESIZE == 64 && __WORDSIZE == 32
> +  if (ret == 0)
> +    {
> +      arg.buf->sem_ctime = arg.buf32->sem_ctime | ((time_t) arg.buf32->sem_ctime_high << 32);
> +      arg.buf->sem_otime = arg.buf32->sem_otime | ((time_t) arg.buf32->sem_otime_high << 32);
> +    }
> +# endif
> +#endif
> +  return ret;
>  }
>
>  int
> diff --git a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> index 5f4e214d12..79655b8149 100644
> --- a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> @@ -24,3 +24,5 @@
>
>  #define __SEM_PAD_AFTER_TIME 0
>  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> index 102e226997..db397857e7 100644
> --- a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> @@ -22,3 +22,4 @@
>
>  #define __SEM_PAD_AFTER_TIME 1
>  #define __SEM_PAD_BEFORE_TIME 0
> +#define __SEMID_DS_HIGH 1
> --
> 2.25.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] sysv: linux: Pass 64-bit version of semctl syscall
  2020-02-14 16:39 ` Alistair Francis
@ 2020-02-20 17:58   ` Alistair Francis
  0 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2020-02-20 17:58 UTC (permalink / raw)
  To: Alistair Francis; +Cc: GNU C Library

On Fri, Feb 14, 2020 at 8:39 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Feb 4, 2020 at 5:02 PM Alistair Francis
> <alistair.francis@wdc.com> 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 union semun to include a struct __semid_ds32
> > which supports 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.
>
> Ping!

Ping^2

Alistair

>
> > ---
> > v2:
> >  - Fix the HPPA error
> >  - Simplfy the code changes
> >
> >  include/sys/sem.h                             | 35 +++++++++++++++++++
> >  sysdeps/unix/sysv/linux/bits/sem-pad.h        |  1 +
> >  sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h   |  2 ++
> >  sysdeps/unix/sysv/linux/mips/bits/sem-pad.h   |  2 ++
> >  .../unix/sysv/linux/powerpc/bits/sem-pad.h    |  2 ++
> >  sysdeps/unix/sysv/linux/semctl.c              | 23 +++++++++---
> >  sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h  |  2 ++
> >  sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    |  1 +
> >  8 files changed, 64 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/sys/sem.h b/include/sys/sem.h
> > index 69fdf1f752..70b83127cb 100644
> > --- a/include/sys/sem.h
> > +++ b/include/sys/sem.h
> > @@ -5,5 +5,40 @@
> >
> >  __typeof__ (semtimedop) __semtimedop attribute_hidden;
> >
> > +# endif
> > +
> > +# ifdef __SEMID_DS_HIGH
> > +#  if defined (__SEMID_DS_HIGH_END)
> > +struct __semid_ds32 {
> > +  struct ipc_perm sem_perm;             /* permissions .. see ipc.h */
> > +  __syscall_ulong_t   sem_otime;          /* last semop time */
> > +  __syscall_ulong_t   sem_ctime;          /* last change time */
> > +  __syscall_ulong_t   sem_nsems;          /* no. of semaphores in array */
> > +  __syscall_ulong_t   sem_otime_high;
> > +  __syscall_ulong_t   sem_ctime_high;
> > +};
> > +#  elif defined (__SEMID_DS_HIGH_SWAP)
> > +struct __semid_ds32 {
> > +  struct ipc_perm sem_perm;              /* operation permission struct */
> > +  __syscall_ulong_t   sem_otime_high;    /* last semop() time high */
> > +  __syscall_ulong_t   sem_otime;         /* last semop() time */
> > +  __syscall_ulong_t   sem_ctime_high;    /* last time changed by semctl() high */
> > +  __syscall_ulong_t   sem_ctime;         /* last time changed by semctl() */
> > +  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
> > +  __syscall_ulong_t   __glibc_reserved3;
> > +  __syscall_ulong_t   __glibc_reserved4;
> > +};
> > +#  else
> > +struct __semid_ds32 {
> > +  struct ipc_perm sem_perm;              /* operation permission struct */
> > +  __syscall_ulong_t   sem_otime;         /* last semop() time */
> > +  __syscall_ulong_t   sem_otime_high;    /* last semop() time high */
> > +  __syscall_ulong_t   sem_ctime;         /* last time changed by semctl() */
> > +  __syscall_ulong_t   sem_ctime_high;    /* last time changed by semctl() high */
> > +  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
> > +  __syscall_ulong_t   __glibc_reserved3;
> > +  __syscall_ulong_t   __glibc_reserved4;
> > +};
> > +#  endif
> >  # endif
> >  #endif
> > diff --git a/sysdeps/unix/sysv/linux/bits/sem-pad.h b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > index 566ce039cc..18235e56df 100644
> > --- a/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > @@ -31,3 +31,4 @@
> >
> >  #define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
> >  #define __SEM_PAD_BEFORE_TIME 0
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> > index ee0332325b..62580792b3 100644
> > --- a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> > @@ -24,3 +24,5 @@
> >
> >  #define __SEM_PAD_AFTER_TIME 0
> >  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> > index 4c581f7694..12f2bd9c62 100644
> > --- a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> > @@ -22,3 +22,5 @@
> >
> >  #define __SEM_PAD_AFTER_TIME 0
> >  #define __SEM_PAD_BEFORE_TIME 0
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_END (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> > index 42d8827906..a13ca5f6bd 100644
> > --- a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> > @@ -24,3 +24,5 @@
> >
> >  #define __SEM_PAD_AFTER_TIME 0
> >  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
> > index 0c3eb0932f..8dcb012b0b 100644
> > --- a/sysdeps/unix/sysv/linux/semctl.c
> > +++ b/sysdeps/unix/sysv/linux/semctl.c
> > @@ -28,6 +28,9 @@ union semun
> >  {
> >    int val;                     /* value for SETVAL */
> >    struct semid_ds *buf;                /* buffer for IPC_STAT & IPC_SET */
> > +#ifdef __SEMID_DS_HIGH
> > +  struct __semid_ds32 *buf32;  /* buffer for IPC_STAT & IPC_SET on 32-bit systems */
> > +#endif
> >    unsigned short int *array;   /* array for GETALL & SETALL */
> >    struct seminfo *__buf;       /* buffer for IPC_INFO */
> >  };
> > @@ -43,13 +46,25 @@ union semun
> >  static int
> >  semctl_syscall (int semid, int semnum, int cmd, union semun arg)
> >  {
> > +  int ret;
> >  #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
> > +#ifdef __SEMID_DS_HIGH
> > +/* If we aren't going to overflow the time_t sem_ctime/sem_otime set it  */
> > +# if __TIMESIZE == 64 && __WORDSIZE == 32
> > +  if (ret == 0)
> > +    {
> > +      arg.buf->sem_ctime = arg.buf32->sem_ctime | ((time_t) arg.buf32->sem_ctime_high << 32);
> > +      arg.buf->sem_otime = arg.buf32->sem_otime | ((time_t) arg.buf32->sem_otime_high << 32);
> > +    }
> > +# endif
> > +#endif
> > +  return ret;
> >  }
> >
> >  int
> > diff --git a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> > index 5f4e214d12..79655b8149 100644
> > --- a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> > @@ -24,3 +24,5 @@
> >
> >  #define __SEM_PAD_AFTER_TIME 0
> >  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> > index 102e226997..db397857e7 100644
> > --- a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> > @@ -22,3 +22,4 @@
> >
> >  #define __SEM_PAD_AFTER_TIME 1
> >  #define __SEM_PAD_BEFORE_TIME 0
> > +#define __SEMID_DS_HIGH 1
> > --
> > 2.25.0
> >

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] sysv: linux: Pass 64-bit version of semctl syscall
  2020-02-05  0:56 [PATCH v2] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
  2020-02-14 16:39 ` Alistair Francis
@ 2020-02-21 14:03 ` Adhemerval Zanella
  2020-02-27 23:57   ` Alistair Francis
  1 sibling, 1 reply; 5+ messages in thread
From: Adhemerval Zanella @ 2020-02-21 14:03 UTC (permalink / raw)
  To: libc-alpha



On 04/02/2020 21:56, 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 union semun to include a struct __semid_ds32
> which supports 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.
> ---
> v2:
>  - Fix the HPPA error
>  - Simplfy the code changes
> 
>  include/sys/sem.h                             | 35 +++++++++++++++++++
>  sysdeps/unix/sysv/linux/bits/sem-pad.h        |  1 +
>  sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h   |  2 ++
>  sysdeps/unix/sysv/linux/mips/bits/sem-pad.h   |  2 ++
>  .../unix/sysv/linux/powerpc/bits/sem-pad.h    |  2 ++
>  sysdeps/unix/sysv/linux/semctl.c              | 23 +++++++++---
>  sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h  |  2 ++
>  sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    |  1 +
>  8 files changed, 64 insertions(+), 4 deletions(-)
> 
> diff --git a/include/sys/sem.h b/include/sys/sem.h
> index 69fdf1f752..70b83127cb 100644
> --- a/include/sys/sem.h
> +++ b/include/sys/sem.h
> @@ -5,5 +5,40 @@
>  
>  __typeof__ (semtimedop) __semtimedop attribute_hidden;
>  
> +# endif
> +
> +# ifdef __SEMID_DS_HIGH
> +#  if defined (__SEMID_DS_HIGH_END)

The current idea for per-architecture overrides is to define the expected
default value on the generic header and check for the value *value* instead
of *existence*.  So here it should be:

#if __SEMID_DS_HIGH
# if __SEMID_DS_HIGH_END
...
# elif __SEMID_DS_HIGH_SWAP
...
# else
# endif
#endif

So you need to define __SEMID_DS_HIGH, __SEMID_DS_HIGH_END, and
__SEMID_DS_HIGH_SWAP for all architectures (this is how __SEM_PAD_BEFORE_TIME
and __SEM_PAD_AFTER_TIME are organized).

And I think it might be worth to add a _Static_assert to avoid possible
errors if an architecture wrongly set __SEMID_DS_HIGH_END and __SEMID_DS_HIGH_SWAP:

#if __SEMID_DS_HIGH
# if __SEMID_DS_HIGH_END
_Static_assert (!__SEMID_DS_HIGH_SWAP, "__SEMID_DS_HIGH_SWAP if defined");
...
# elif __SEMID_DS_HIGH_SWAP
...
# else
# endif
#endif

> +struct __semid_ds32 {
> +  struct ipc_perm sem_perm;             /* permissions .. see ipc.h */
> +  __syscall_ulong_t   sem_otime;          /* last semop time */
> +  __syscall_ulong_t   sem_ctime;          /* last change time */
> +  __syscall_ulong_t   sem_nsems;          /* no. of semaphores in array */
> +  __syscall_ulong_t   sem_otime_high;
> +  __syscall_ulong_t   sem_ctime_high;
> +};
> +#  elif defined (__SEMID_DS_HIGH_SWAP)
> +struct __semid_ds32 {
> +  struct ipc_perm sem_perm;              /* operation permission struct */
> +  __syscall_ulong_t   sem_otime_high;    /* last semop() time high */
> +  __syscall_ulong_t   sem_otime;         /* last semop() time */
> +  __syscall_ulong_t   sem_ctime_high;    /* last time changed by semctl() high */
> +  __syscall_ulong_t   sem_ctime;         /* last time changed by semctl() */
> +  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
> +  __syscall_ulong_t   __glibc_reserved3;
> +  __syscall_ulong_t   __glibc_reserved4;
> +};
> +#  else
> +struct __semid_ds32 {
> +  struct ipc_perm sem_perm;              /* operation permission struct */
> +  __syscall_ulong_t   sem_otime;         /* last semop() time */
> +  __syscall_ulong_t   sem_otime_high;    /* last semop() time high */
> +  __syscall_ulong_t   sem_ctime;         /* last time changed by semctl() */
> +  __syscall_ulong_t   sem_ctime_high;    /* last time changed by semctl() high */
> +  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
> +  __syscall_ulong_t   __glibc_reserved3;
> +  __syscall_ulong_t   __glibc_reserved4;
> +};
> +#  endif
>  # endif
>  #endif

From my pthread refactoring, I wonder if would be better to parametriz the
semid_ds instead and define the sem_{o,c}time{_time} directly on the structure
instead of adding define layer.

So we will have an installed Linux semid_ds_t.h header which Linux sem.h includes
and each architecture redefines if required.  It would allow to add the requires
y2038 fields you are proposing and semctl will change it directly.

> diff --git a/sysdeps/unix/sysv/linux/bits/sem-pad.h b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> index 566ce039cc..18235e56df 100644
> --- a/sysdeps/unix/sysv/linux/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> @@ -31,3 +31,4 @@
>  
>  #define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
>  #define __SEM_PAD_BEFORE_TIME 0
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> index ee0332325b..62580792b3 100644
> --- a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> @@ -24,3 +24,5 @@
>  
>  #define __SEM_PAD_AFTER_TIME 0
>  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> index 4c581f7694..12f2bd9c62 100644
> --- a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> @@ -22,3 +22,5 @@
>  
>  #define __SEM_PAD_AFTER_TIME 0
>  #define __SEM_PAD_BEFORE_TIME 0
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_END (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> index 42d8827906..a13ca5f6bd 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> @@ -24,3 +24,5 @@
>  
>  #define __SEM_PAD_AFTER_TIME 0
>  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
> index 0c3eb0932f..8dcb012b0b 100644
> --- a/sysdeps/unix/sysv/linux/semctl.c
> +++ b/sysdeps/unix/sysv/linux/semctl.c
> @@ -28,6 +28,9 @@ union semun
>  {
>    int val;			/* value for SETVAL */
>    struct semid_ds *buf;		/* buffer for IPC_STAT & IPC_SET */
> +#ifdef __SEMID_DS_HIGH
> +  struct __semid_ds32 *buf32;  /* buffer for IPC_STAT & IPC_SET on 32-bit systems */
> +#endif
>    unsigned short int *array;	/* array for GETALL & SETALL */
>    struct seminfo *__buf;	/* buffer for IPC_INFO */
>  };
> @@ -43,13 +46,25 @@ union semun
>  static int
>  semctl_syscall (int semid, int semnum, int cmd, union semun arg)
>  {
> +  int ret;
>  #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
> +#ifdef __SEMID_DS_HIGH
> +/* If we aren't going to overflow the time_t sem_ctime/sem_otime set it  */
> +# if __TIMESIZE == 64 && __WORDSIZE == 32

My understanding is for the SysV IPC y2038-safe call it should be done with 
a different IPC_STAT value than current one (with the __IPC_64 bit set). 
So I think a better strategy would be to:

  1. Check if IPC_STAT has the __IPC_64 bit set.
  2. Only set the y2038 safe values if command also has __IPC_64 bit set.
  3. Adjust all the requires architecture to have IPC_STAT with __IPC_64 set.
  4. For time32 symbol, clear the __IPC_64 bit.

So here it would be:

[...]
#define __IPC_TIME64 (IPC_STAT & __IPC_64)
[...]

#if __IPC_TIME64
  if (ret == 0 && (cmd & __IPC_TIME64))
    {
      [...]
    }
#endif

It wouldn't be active since IPC_STAT does not have IPC_64 set currently.

> +  if (ret == 0)
> +    {
> +      arg.buf->sem_ctime = arg.buf32->sem_ctime | ((time_t) arg.buf32->sem_ctime_high << 32);
> +      arg.buf->sem_otime = arg.buf32->sem_otime | ((time_t) arg.buf32->sem_otime_high << 32);
> +    }
> +# endif

Line too long.

> +#endif
> +  return ret;
>  }
>  
>  int
> diff --git a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> index 5f4e214d12..79655b8149 100644
> --- a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> @@ -24,3 +24,5 @@
>  
>  #define __SEM_PAD_AFTER_TIME 0
>  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> index 102e226997..db397857e7 100644
> --- a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> @@ -22,3 +22,4 @@
>  
>  #define __SEM_PAD_AFTER_TIME 1
>  #define __SEM_PAD_BEFORE_TIME 0
> +#define __SEMID_DS_HIGH 1
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] sysv: linux: Pass 64-bit version of semctl syscall
  2020-02-21 14:03 ` Adhemerval Zanella
@ 2020-02-27 23:57   ` Alistair Francis
  0 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2020-02-27 23:57 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library

 On Fri, Feb 21, 2020 at 6:03 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 04/02/2020 21:56, 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 union semun to include a struct __semid_ds32
> > which supports 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.
> > ---
> > v2:
> >  - Fix the HPPA error
> >  - Simplfy the code changes
> >
> >  include/sys/sem.h                             | 35 +++++++++++++++++++
> >  sysdeps/unix/sysv/linux/bits/sem-pad.h        |  1 +
> >  sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h   |  2 ++
> >  sysdeps/unix/sysv/linux/mips/bits/sem-pad.h   |  2 ++
> >  .../unix/sysv/linux/powerpc/bits/sem-pad.h    |  2 ++
> >  sysdeps/unix/sysv/linux/semctl.c              | 23 +++++++++---
> >  sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h  |  2 ++
> >  sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    |  1 +
> >  8 files changed, 64 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/sys/sem.h b/include/sys/sem.h
> > index 69fdf1f752..70b83127cb 100644
> > --- a/include/sys/sem.h
> > +++ b/include/sys/sem.h
> > @@ -5,5 +5,40 @@
> >
> >  __typeof__ (semtimedop) __semtimedop attribute_hidden;
> >
> > +# endif
> > +
> > +# ifdef __SEMID_DS_HIGH
> > +#  if defined (__SEMID_DS_HIGH_END)
>
> The current idea for per-architecture overrides is to define the expected
> default value on the generic header and check for the value *value* instead
> of *existence*.  So here it should be:
>
> #if __SEMID_DS_HIGH
> # if __SEMID_DS_HIGH_END
> ...
> # elif __SEMID_DS_HIGH_SWAP
> ...
> # else
> # endif
> #endif
>
> So you need to define __SEMID_DS_HIGH, __SEMID_DS_HIGH_END, and
> __SEMID_DS_HIGH_SWAP for all architectures (this is how __SEM_PAD_BEFORE_TIME
> and __SEM_PAD_AFTER_TIME are organized).
>
> And I think it might be worth to add a _Static_assert to avoid possible
> errors if an architecture wrongly set __SEMID_DS_HIGH_END and __SEMID_DS_HIGH_SWAP:
>
> #if __SEMID_DS_HIGH
> # if __SEMID_DS_HIGH_END
> _Static_assert (!__SEMID_DS_HIGH_SWAP, "__SEMID_DS_HIGH_SWAP if defined");
> ...
> # elif __SEMID_DS_HIGH_SWAP
> ...
> # else
> # endif
> #endif

I am dropping all of the defines based on your comments further down.

>
> > +struct __semid_ds32 {
> > +  struct ipc_perm sem_perm;             /* permissions .. see ipc.h */
> > +  __syscall_ulong_t   sem_otime;          /* last semop time */
> > +  __syscall_ulong_t   sem_ctime;          /* last change time */
> > +  __syscall_ulong_t   sem_nsems;          /* no. of semaphores in array */
> > +  __syscall_ulong_t   sem_otime_high;
> > +  __syscall_ulong_t   sem_ctime_high;
> > +};
> > +#  elif defined (__SEMID_DS_HIGH_SWAP)
> > +struct __semid_ds32 {
> > +  struct ipc_perm sem_perm;              /* operation permission struct */
> > +  __syscall_ulong_t   sem_otime_high;    /* last semop() time high */
> > +  __syscall_ulong_t   sem_otime;         /* last semop() time */
> > +  __syscall_ulong_t   sem_ctime_high;    /* last time changed by semctl() high */
> > +  __syscall_ulong_t   sem_ctime;         /* last time changed by semctl() */
> > +  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
> > +  __syscall_ulong_t   __glibc_reserved3;
> > +  __syscall_ulong_t   __glibc_reserved4;
> > +};
> > +#  else
> > +struct __semid_ds32 {
> > +  struct ipc_perm sem_perm;              /* operation permission struct */
> > +  __syscall_ulong_t   sem_otime;         /* last semop() time */
> > +  __syscall_ulong_t   sem_otime_high;    /* last semop() time high */
> > +  __syscall_ulong_t   sem_ctime;         /* last time changed by semctl() */
> > +  __syscall_ulong_t   sem_ctime_high;    /* last time changed by semctl() high */
> > +  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
> > +  __syscall_ulong_t   __glibc_reserved3;
> > +  __syscall_ulong_t   __glibc_reserved4;
> > +};
> > +#  endif
> >  # endif
> >  #endif
>
> From my pthread refactoring, I wonder if would be better to parametriz the
> semid_ds instead and define the sem_{o,c}time{_time} directly on the structure
> instead of adding define layer.
>
> So we will have an installed Linux semid_ds_t.h header which Linux sem.h includes
> and each architecture redefines if required.  It would allow to add the requires
> y2038 fields you are proposing and semctl will change it directly.

I have added a architecture specific semid_ds_t.h, I don't see how I
can use that for a y2038 safe implementation though, as we need to
expose to user space sem_{c, o}time as time_t (64-bit) but expose it
to the kernel as ulong (32-bit) sem_{c, o}time{_high}.

Without adding a new __semid_ds32I (for internal use) or creating a
internal use union semun I don't see how I could do this.

>
> > diff --git a/sysdeps/unix/sysv/linux/bits/sem-pad.h b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > index 566ce039cc..18235e56df 100644
> > --- a/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > @@ -31,3 +31,4 @@
> >
> >  #define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
> >  #define __SEM_PAD_BEFORE_TIME 0
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> > index ee0332325b..62580792b3 100644
> > --- a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> > @@ -24,3 +24,5 @@
> >
> >  #define __SEM_PAD_AFTER_TIME 0
> >  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> > index 4c581f7694..12f2bd9c62 100644
> > --- a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> > @@ -22,3 +22,5 @@
> >
> >  #define __SEM_PAD_AFTER_TIME 0
> >  #define __SEM_PAD_BEFORE_TIME 0
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_END (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> > index 42d8827906..a13ca5f6bd 100644
> > --- a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> > @@ -24,3 +24,5 @@
> >
> >  #define __SEM_PAD_AFTER_TIME 0
> >  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
> > index 0c3eb0932f..8dcb012b0b 100644
> > --- a/sysdeps/unix/sysv/linux/semctl.c
> > +++ b/sysdeps/unix/sysv/linux/semctl.c
> > @@ -28,6 +28,9 @@ union semun
> >  {
> >    int val;                   /* value for SETVAL */
> >    struct semid_ds *buf;              /* buffer for IPC_STAT & IPC_SET */
> > +#ifdef __SEMID_DS_HIGH
> > +  struct __semid_ds32 *buf32;  /* buffer for IPC_STAT & IPC_SET on 32-bit systems */
> > +#endif
> >    unsigned short int *array; /* array for GETALL & SETALL */
> >    struct seminfo *__buf;     /* buffer for IPC_INFO */
> >  };
> > @@ -43,13 +46,25 @@ union semun
> >  static int
> >  semctl_syscall (int semid, int semnum, int cmd, union semun arg)
> >  {
> > +  int ret;
> >  #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
> > +#ifdef __SEMID_DS_HIGH
> > +/* If we aren't going to overflow the time_t sem_ctime/sem_otime set it  */
> > +# if __TIMESIZE == 64 && __WORDSIZE == 32
>
> My understanding is for the SysV IPC y2038-safe call it should be done with
> a different IPC_STAT value than current one (with the __IPC_64 bit set).
> So I think a better strategy would be to:
>
>   1. Check if IPC_STAT has the __IPC_64 bit set.
>   2. Only set the y2038 safe values if command also has __IPC_64 bit set.
>   3. Adjust all the requires architecture to have IPC_STAT with __IPC_64 set.
>   4. For time32 symbol, clear the __IPC_64 bit.
>
> So here it would be:
>
> [...]
> #define __IPC_TIME64 (IPC_STAT & __IPC_64)
> [...]
>
> #if __IPC_TIME64
>   if (ret == 0 && (cmd & __IPC_TIME64))
>     {
>       [...]
>     }
> #endif
>
> It wouldn't be active since IPC_STAT does not have IPC_64 set currently.

I think I have addressed this.

>
> > +  if (ret == 0)
> > +    {
> > +      arg.buf->sem_ctime = arg.buf32->sem_ctime | ((time_t) arg.buf32->sem_ctime_high << 32);
> > +      arg.buf->sem_otime = arg.buf32->sem_otime | ((time_t) arg.buf32->sem_otime_high << 32);
> > +    }
> > +# endif
>
> Line too long.

Fixed.

Alistair

>
> > +#endif
> > +  return ret;
> >  }
> >
> >  int
> > diff --git a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> > index 5f4e214d12..79655b8149 100644
> > --- a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> > @@ -24,3 +24,5 @@
> >
> >  #define __SEM_PAD_AFTER_TIME 0
> >  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> > index 102e226997..db397857e7 100644
> > --- a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> > @@ -22,3 +22,4 @@
> >
> >  #define __SEM_PAD_AFTER_TIME 1
> >  #define __SEM_PAD_BEFORE_TIME 0
> > +#define __SEMID_DS_HIGH 1
> >

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-02-28  0:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-05  0:56 [PATCH v2] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
2020-02-14 16:39 ` Alistair Francis
2020-02-20 17:58   ` Alistair Francis
2020-02-21 14:03 ` Adhemerval Zanella
2020-02-27 23:57   ` Alistair Francis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).