public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v5 0/3] Support y2038 semctl_syscall()
@ 2020-04-01 16:53 Alistair Francis
  2020-04-01 16:53 ` [PATCH v5 1/3] bits/sem.h: Split out struct semid_ds Alistair Francis
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Alistair Francis @ 2020-04-01 16:53 UTC (permalink / raw)
  To: libc-alpha; +Cc: alistair23, Alistair Francis

This series does three things:
 1. Creates a bits/semid_ds_t.h file (in every arch) that specifies
    struct semid_ds so we no longer have to use macros defined in
    sem-pad.h.
 2. Removes the sem-pad.h file as it is no longer needed.
 3. Adds a new __semid_ds32 that is passed to the kernel (as part of
    a union) when running on 32-bit systems. If we are doing an
    IPC_STAT command then the 32-bit sem_{c,o}time{_high} values are
    combined to create a 64-bit value.

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 series was tested by running:
  ./scripts/build-many-glibcs.py ... compilers
  ./scripts/build-many-glibcs.py ... glibcs
on my x86_64 machine.

I also ran make check on RV32 and I only see a total of 9 test failures.

v5:
 - Address v4 review comments
 - Set the semid_ds struct from a temp struct
v4:
 - Remove the __IPC_TIME64 macro
    - It was only used once and doesn't work if __IPC_64 is 0 (which is
      usually is)
 - Address failures pointed out by Vineet Gupta

Alistair Francis (3):
  bits/sem.h: Split out struct semid_ds
  semctl: Remove the sem-pad.h file
  sysv: linux: Pass 64-bit version of semctl syscall

 sysdeps/unix/sysv/linux/Makefile              |  3 +-
 sysdeps/unix/sysv/linux/bits/sem-pad.h        | 33 -------------
 sysdeps/unix/sysv/linux/bits/sem.h            | 26 +----------
 .../sysv/linux/bits/types/struct_semid_ds.h   | 46 +++++++++++++++++++
 .../linux/hppa/bits/types/struct_semid_ds.h   | 46 +++++++++++++++++++
 .../unix/sysv/linux/hppa/struct__semid_ds32.h | 32 +++++++++++++
 .../bits/types/struct_semid_ds.h}             | 20 +++++---
 .../{bits/sem-pad.h => struct__semid_ds32.h}  | 20 +++++---
 .../unix/sysv/linux/powerpc/bits/sem-pad.h    | 26 -----------
 .../powerpc/bits/types/struct_semid_ds.h      | 46 +++++++++++++++++++
 .../sysv/linux/powerpc/struct__semid_ds32.h   | 32 +++++++++++++
 sysdeps/unix/sysv/linux/semctl.c              | 30 ++++++++++--
 .../linux/sparc/bits/types/struct_semid_ds.h  | 46 +++++++++++++++++++
 .../sysv/linux/sparc/struct__semid_ds32.h     | 32 +++++++++++++
 sysdeps/unix/sysv/linux/struct__semid_ds32.h  | 32 +++++++++++++
 sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    | 24 ----------
 .../bits/types/struct_semid_ds.h}             | 22 ++++++---
 .../unix/sysv/linux/x86/struct__semid_ds32.h  | 32 +++++++++++++
 18 files changed, 415 insertions(+), 133 deletions(-)
 delete mode 100644 sysdeps/unix/sysv/linux/bits/sem-pad.h
 create mode 100644 sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
 create mode 100644 sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h
 create mode 100644 sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h
 rename sysdeps/unix/sysv/linux/{sparc/bits/sem-pad.h => mips/bits/types/struct_semid_ds.h} (56%)
 rename sysdeps/unix/sysv/linux/mips/{bits/sem-pad.h => struct__semid_ds32.h} (52%)
 delete mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h
 create mode 100644 sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h
 create mode 100644 sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h
 create mode 100644 sysdeps/unix/sysv/linux/struct__semid_ds32.h
 delete mode 100644 sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
 rename sysdeps/unix/sysv/linux/{hppa/bits/sem-pad.h => x86/bits/types/struct_semid_ds.h} (52%)
 create mode 100644 sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h

-- 
2.26.0


^ permalink raw reply	[flat|nested] 16+ messages in thread
* [PATCH v5 1/3] bits/sem.h: Split out struct semid_ds
  2020-04-01 16:53 [PATCH v5 0/3] Support y2038 semctl_syscall() Alistair Francis
@ 2020-04-01 16:53 ` Alistair Francis
  2020-04-17 18:06   ` Adhemerval Zanella
  2020-04-01 16:53 ` [PATCH v5 2/3] semctl: Remove the sem-pad.h file Alistair Francis
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Alistair Francis @ 2020-04-01 16:53 UTC (permalink / raw)
  To: libc-alpha; +Cc: alistair23, Alistair Francis

Split out the struct semid_ds into it's own file. This will allow us to
have architectures specify their own version.
---
 sysdeps/unix/sysv/linux/Makefile              |  1 +
 sysdeps/unix/sysv/linux/bits/sem.h            | 24 +----------
 .../sysv/linux/bits/types/struct_semid_ds.h   | 43 +++++++++++++++++++
 3 files changed, 45 insertions(+), 23 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h

diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 60dc5cf9e5..bbf1d60fc6 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -92,6 +92,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
 		  bits/termios-baud.h bits/termios-c_cflag.h \
 		  bits/termios-c_lflag.h bits/termios-tcflow.h \
 		  bits/termios-misc.h \
+		  bits/types/struct_semid_ds.h \
 		  bits/ipc-perm.h
 
 tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
diff --git a/sysdeps/unix/sysv/linux/bits/sem.h b/sysdeps/unix/sysv/linux/bits/sem.h
index e0f4155c67..0d1813ec67 100644
--- a/sysdeps/unix/sysv/linux/bits/sem.h
+++ b/sysdeps/unix/sysv/linux/bits/sem.h
@@ -21,6 +21,7 @@
 
 #include <sys/types.h>
 #include <bits/sem-pad.h>
+#include <bits/types/struct_semid_ds.h>
 
 /* Flags for `semop'.  */
 #define SEM_UNDO	0x1000		/* undo the operation on exit */
@@ -34,29 +35,6 @@
 #define SETVAL		16		/* set semval */
 #define SETALL		17		/* set all semval's */
 
-
-#if __SEM_PAD_BEFORE_TIME
-# define __SEM_PAD_TIME(NAME, RES)				\
-  __syscall_ulong_t __glibc_reserved ## RES; __time_t NAME
-#elif __SEM_PAD_AFTER_TIME
-# define __SEM_PAD_TIME(NAME, RES)				\
-  __time_t NAME; __syscall_ulong_t __glibc_reserved ## RES
-#else
-# define __SEM_PAD_TIME(NAME, RES)		\
-  __time_t NAME
-#endif
-
-/* Data structure describing a set of semaphores.  */
-struct semid_ds
-{
-  struct ipc_perm sem_perm;		/* operation permission struct */
-  __SEM_PAD_TIME (sem_otime, 1);	/* last semop() time */
-  __SEM_PAD_TIME (sem_ctime, 2);	/* 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;
-};
-
 /* The user should define a union like the following to use it for arguments
    for `semctl'.
 
diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
new file mode 100644
index 0000000000..ba0719e77a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
@@ -0,0 +1,43 @@
+/* Generic implementation of the semaphore struct semid_ds
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _SYS_SEM_H
+# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
+#endif
+
+#if __SEM_PAD_BEFORE_TIME
+# define __SEM_PAD_TIME(NAME, RES)        \
+  __syscall_ulong_t __glibc_reserved ## RES; __time_t NAME
+#elif __SEM_PAD_AFTER_TIME
+# define __SEM_PAD_TIME(NAME, RES)        \
+  __time_t NAME; __syscall_ulong_t __glibc_reserved ## RES
+#else
+# define __SEM_PAD_TIME(NAME, RES)    \
+  __time_t NAME
+#endif
+
+/* Data structure describing a set of semaphores.  */
+struct semid_ds
+{
+  struct ipc_perm sem_perm;		/* operation permission struct */
+  __SEM_PAD_TIME (sem_otime, 1);	/* last semop() time */
+  __SEM_PAD_TIME (sem_ctime, 2);	/* 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;
+};
-- 
2.26.0


^ permalink raw reply	[flat|nested] 16+ messages in thread
* [PATCH v5 2/3] semctl: Remove the sem-pad.h file
  2020-04-01 16:53 [PATCH v5 0/3] Support y2038 semctl_syscall() Alistair Francis
  2020-04-01 16:53 ` [PATCH v5 1/3] bits/sem.h: Split out struct semid_ds Alistair Francis
@ 2020-04-01 16:53 ` Alistair Francis
  2020-04-17 19:03   ` Adhemerval Zanella
  2020-04-22 19:22   ` Adhemerval Zanella
  2020-04-01 16:53 ` [PATCH v5 3/3] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
  2020-04-16 16:23 ` [PATCH v5 0/3] Support y2038 semctl_syscall() Alistair Francis
  3 siblings, 2 replies; 16+ messages in thread
From: Alistair Francis @ 2020-04-01 16:53 UTC (permalink / raw)
  To: libc-alpha; +Cc: alistair23, Alistair Francis

Remove the sem-pad.h file and instead have architectures override the
struct semid_ds via the bits/types/struct_semid_ds.h file.
---
 sysdeps/unix/sysv/linux/Makefile              |  2 +-
 sysdeps/unix/sysv/linux/bits/sem-pad.h        | 33 -------------
 sysdeps/unix/sysv/linux/bits/sem.h            |  2 +-
 .../sysv/linux/bits/types/struct_semid_ds.h   | 29 ++++++------
 .../linux/hppa/bits/types/struct_semid_ds.h   | 46 +++++++++++++++++++
 sysdeps/unix/sysv/linux/mips/bits/sem-pad.h   | 24 ----------
 .../bits/types/struct_semid_ds.h}             | 20 +++++---
 .../unix/sysv/linux/powerpc/bits/sem-pad.h    | 26 -----------
 .../powerpc/bits/types/struct_semid_ds.h      | 46 +++++++++++++++++++
 .../linux/sparc/bits/types/struct_semid_ds.h  | 46 +++++++++++++++++++
 sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    | 24 ----------
 .../bits/types/struct_semid_ds.h}             | 22 ++++++---
 12 files changed, 184 insertions(+), 136 deletions(-)
 delete mode 100644 sysdeps/unix/sysv/linux/bits/sem-pad.h
 create mode 100644 sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h
 delete mode 100644 sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
 rename sysdeps/unix/sysv/linux/{sparc/bits/sem-pad.h => mips/bits/types/struct_semid_ds.h} (56%)
 delete mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h
 create mode 100644 sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h
 delete mode 100644 sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
 rename sysdeps/unix/sysv/linux/{hppa/bits/sem-pad.h => x86/bits/types/struct_semid_ds.h} (52%)

diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index bbf1d60fc6..84ed4344d3 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -86,7 +86,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
 		  bits/siginfo-arch.h bits/siginfo-consts-arch.h \
 		  bits/procfs.h bits/procfs-id.h bits/procfs-extra.h \
 		  bits/procfs-prregset.h bits/mman-map-flags-generic.h \
-		  bits/msq-pad.h bits/sem-pad.h bits/shmlba.h bits/shm-pad.h \
+		  bits/msq-pad.h bits/shmlba.h bits/shm-pad.h \
 		  bits/termios-struct.h bits/termios-c_cc.h \
 		  bits/termios-c_iflag.h bits/termios-c_oflag.h \
 		  bits/termios-baud.h bits/termios-c_cflag.h \
diff --git a/sysdeps/unix/sysv/linux/bits/sem-pad.h b/sysdeps/unix/sysv/linux/bits/sem-pad.h
deleted file mode 100644
index 566ce039cc..0000000000
--- a/sysdeps/unix/sysv/linux/bits/sem-pad.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/* Define where padding goes in struct semid_ds.  Generic version.
-   Copyright (C) 2018-2020 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#ifndef _SYS_SEM_H
-# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
-#endif
-
-#include <bits/timesize.h>
-
-/* On most architectures, padding goes after time fields for 32-bit
-   systems and is omitted for 64-bit systems.  Some architectures pad
-   before time fields instead, or omit padding despite being 32-bit,
-   or include it despite being 64-bit.  This must match the layout
-   used for struct semid64_ds in <asm/sembuf.h>, as glibc does not do
-   layout conversions for this structure.  */
-
-#define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
-#define __SEM_PAD_BEFORE_TIME 0
diff --git a/sysdeps/unix/sysv/linux/bits/sem.h b/sysdeps/unix/sysv/linux/bits/sem.h
index 0d1813ec67..ba1169fdb3 100644
--- a/sysdeps/unix/sysv/linux/bits/sem.h
+++ b/sysdeps/unix/sysv/linux/bits/sem.h
@@ -20,7 +20,7 @@
 #endif
 
 #include <sys/types.h>
-#include <bits/sem-pad.h>
+#include <bits/timesize.h>
 #include <bits/types/struct_semid_ds.h>
 
 /* Flags for `semop'.  */
diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
index ba0719e77a..659db85db8 100644
--- a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
+++ b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
@@ -20,24 +20,27 @@
 # error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
 #endif
 
-#if __SEM_PAD_BEFORE_TIME
-# define __SEM_PAD_TIME(NAME, RES)        \
-  __syscall_ulong_t __glibc_reserved ## RES; __time_t NAME
-#elif __SEM_PAD_AFTER_TIME
-# define __SEM_PAD_TIME(NAME, RES)        \
-  __time_t NAME; __syscall_ulong_t __glibc_reserved ## RES
-#else
-# define __SEM_PAD_TIME(NAME, RES)    \
-  __time_t NAME
-#endif
-
 /* Data structure describing a set of semaphores.  */
+#if __TIMESIZE == 32
+struct semid_ds
+{
+  struct ipc_perm sem_perm;        /* operation permission struct */
+  __time_t sem_otime;              /* last semop() time */
+  __syscall_ulong_t __glibc_reserved1;
+  __time_t sem_ctime;             /* last time changed by semctl() */
+  __syscall_ulong_t __glibc_reserved2;
+  __syscall_ulong_t sem_nsems;    /* number of semaphores in set */
+  __syscall_ulong_t __glibc_reserved3;
+  __syscall_ulong_t __glibc_reserved4;
+};
+#else
 struct semid_ds
 {
   struct ipc_perm sem_perm;		/* operation permission struct */
-  __SEM_PAD_TIME (sem_otime, 1);	/* last semop() time */
-  __SEM_PAD_TIME (sem_ctime, 2);	/* last time changed by semctl() */
+  __time_t sem_otime;	        /* last semop() time */
+  __time_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;
 };
+#endif
diff --git a/sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h
new file mode 100644
index 0000000000..fbc26ef2ca
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h
@@ -0,0 +1,46 @@
+/* HPPA implementation of the semaphore struct semid_ds
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _SYS_SEM_H
+# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
+#endif
+
+/* Data structure describing a set of semaphores.  */
+#if __TIMESIZE == 32
+struct semid_ds
+{
+  struct ipc_perm sem_perm;   /* operation permission struct */
+  __syscall_ulong_t __glibc_reserved1;
+  __time_t sem_otime;         /* last semop() time */
+  __syscall_ulong_t __glibc_reserved2;
+  __time_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_ds
+{
+  struct ipc_perm sem_perm;		/* operation permission struct */
+  __time_t sem_otime;       	/* last semop() time */
+  __time_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;
+};
+#endif
diff --git a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
deleted file mode 100644
index 4c581f7694..0000000000
--- a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/* Define where padding goes in struct semid_ds.  MIPS version.
-   Copyright (C) 2018-2020 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#ifndef _SYS_SEM_H
-# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
-#endif
-
-#define __SEM_PAD_AFTER_TIME 0
-#define __SEM_PAD_BEFORE_TIME 0
diff --git a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/mips/bits/types/struct_semid_ds.h
similarity index 56%
rename from sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
rename to sysdeps/unix/sysv/linux/mips/bits/types/struct_semid_ds.h
index 5f4e214d12..8954209a29 100644
--- a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
+++ b/sysdeps/unix/sysv/linux/mips/bits/types/struct_semid_ds.h
@@ -1,5 +1,5 @@
-/* Define where padding goes in struct semid_ds.  SPARC version.
-   Copyright (C) 2018-2020 Free Software Foundation, Inc.
+/* MIPS implementation of the semaphore struct semid_ds
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -17,10 +17,16 @@
    <https://www.gnu.org/licenses/>.  */
 
 #ifndef _SYS_SEM_H
-# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
+# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
 #endif
 
-#include <bits/timesize.h>
-
-#define __SEM_PAD_AFTER_TIME 0
-#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
+/* Data structure describing a set of semaphores.  */
+struct semid_ds
+{
+  struct ipc_perm sem_perm;		/* operation permission struct */
+  __time_t sem_otime;	/* last semop() time */
+  __time_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;
+};
diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
deleted file mode 100644
index 42d8827906..0000000000
--- a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/* Define where padding goes in struct semid_ds.  PowerPC version.
-   Copyright (C) 2018-2020 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#ifndef _SYS_SEM_H
-# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
-#endif
-
-#include <bits/timesize.h>
-
-#define __SEM_PAD_AFTER_TIME 0
-#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h
new file mode 100644
index 0000000000..d393141808
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h
@@ -0,0 +1,46 @@
+/* PowerPC implementation of the semaphore struct semid_ds
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _SYS_SEM_H
+# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
+#endif
+
+/* Data structure describing a set of semaphores.  */
+#if __TIMESIZE == 32
+struct semid_ds
+{
+  struct ipc_perm sem_perm;   /* operation permission struct */
+  __syscall_ulong_t __glibc_reserved1;
+  __time_t sem_otime;  /* last semop() time */
+  __syscall_ulong_t __glibc_reserved2;
+  __time_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_ds
+{
+  struct ipc_perm sem_perm;		/* operation permission struct */
+  __time_t sem_otime;	/* last semop() time */
+  __time_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;
+};
+#endif
diff --git a/sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h
new file mode 100644
index 0000000000..84c7a9022a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h
@@ -0,0 +1,46 @@
+/* Sparc implementation of the semaphore struct semid_ds
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _SYS_SEM_H
+# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
+#endif
+
+/* Data structure describing a set of semaphores.  */
+#if __TIMESIZE == 32
+struct semid_ds
+{
+  struct ipc_perm sem_perm;   /* operation permission struct */
+  __syscall_ulong_t __glibc_reserved1;
+  __time_t sem_otime;  /* last semop() time */
+  __syscall_ulong_t __glibc_reserved2;
+  __time_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_ds
+{
+  struct ipc_perm sem_perm;		/* operation permission struct */
+  __time_t sem_otime;	/* last semop() time */
+  __time_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;
+};
+#endif
diff --git a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
deleted file mode 100644
index 102e226997..0000000000
--- a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/* Define where padding goes in struct semid_ds.  x86 version.
-   Copyright (C) 2018-2020 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#ifndef _SYS_SEM_H
-# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
-#endif
-
-#define __SEM_PAD_AFTER_TIME 1
-#define __SEM_PAD_BEFORE_TIME 0
diff --git a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
similarity index 52%
rename from sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
rename to sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
index ee0332325b..22f0645f85 100644
--- a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
+++ b/sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
@@ -1,5 +1,5 @@
-/* Define where padding goes in struct semid_ds.  HPPA version.
-   Copyright (C) 2018-2020 Free Software Foundation, Inc.
+/* Sparc implementation of the semaphore struct semid_ds
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -17,10 +17,18 @@
    <https://www.gnu.org/licenses/>.  */
 
 #ifndef _SYS_SEM_H
-# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
+# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
 #endif
 
-#include <bits/timesize.h>
-
-#define __SEM_PAD_AFTER_TIME 0
-#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
+/* Data structure describing a set of semaphores.  */
+struct semid_ds
+{
+  struct ipc_perm sem_perm;   /* operation permission struct */
+  __time_t sem_otime;  /* last semop() time */
+  __syscall_ulong_t __glibc_reserved1;
+  __time_t sem_ctime;  /* last time changed by semctl() */
+  __syscall_ulong_t __glibc_reserved2;
+  __syscall_ulong_t sem_nsems;    /* number of semaphores in set */
+  __syscall_ulong_t __glibc_reserved3;
+  __syscall_ulong_t __glibc_reserved4;
+};
-- 
2.26.0


^ permalink raw reply	[flat|nested] 16+ messages in thread
* [PATCH v5 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-04-01 16:53 [PATCH v5 0/3] Support y2038 semctl_syscall() Alistair Francis
  2020-04-01 16:53 ` [PATCH v5 1/3] bits/sem.h: Split out struct semid_ds Alistair Francis
  2020-04-01 16:53 ` [PATCH v5 2/3] semctl: Remove the sem-pad.h file Alistair Francis
@ 2020-04-01 16:53 ` Alistair Francis
  2020-04-21 20:52   ` Stepan Golosunov
  2020-04-22 21:25   ` Adhemerval Zanella
  2020-04-16 16:23 ` [PATCH v5 0/3] Support y2038 semctl_syscall() Alistair Francis
  3 siblings, 2 replies; 16+ messages in thread
From: Alistair Francis @ 2020-04-01 16:53 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 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.

Due to allignment differences between 64-bit and 32-bit variables we
also need to set nsems to ensure it's correct.
---
 .../unix/sysv/linux/hppa/struct__semid_ds32.h | 32 +++++++++++++++++++
 .../unix/sysv/linux/mips/struct__semid_ds32.h | 30 +++++++++++++++++
 .../sysv/linux/powerpc/struct__semid_ds32.h   | 32 +++++++++++++++++++
 sysdeps/unix/sysv/linux/semctl.c              | 30 ++++++++++++++---
 .../sysv/linux/sparc/struct__semid_ds32.h     | 32 +++++++++++++++++++
 sysdeps/unix/sysv/linux/struct__semid_ds32.h  | 32 +++++++++++++++++++
 .../unix/sysv/linux/x86/struct__semid_ds32.h  | 32 +++++++++++++++++++
 7 files changed, 216 insertions(+), 4 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h
 create mode 100644 sysdeps/unix/sysv/linux/mips/struct__semid_ds32.h
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h
 create mode 100644 sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h
 create mode 100644 sysdeps/unix/sysv/linux/struct__semid_ds32.h
 create mode 100644 sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h

diff --git a/sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h
new file mode 100644
index 0000000000..db78794e8c
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h
@@ -0,0 +1,32 @@
+/* HPPA implementation of the semaphore struct __semid_ds32
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#if __WORDSIZE == 32
+/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
+ * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
+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;
+};
+#endif
diff --git a/sysdeps/unix/sysv/linux/mips/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/mips/struct__semid_ds32.h
new file mode 100644
index 0000000000..d3eb47611c
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/struct__semid_ds32.h
@@ -0,0 +1,30 @@
+/* MIPS implementation of the semaphore struct __semid_ds32
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#if __WORDSIZE == 32
+/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
+ * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
+struct __semid_ds32 {
+  struct ipc_perm sem_perm;              /* operation permission struct */
+  __syscall_ulong_t   sem_otime;          /* last semop time */
+  __syscall_ulong_t   sem_ctime;          /* last change time */
+  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
+  __syscall_ulong_t   sem_otime_high;
+  __syscall_ulong_t   sem_ctime_high;
+};
+#endif
diff --git a/sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h
new file mode 100644
index 0000000000..7ba6b852f1
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h
@@ -0,0 +1,32 @@
+/* PowerPC implementation of the semaphore struct __semid_ds32
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#if __WORDSIZE == 32
+/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
+ * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
+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;
+};
+#endif
diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
index 30571af49f..b0e4d828ed 100644
--- a/sysdeps/unix/sysv/linux/semctl.c
+++ b/sysdeps/unix/sysv/linux/semctl.c
@@ -22,6 +22,7 @@
 #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.  */
@@ -29,6 +30,9 @@ union semun
 {
   int val;			/* value for SETVAL */
   struct semid_ds *buf;		/* buffer for IPC_STAT & IPC_SET */
+#if __WORDSIZE == 32
+  struct __semid_ds32 *buf32;   /* 32-bit buffer for IPC_STAT */
+#endif
   unsigned short int *array;	/* array for GETALL & SETALL */
   struct seminfo *__buf;	/* buffer for IPC_INFO */
 };
@@ -44,13 +48,31 @@ 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
+
+#if (__WORDSIZE == 32 && __TIMESIZE == 64 \
+     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
+  if (ret == 0 && (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY))
+    {
+      struct semid_ds temp_ds = (struct semid_ds) {
+        .sem_perm = arg.buf32->sem_perm,
+        .sem_nsems = arg.buf32->sem_nsems,
+        .sem_otime = (arg.buf32->sem_otime
+                      | ((time_t) arg.buf32->sem_otime_high << 32)),
+        .sem_ctime = (arg.buf32->sem_ctime
+                      | ((time_t) arg.buf32->sem_ctime_high << 32)),
+      };
+      *arg.buf = temp_ds;
+    }
+#endif
+  return ret;
 }
 
 int
diff --git a/sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h
new file mode 100644
index 0000000000..c39d65d710
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h
@@ -0,0 +1,32 @@
+/* Sparc implementation of the semaphore struct __semid_ds32
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#if __WORDSIZE == 32
+/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
+ * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
+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;
+};
+#endif
diff --git a/sysdeps/unix/sysv/linux/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/struct__semid_ds32.h
new file mode 100644
index 0000000000..c82bf01bf3
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/struct__semid_ds32.h
@@ -0,0 +1,32 @@
+/* Generic implementation of the semaphore struct __semid_ds32
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#if __WORDSIZE == 32
+/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
+ * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
+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
diff --git a/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
new file mode 100644
index 0000000000..4e4ab26661
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
@@ -0,0 +1,32 @@
+/* Sparc implementation of the semaphore struct __semid_ds32
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifdef __i386__
+/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
+ * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
+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
-- 
2.26.0


^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 0/3] Support y2038 semctl_syscall()
  2020-04-01 16:53 [PATCH v5 0/3] Support y2038 semctl_syscall() Alistair Francis
                   ` (2 preceding siblings ...)
  2020-04-01 16:53 ` [PATCH v5 3/3] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
@ 2020-04-16 16:23 ` Alistair Francis
  3 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2020-04-16 16:23 UTC (permalink / raw)
  To: Alistair Francis; +Cc: GNU C Library

On Wed, Apr 1, 2020 at 10:01 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> This series does three things:
>  1. Creates a bits/semid_ds_t.h file (in every arch) that specifies
>     struct semid_ds so we no longer have to use macros defined in
>     sem-pad.h.
>  2. Removes the sem-pad.h file as it is no longer needed.
>  3. Adds a new __semid_ds32 that is passed to the kernel (as part of
>     a union) when running on 32-bit systems. If we are doing an
>     IPC_STAT command then the 32-bit sem_{c,o}time{_high} values are
>     combined to create a 64-bit value.
>
> 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 series was tested by running:
>   ./scripts/build-many-glibcs.py ... compilers
>   ./scripts/build-many-glibcs.py ... glibcs
> on my x86_64 machine.
>
> I also ran make check on RV32 and I only see a total of 9 test failures.

Ping!

>
> v5:
>  - Address v4 review comments
>  - Set the semid_ds struct from a temp struct
> v4:
>  - Remove the __IPC_TIME64 macro
>     - It was only used once and doesn't work if __IPC_64 is 0 (which is
>       usually is)
>  - Address failures pointed out by Vineet Gupta
>
> Alistair Francis (3):
>   bits/sem.h: Split out struct semid_ds
>   semctl: Remove the sem-pad.h file
>   sysv: linux: Pass 64-bit version of semctl syscall
>
>  sysdeps/unix/sysv/linux/Makefile              |  3 +-
>  sysdeps/unix/sysv/linux/bits/sem-pad.h        | 33 -------------
>  sysdeps/unix/sysv/linux/bits/sem.h            | 26 +----------
>  .../sysv/linux/bits/types/struct_semid_ds.h   | 46 +++++++++++++++++++
>  .../linux/hppa/bits/types/struct_semid_ds.h   | 46 +++++++++++++++++++
>  .../unix/sysv/linux/hppa/struct__semid_ds32.h | 32 +++++++++++++
>  .../bits/types/struct_semid_ds.h}             | 20 +++++---
>  .../{bits/sem-pad.h => struct__semid_ds32.h}  | 20 +++++---
>  .../unix/sysv/linux/powerpc/bits/sem-pad.h    | 26 -----------
>  .../powerpc/bits/types/struct_semid_ds.h      | 46 +++++++++++++++++++
>  .../sysv/linux/powerpc/struct__semid_ds32.h   | 32 +++++++++++++
>  sysdeps/unix/sysv/linux/semctl.c              | 30 ++++++++++--
>  .../linux/sparc/bits/types/struct_semid_ds.h  | 46 +++++++++++++++++++
>  .../sysv/linux/sparc/struct__semid_ds32.h     | 32 +++++++++++++
>  sysdeps/unix/sysv/linux/struct__semid_ds32.h  | 32 +++++++++++++
>  sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    | 24 ----------
>  .../bits/types/struct_semid_ds.h}             | 22 ++++++---
>  .../unix/sysv/linux/x86/struct__semid_ds32.h  | 32 +++++++++++++
>  18 files changed, 415 insertions(+), 133 deletions(-)
>  delete mode 100644 sysdeps/unix/sysv/linux/bits/sem-pad.h
>  create mode 100644 sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
>  create mode 100644 sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h
>  create mode 100644 sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h
>  rename sysdeps/unix/sysv/linux/{sparc/bits/sem-pad.h => mips/bits/types/struct_semid_ds.h} (56%)
>  rename sysdeps/unix/sysv/linux/mips/{bits/sem-pad.h => struct__semid_ds32.h} (52%)
>  delete mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
>  create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h
>  create mode 100644 sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h
>  create mode 100644 sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/struct__semid_ds32.h
>  delete mode 100644 sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
>  rename sysdeps/unix/sysv/linux/{hppa/bits/sem-pad.h => x86/bits/types/struct_semid_ds.h} (52%)
>  create mode 100644 sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
>
> --
> 2.26.0
>

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 1/3] bits/sem.h: Split out struct semid_ds
  2020-04-01 16:53 ` [PATCH v5 1/3] bits/sem.h: Split out struct semid_ds Alistair Francis
@ 2020-04-17 18:06   ` Adhemerval Zanella
  0 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2020-04-17 18:06 UTC (permalink / raw)
  To: libc-alpha



On 01/04/2020 13:53, Alistair Francis via Libc-alpha wrote:
> Split out the struct semid_ds into it's own file. This will allow us to
> have architectures specify their own version.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/unix/sysv/linux/Makefile              |  1 +
>  sysdeps/unix/sysv/linux/bits/sem.h            | 24 +----------
>  .../sysv/linux/bits/types/struct_semid_ds.h   | 43 +++++++++++++++++++
>  3 files changed, 45 insertions(+), 23 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> 
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 60dc5cf9e5..bbf1d60fc6 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -92,6 +92,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
>  		  bits/termios-baud.h bits/termios-c_cflag.h \
>  		  bits/termios-c_lflag.h bits/termios-tcflow.h \
>  		  bits/termios-misc.h \
> +		  bits/types/struct_semid_ds.h \
>  		  bits/ipc-perm.h
>  
>  tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \

Ok.

> diff --git a/sysdeps/unix/sysv/linux/bits/sem.h b/sysdeps/unix/sysv/linux/bits/sem.h
> index e0f4155c67..0d1813ec67 100644
> --- a/sysdeps/unix/sysv/linux/bits/sem.h
> +++ b/sysdeps/unix/sysv/linux/bits/sem.h
> @@ -21,6 +21,7 @@
>  
>  #include <sys/types.h>
>  #include <bits/sem-pad.h>
> +#include <bits/types/struct_semid_ds.h>
>  
>  /* Flags for `semop'.  */
>  #define SEM_UNDO	0x1000		/* undo the operation on exit */
> @@ -34,29 +35,6 @@
>  #define SETVAL		16		/* set semval */
>  #define SETALL		17		/* set all semval's */
>  
> -
> -#if __SEM_PAD_BEFORE_TIME
> -# define __SEM_PAD_TIME(NAME, RES)				\
> -  __syscall_ulong_t __glibc_reserved ## RES; __time_t NAME
> -#elif __SEM_PAD_AFTER_TIME
> -# define __SEM_PAD_TIME(NAME, RES)				\
> -  __time_t NAME; __syscall_ulong_t __glibc_reserved ## RES
> -#else
> -# define __SEM_PAD_TIME(NAME, RES)		\
> -  __time_t NAME
> -#endif
> -
> -/* Data structure describing a set of semaphores.  */
> -struct semid_ds
> -{
> -  struct ipc_perm sem_perm;		/* operation permission struct */
> -  __SEM_PAD_TIME (sem_otime, 1);	/* last semop() time */
> -  __SEM_PAD_TIME (sem_ctime, 2);	/* 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;
> -};
> -
>  /* The user should define a union like the following to use it for arguments
>     for `semctl'.
>  

Ok.

> diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> new file mode 100644
> index 0000000000..ba0719e77a
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> @@ -0,0 +1,43 @@
> +/* Generic implementation of the semaphore struct semid_ds

Missing period.

> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_SEM_H
> +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
> +#endif
> +
> +#if __SEM_PAD_BEFORE_TIME
> +# define __SEM_PAD_TIME(NAME, RES)        \
> +  __syscall_ulong_t __glibc_reserved ## RES; __time_t NAME
> +#elif __SEM_PAD_AFTER_TIME
> +# define __SEM_PAD_TIME(NAME, RES)        \
> +  __time_t NAME; __syscall_ulong_t __glibc_reserved ## RES
> +#else
> +# define __SEM_PAD_TIME(NAME, RES)    \
> +  __time_t NAME
> +#endif
> +
> +/* Data structure describing a set of semaphores.  */
> +struct semid_ds
> +{
> +  struct ipc_perm sem_perm;		/* operation permission struct */
> +  __SEM_PAD_TIME (sem_otime, 1);	/* last semop() time */
> +  __SEM_PAD_TIME (sem_ctime, 2);	/* 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;
> +};
> 

Ok.

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/3] semctl: Remove the sem-pad.h file
  2020-04-01 16:53 ` [PATCH v5 2/3] semctl: Remove the sem-pad.h file Alistair Francis
@ 2020-04-17 19:03   ` Adhemerval Zanella
  2020-04-17 20:38     ` Alistair Francis
  2020-04-22 19:22   ` Adhemerval Zanella
  1 sibling, 1 reply; 16+ messages in thread
From: Adhemerval Zanella @ 2020-04-17 19:03 UTC (permalink / raw)
  To: libc-alpha



On 01/04/2020 13:53, Alistair Francis via Libc-alpha wrote:
> Remove the sem-pad.h file and instead have architectures override the
> struct semid_ds via the bits/types/struct_semid_ds.h file.

Ok with the remarks below.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/unix/sysv/linux/Makefile              |  2 +-
>  sysdeps/unix/sysv/linux/bits/sem-pad.h        | 33 -------------
>  sysdeps/unix/sysv/linux/bits/sem.h            |  2 +-
>  .../sysv/linux/bits/types/struct_semid_ds.h   | 29 ++++++------
>  .../linux/hppa/bits/types/struct_semid_ds.h   | 46 +++++++++++++++++++
>  sysdeps/unix/sysv/linux/mips/bits/sem-pad.h   | 24 ----------
>  .../bits/types/struct_semid_ds.h}             | 20 +++++---
>  .../unix/sysv/linux/powerpc/bits/sem-pad.h    | 26 -----------
>  .../powerpc/bits/types/struct_semid_ds.h      | 46 +++++++++++++++++++
>  .../linux/sparc/bits/types/struct_semid_ds.h  | 46 +++++++++++++++++++
>  sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    | 24 ----------
>  .../bits/types/struct_semid_ds.h}             | 22 ++++++---
>  12 files changed, 184 insertions(+), 136 deletions(-)
>  delete mode 100644 sysdeps/unix/sysv/linux/bits/sem-pad.h
>  create mode 100644 sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h
>  delete mode 100644 sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
>  rename sysdeps/unix/sysv/linux/{sparc/bits/sem-pad.h => mips/bits/types/struct_semid_ds.h} (56%)
>  delete mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
>  create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h
>  create mode 100644 sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h
>  delete mode 100644 sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
>  rename sysdeps/unix/sysv/linux/{hppa/bits/sem-pad.h => x86/bits/types/struct_semid_ds.h} (52%)
> 
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index bbf1d60fc6..84ed4344d3 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -86,7 +86,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
>  		  bits/siginfo-arch.h bits/siginfo-consts-arch.h \
>  		  bits/procfs.h bits/procfs-id.h bits/procfs-extra.h \
>  		  bits/procfs-prregset.h bits/mman-map-flags-generic.h \
> -		  bits/msq-pad.h bits/sem-pad.h bits/shmlba.h bits/shm-pad.h \
> +		  bits/msq-pad.h bits/shmlba.h bits/shm-pad.h \
>  		  bits/termios-struct.h bits/termios-c_cc.h \
>  		  bits/termios-c_iflag.h bits/termios-c_oflag.h \
>  		  bits/termios-baud.h bits/termios-c_cflag.h \

Ok.

> diff --git a/sysdeps/unix/sysv/linux/bits/sem-pad.h b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> deleted file mode 100644
> index 566ce039cc..0000000000
> --- a/sysdeps/unix/sysv/linux/bits/sem-pad.h
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -/* Define where padding goes in struct semid_ds.  Generic version.
> -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#ifndef _SYS_SEM_H
> -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> -#endif
> -
> -#include <bits/timesize.h>
> -
> -/* On most architectures, padding goes after time fields for 32-bit
> -   systems and is omitted for 64-bit systems.  Some architectures pad
> -   before time fields instead, or omit padding despite being 32-bit,
> -   or include it despite being 64-bit.  This must match the layout
> -   used for struct semid64_ds in <asm/sembuf.h>, as glibc does not do
> -   layout conversions for this structure.  */
> -
> -#define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
> -#define __SEM_PAD_BEFORE_TIME 0

Ok.

> diff --git a/sysdeps/unix/sysv/linux/bits/sem.h b/sysdeps/unix/sysv/linux/bits/sem.h
> index 0d1813ec67..ba1169fdb3 100644
> --- a/sysdeps/unix/sysv/linux/bits/sem.h
> +++ b/sysdeps/unix/sysv/linux/bits/sem.h
> @@ -20,7 +20,7 @@
>  #endif
>  
>  #include <sys/types.h>
> -#include <bits/sem-pad.h>
> +#include <bits/timesize.h>
>  #include <bits/types/struct_semid_ds.h>
>  
>  /* Flags for `semop'.  */

Ok.

> diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> index ba0719e77a..659db85db8 100644
> --- a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> +++ b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> @@ -20,24 +20,27 @@
>  # error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
>  #endif
>  
> -#if __SEM_PAD_BEFORE_TIME
> -# define __SEM_PAD_TIME(NAME, RES)        \
> -  __syscall_ulong_t __glibc_reserved ## RES; __time_t NAME
> -#elif __SEM_PAD_AFTER_TIME
> -# define __SEM_PAD_TIME(NAME, RES)        \
> -  __time_t NAME; __syscall_ulong_t __glibc_reserved ## RES
> -#else
> -# define __SEM_PAD_TIME(NAME, RES)    \
> -  __time_t NAME
> -#endif
> -
>  /* Data structure describing a set of semaphores.  */
> +#if __TIMESIZE == 32
> +struct semid_ds
> +{
> +  struct ipc_perm sem_perm;        /* operation permission struct */
> +  __time_t sem_otime;              /* last semop() time */
> +  __syscall_ulong_t __glibc_reserved1;
> +  __time_t sem_ctime;             /* last time changed by semctl() */
> +  __syscall_ulong_t __glibc_reserved2;
> +  __syscall_ulong_t sem_nsems;    /* number of semaphores in set */
> +  __syscall_ulong_t __glibc_reserved3;
> +  __syscall_ulong_t __glibc_reserved4;
> +};
> +#else
>  struct semid_ds
>  {
>    struct ipc_perm sem_perm;		/* operation permission struct */
> -  __SEM_PAD_TIME (sem_otime, 1);	/* last semop() time */
> -  __SEM_PAD_TIME (sem_ctime, 2);	/* last time changed by semctl() */
> +  __time_t sem_otime;	        /* last semop() time */
> +  __time_t sem_ctime;	        /* last time changed by semctl() */

Shouldn't be __time64_t?

>    __syscall_ulong_t sem_nsems;		/* number of semaphores in set */
>    __syscall_ulong_t __glibc_reserved3;
>    __syscall_ulong_t __glibc_reserved4;
>  };
> +#endif> diff --git a/sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h
> new file mode 100644
> index 0000000000..fbc26ef2ca
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h
> @@ -0,0 +1,46 @@
> +/* HPPA implementation of the semaphore struct semid_ds
> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_SEM_H
> +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
> +#endif
> +
> +/* Data structure describing a set of semaphores.  */
> +#if __TIMESIZE == 32
> +struct semid_ds
> +{
> +  struct ipc_perm sem_perm;   /* operation permission struct */
> +  __syscall_ulong_t __glibc_reserved1;
> +  __time_t sem_otime;         /* last semop() time */
> +  __syscall_ulong_t __glibc_reserved2;
> +  __time_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;
> +};

Ok.

> +#else
> +struct semid_ds
> +{
> +  struct ipc_perm sem_perm;		/* operation permission struct */
> +  __time_t sem_otime;       	/* last semop() time */
> +  __time_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;
> +};
> +#endif

There is no support for 64-bit userland neither it is a valid glibc
support ABI, so we can remove it.

> diff --git a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> deleted file mode 100644
> index 4c581f7694..0000000000
> --- a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/* Define where padding goes in struct semid_ds.  MIPS version.
> -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#ifndef _SYS_SEM_H
> -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> -#endif
> -
> -#define __SEM_PAD_AFTER_TIME 0
> -#define __SEM_PAD_BEFORE_TIME 0

Ok.

> diff --git a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/mips/bits/types/struct_semid_ds.h
> similarity index 56%
> rename from sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> rename to sysdeps/unix/sysv/linux/mips/bits/types/struct_semid_ds.h
> index 5f4e214d12..8954209a29 100644
> --- a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/mips/bits/types/struct_semid_ds.h
> @@ -1,5 +1,5 @@
> -/* Define where padding goes in struct semid_ds.  SPARC version.
> -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> +/* MIPS implementation of the semaphore struct semid_ds
> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -17,10 +17,16 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #ifndef _SYS_SEM_H
> -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
>  #endif
>  
> -#include <bits/timesize.h>
> -
> -#define __SEM_PAD_AFTER_TIME 0
> -#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +/* Data structure describing a set of semaphores.  */
> +struct semid_ds
> +{
> +  struct ipc_perm sem_perm;		/* operation permission struct */
> +  __time_t sem_otime;	/* last semop() time */
> +  __time_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;
> +};

Ok.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> deleted file mode 100644
> index 42d8827906..0000000000
> --- a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -/* Define where padding goes in struct semid_ds.  PowerPC version.
> -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#ifndef _SYS_SEM_H
> -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> -#endif
> -
> -#include <bits/timesize.h>
> -
> -#define __SEM_PAD_AFTER_TIME 0
> -#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)

Ok.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h
> new file mode 100644
> index 0000000000..d393141808
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h
> @@ -0,0 +1,46 @@
> +/* PowerPC implementation of the semaphore struct semid_ds

Missing period.

> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_SEM_H
> +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
> +#endif
> +
> +/* Data structure describing a set of semaphores.  */
> +#if __TIMESIZE == 32
> +struct semid_ds
> +{
> +  struct ipc_perm sem_perm;   /* operation permission struct */
> +  __syscall_ulong_t __glibc_reserved1;
> +  __time_t sem_otime;  /* last semop() time */
> +  __syscall_ulong_t __glibc_reserved2;
> +  __time_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_ds
> +{
> +  struct ipc_perm sem_perm;		/* operation permission struct */
> +  __time_t sem_otime;	/* last semop() time */
> +  __time_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;
> +};
> +#endif

Ok, although it can be simplified to:

  struct semid_ds
  {
    struct ipc_perm sem_perm;
  #if __TIMESIZE == 32
    __syscall_ulong_t __glibc_reserved1;
    __time_t          sem_otime;
    __syscall_ulong_t __glibc_reserved2;
    __time_t          sem_ctime;
  #else
    __time_t          sem_otime;
    __time_t          sem_ctime;
  #endif
    __syscall_ulong_t sem_nsems;
    __syscall_ulong_t __glibc_reserved3;
    __syscall_ulong_t __glibc_reserved4; 
  };

> diff --git a/sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h
> new file mode 100644
> index 0000000000..84c7a9022a
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h
> @@ -0,0 +1,46 @@
> +/* Sparc implementation of the semaphore struct semid_ds
> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_SEM_H
> +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
> +#endif
> +
> +/* Data structure describing a set of semaphores.  */
> +#if __TIMESIZE == 32
> +struct semid_ds
> +{
> +  struct ipc_perm sem_perm;   /* operation permission struct */
> +  __syscall_ulong_t __glibc_reserved1;
> +  __time_t sem_otime;  /* last semop() time */
> +  __syscall_ulong_t __glibc_reserved2;
> +  __time_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_ds
> +{
> +  struct ipc_perm sem_perm;		/* operation permission struct */
> +  __time_t sem_otime;	/* last semop() time */
> +  __time_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;
> +};
> +#endif

Ok, although it also can be simplified as the powerpc one.

> diff --git a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> deleted file mode 100644
> index 102e226997..0000000000
> --- a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/* Define where padding goes in struct semid_ds.  x86 version.
> -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#ifndef _SYS_SEM_H
> -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> -#endif
> -
> -#define __SEM_PAD_AFTER_TIME 1
> -#define __SEM_PAD_BEFORE_TIME 0

Ok.

> diff --git a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
> similarity index 52%
> rename from sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> rename to sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
> index ee0332325b..22f0645f85 100644
> --- a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
> @@ -1,5 +1,5 @@
> -/* Define where padding goes in struct semid_ds.  HPPA version.
> -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> +/* Sparc implementation of the semaphore struct semid_ds
> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -17,10 +17,18 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #ifndef _SYS_SEM_H
> -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
>  #endif
>  
> -#include <bits/timesize.h>
> -
> -#define __SEM_PAD_AFTER_TIME 0
> -#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +/* Data structure describing a set of semaphores.  */
> +struct semid_ds
> +{
> +  struct ipc_perm sem_perm;   /* operation permission struct */
> +  __time_t sem_otime;  /* last semop() time */
> +  __syscall_ulong_t __glibc_reserved1;
> +  __time_t sem_ctime;  /* last time changed by semctl() */
> +  __syscall_ulong_t __glibc_reserved2;
> +  __syscall_ulong_t sem_nsems;    /* number of semaphores in set */
> +  __syscall_ulong_t __glibc_reserved3;
> +  __syscall_ulong_t __glibc_reserved4;
> +};
> 

Ok.


^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/3] semctl: Remove the sem-pad.h file
  2020-04-17 19:03   ` Adhemerval Zanella
@ 2020-04-17 20:38     ` Alistair Francis
  2020-04-20 20:18       ` Adhemerval Zanella
  0 siblings, 1 reply; 16+ messages in thread
From: Alistair Francis @ 2020-04-17 20:38 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library

On Fri, Apr 17, 2020 at 12:03 PM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 01/04/2020 13:53, Alistair Francis via Libc-alpha wrote:
> > Remove the sem-pad.h file and instead have architectures override the
> > struct semid_ds via the bits/types/struct_semid_ds.h file.
>
> Ok with the remarks below.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> > ---
> >  sysdeps/unix/sysv/linux/Makefile              |  2 +-
> >  sysdeps/unix/sysv/linux/bits/sem-pad.h        | 33 -------------
> >  sysdeps/unix/sysv/linux/bits/sem.h            |  2 +-
> >  .../sysv/linux/bits/types/struct_semid_ds.h   | 29 ++++++------
> >  .../linux/hppa/bits/types/struct_semid_ds.h   | 46 +++++++++++++++++++
> >  sysdeps/unix/sysv/linux/mips/bits/sem-pad.h   | 24 ----------
> >  .../bits/types/struct_semid_ds.h}             | 20 +++++---
> >  .../unix/sysv/linux/powerpc/bits/sem-pad.h    | 26 -----------
> >  .../powerpc/bits/types/struct_semid_ds.h      | 46 +++++++++++++++++++
> >  .../linux/sparc/bits/types/struct_semid_ds.h  | 46 +++++++++++++++++++
> >  sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    | 24 ----------
> >  .../bits/types/struct_semid_ds.h}             | 22 ++++++---
> >  12 files changed, 184 insertions(+), 136 deletions(-)
> >  delete mode 100644 sysdeps/unix/sysv/linux/bits/sem-pad.h
> >  create mode 100644 sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h
> >  delete mode 100644 sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> >  rename sysdeps/unix/sysv/linux/{sparc/bits/sem-pad.h => mips/bits/types/struct_semid_ds.h} (56%)
> >  delete mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> >  create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h
> >  create mode 100644 sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h
> >  delete mode 100644 sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> >  rename sysdeps/unix/sysv/linux/{hppa/bits/sem-pad.h => x86/bits/types/struct_semid_ds.h} (52%)
> >
> > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> > index bbf1d60fc6..84ed4344d3 100644
> > --- a/sysdeps/unix/sysv/linux/Makefile
> > +++ b/sysdeps/unix/sysv/linux/Makefile
> > @@ -86,7 +86,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
> >                 bits/siginfo-arch.h bits/siginfo-consts-arch.h \
> >                 bits/procfs.h bits/procfs-id.h bits/procfs-extra.h \
> >                 bits/procfs-prregset.h bits/mman-map-flags-generic.h \
> > -               bits/msq-pad.h bits/sem-pad.h bits/shmlba.h bits/shm-pad.h \
> > +               bits/msq-pad.h bits/shmlba.h bits/shm-pad.h \
> >                 bits/termios-struct.h bits/termios-c_cc.h \
> >                 bits/termios-c_iflag.h bits/termios-c_oflag.h \
> >                 bits/termios-baud.h bits/termios-c_cflag.h \
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/bits/sem-pad.h b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > deleted file mode 100644
> > index 566ce039cc..0000000000
> > --- a/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > +++ /dev/null
> > @@ -1,33 +0,0 @@
> > -/* Define where padding goes in struct semid_ds.  Generic version.
> > -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> > -   This file is part of the GNU C Library.
> > -
> > -   The GNU C Library is free software; you can redistribute it and/or
> > -   modify it under the terms of the GNU Lesser General Public
> > -   License as published by the Free Software Foundation; either
> > -   version 2.1 of the License, or (at your option) any later version.
> > -
> > -   The GNU C Library is distributed in the hope that it will be useful,
> > -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > -   Lesser General Public License for more details.
> > -
> > -   You should have received a copy of the GNU Lesser General Public
> > -   License along with the GNU C Library; if not, see
> > -   <https://www.gnu.org/licenses/>.  */
> > -
> > -#ifndef _SYS_SEM_H
> > -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> > -#endif
> > -
> > -#include <bits/timesize.h>
> > -
> > -/* On most architectures, padding goes after time fields for 32-bit
> > -   systems and is omitted for 64-bit systems.  Some architectures pad
> > -   before time fields instead, or omit padding despite being 32-bit,
> > -   or include it despite being 64-bit.  This must match the layout
> > -   used for struct semid64_ds in <asm/sembuf.h>, as glibc does not do
> > -   layout conversions for this structure.  */
> > -
> > -#define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
> > -#define __SEM_PAD_BEFORE_TIME 0
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/bits/sem.h b/sysdeps/unix/sysv/linux/bits/sem.h
> > index 0d1813ec67..ba1169fdb3 100644
> > --- a/sysdeps/unix/sysv/linux/bits/sem.h
> > +++ b/sysdeps/unix/sysv/linux/bits/sem.h
> > @@ -20,7 +20,7 @@
> >  #endif
> >
> >  #include <sys/types.h>
> > -#include <bits/sem-pad.h>
> > +#include <bits/timesize.h>
> >  #include <bits/types/struct_semid_ds.h>
> >
> >  /* Flags for `semop'.  */
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> > index ba0719e77a..659db85db8 100644
> > --- a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> > +++ b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> > @@ -20,24 +20,27 @@
> >  # error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
> >  #endif
> >
> > -#if __SEM_PAD_BEFORE_TIME
> > -# define __SEM_PAD_TIME(NAME, RES)        \
> > -  __syscall_ulong_t __glibc_reserved ## RES; __time_t NAME
> > -#elif __SEM_PAD_AFTER_TIME
> > -# define __SEM_PAD_TIME(NAME, RES)        \
> > -  __time_t NAME; __syscall_ulong_t __glibc_reserved ## RES
> > -#else
> > -# define __SEM_PAD_TIME(NAME, RES)    \
> > -  __time_t NAME
> > -#endif
> > -
> >  /* Data structure describing a set of semaphores.  */
> > +#if __TIMESIZE == 32
> > +struct semid_ds
> > +{
> > +  struct ipc_perm sem_perm;        /* operation permission struct */
> > +  __time_t sem_otime;              /* last semop() time */
> > +  __syscall_ulong_t __glibc_reserved1;
> > +  __time_t sem_ctime;             /* last time changed by semctl() */
> > +  __syscall_ulong_t __glibc_reserved2;
> > +  __syscall_ulong_t sem_nsems;    /* number of semaphores in set */
> > +  __syscall_ulong_t __glibc_reserved3;
> > +  __syscall_ulong_t __glibc_reserved4;
> > +};
> > +#else
> >  struct semid_ds
> >  {
> >    struct ipc_perm sem_perm;          /* operation permission struct */
> > -  __SEM_PAD_TIME (sem_otime, 1);     /* last semop() time */
> > -  __SEM_PAD_TIME (sem_ctime, 2);     /* last time changed by semctl() */
> > +  __time_t sem_otime;                /* last semop() time */
> > +  __time_t sem_ctime;                /* last time changed by semctl() */
>
> Shouldn't be __time64_t?

They should be the same as we are outside the __TIMESIZE == 32 define,
time_t just seemed more generic.

Do you want me to change it?

>
> >    __syscall_ulong_t sem_nsems;               /* number of semaphores in set */
> >    __syscall_ulong_t __glibc_reserved3;
> >    __syscall_ulong_t __glibc_reserved4;
> >  };
> > +#endif> diff --git a/sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h
> > new file mode 100644
> > index 0000000000..fbc26ef2ca
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/hppa/bits/types/struct_semid_ds.h
> > @@ -0,0 +1,46 @@
> > +/* HPPA implementation of the semaphore struct semid_ds
> > +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#ifndef _SYS_SEM_H
> > +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
> > +#endif
> > +
> > +/* Data structure describing a set of semaphores.  */
> > +#if __TIMESIZE == 32
> > +struct semid_ds
> > +{
> > +  struct ipc_perm sem_perm;   /* operation permission struct */
> > +  __syscall_ulong_t __glibc_reserved1;
> > +  __time_t sem_otime;         /* last semop() time */
> > +  __syscall_ulong_t __glibc_reserved2;
> > +  __time_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;
> > +};
>
> Ok.
>
> > +#else
> > +struct semid_ds
> > +{
> > +  struct ipc_perm sem_perm;          /* operation permission struct */
> > +  __time_t sem_otime;        /* last semop() time */
> > +  __time_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;
> > +};
> > +#endif
>
> There is no support for 64-bit userland neither it is a valid glibc
> support ABI, so we can remove it.

Removed!

>
> > diff --git a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> > deleted file mode 100644
> > index 4c581f7694..0000000000
> > --- a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> > +++ /dev/null
> > @@ -1,24 +0,0 @@
> > -/* Define where padding goes in struct semid_ds.  MIPS version.
> > -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> > -   This file is part of the GNU C Library.
> > -
> > -   The GNU C Library is free software; you can redistribute it and/or
> > -   modify it under the terms of the GNU Lesser General Public
> > -   License as published by the Free Software Foundation; either
> > -   version 2.1 of the License, or (at your option) any later version.
> > -
> > -   The GNU C Library is distributed in the hope that it will be useful,
> > -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > -   Lesser General Public License for more details.
> > -
> > -   You should have received a copy of the GNU Lesser General Public
> > -   License along with the GNU C Library; if not, see
> > -   <https://www.gnu.org/licenses/>.  */
> > -
> > -#ifndef _SYS_SEM_H
> > -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> > -#endif
> > -
> > -#define __SEM_PAD_AFTER_TIME 0
> > -#define __SEM_PAD_BEFORE_TIME 0
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/mips/bits/types/struct_semid_ds.h
> > similarity index 56%
> > rename from sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> > rename to sysdeps/unix/sysv/linux/mips/bits/types/struct_semid_ds.h
> > index 5f4e214d12..8954209a29 100644
> > --- a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/mips/bits/types/struct_semid_ds.h
> > @@ -1,5 +1,5 @@
> > -/* Define where padding goes in struct semid_ds.  SPARC version.
> > -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> > +/* MIPS implementation of the semaphore struct semid_ds
> > +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> >     This file is part of the GNU C Library.
> >
> >     The GNU C Library is free software; you can redistribute it and/or
> > @@ -17,10 +17,16 @@
> >     <https://www.gnu.org/licenses/>.  */
> >
> >  #ifndef _SYS_SEM_H
> > -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> > +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
> >  #endif
> >
> > -#include <bits/timesize.h>
> > -
> > -#define __SEM_PAD_AFTER_TIME 0
> > -#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +/* Data structure describing a set of semaphores.  */
> > +struct semid_ds
> > +{
> > +  struct ipc_perm sem_perm;          /* operation permission struct */
> > +  __time_t sem_otime;        /* last semop() time */
> > +  __time_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;
> > +};
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> > deleted file mode 100644
> > index 42d8827906..0000000000
> > --- a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> > +++ /dev/null
> > @@ -1,26 +0,0 @@
> > -/* Define where padding goes in struct semid_ds.  PowerPC version.
> > -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> > -   This file is part of the GNU C Library.
> > -
> > -   The GNU C Library is free software; you can redistribute it and/or
> > -   modify it under the terms of the GNU Lesser General Public
> > -   License as published by the Free Software Foundation; either
> > -   version 2.1 of the License, or (at your option) any later version.
> > -
> > -   The GNU C Library is distributed in the hope that it will be useful,
> > -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > -   Lesser General Public License for more details.
> > -
> > -   You should have received a copy of the GNU Lesser General Public
> > -   License along with the GNU C Library; if not, see
> > -   <https://www.gnu.org/licenses/>.  */
> > -
> > -#ifndef _SYS_SEM_H
> > -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> > -#endif
> > -
> > -#include <bits/timesize.h>
> > -
> > -#define __SEM_PAD_AFTER_TIME 0
> > -#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h
> > new file mode 100644
> > index 0000000000..d393141808
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/powerpc/bits/types/struct_semid_ds.h
> > @@ -0,0 +1,46 @@
> > +/* PowerPC implementation of the semaphore struct semid_ds
>
> Missing period.

Added

>
> > +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#ifndef _SYS_SEM_H
> > +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
> > +#endif
> > +
> > +/* Data structure describing a set of semaphores.  */
> > +#if __TIMESIZE == 32
> > +struct semid_ds
> > +{
> > +  struct ipc_perm sem_perm;   /* operation permission struct */
> > +  __syscall_ulong_t __glibc_reserved1;
> > +  __time_t sem_otime;  /* last semop() time */
> > +  __syscall_ulong_t __glibc_reserved2;
> > +  __time_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_ds
> > +{
> > +  struct ipc_perm sem_perm;          /* operation permission struct */
> > +  __time_t sem_otime;        /* last semop() time */
> > +  __time_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;
> > +};
> > +#endif
>
> Ok, although it can be simplified to:
>
>   struct semid_ds
>   {
>     struct ipc_perm sem_perm;
>   #if __TIMESIZE == 32
>     __syscall_ulong_t __glibc_reserved1;
>     __time_t          sem_otime;
>     __syscall_ulong_t __glibc_reserved2;
>     __time_t          sem_ctime;
>   #else
>     __time_t          sem_otime;
>     __time_t          sem_ctime;
>   #endif
>     __syscall_ulong_t sem_nsems;
>     __syscall_ulong_t __glibc_reserved3;
>     __syscall_ulong_t __glibc_reserved4;
>   };

Done

>
> > diff --git a/sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h
> > new file mode 100644
> > index 0000000000..84c7a9022a
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/sparc/bits/types/struct_semid_ds.h
> > @@ -0,0 +1,46 @@
> > +/* Sparc implementation of the semaphore struct semid_ds
> > +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#ifndef _SYS_SEM_H
> > +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
> > +#endif
> > +
> > +/* Data structure describing a set of semaphores.  */
> > +#if __TIMESIZE == 32
> > +struct semid_ds
> > +{
> > +  struct ipc_perm sem_perm;   /* operation permission struct */
> > +  __syscall_ulong_t __glibc_reserved1;
> > +  __time_t sem_otime;  /* last semop() time */
> > +  __syscall_ulong_t __glibc_reserved2;
> > +  __time_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_ds
> > +{
> > +  struct ipc_perm sem_perm;          /* operation permission struct */
> > +  __time_t sem_otime;        /* last semop() time */
> > +  __time_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;
> > +};
> > +#endif
>
> Ok, although it also can be simplified as the powerpc one.

Done.

Thanks for the review.

Alistair

>
> > diff --git a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> > deleted file mode 100644
> > index 102e226997..0000000000
> > --- a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> > +++ /dev/null
> > @@ -1,24 +0,0 @@
> > -/* Define where padding goes in struct semid_ds.  x86 version.
> > -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> > -   This file is part of the GNU C Library.
> > -
> > -   The GNU C Library is free software; you can redistribute it and/or
> > -   modify it under the terms of the GNU Lesser General Public
> > -   License as published by the Free Software Foundation; either
> > -   version 2.1 of the License, or (at your option) any later version.
> > -
> > -   The GNU C Library is distributed in the hope that it will be useful,
> > -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > -   Lesser General Public License for more details.
> > -
> > -   You should have received a copy of the GNU Lesser General Public
> > -   License along with the GNU C Library; if not, see
> > -   <https://www.gnu.org/licenses/>.  */
> > -
> > -#ifndef _SYS_SEM_H
> > -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> > -#endif
> > -
> > -#define __SEM_PAD_AFTER_TIME 1
> > -#define __SEM_PAD_BEFORE_TIME 0
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
> > similarity index 52%
> > rename from sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> > rename to sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
> > index ee0332325b..22f0645f85 100644
> > --- a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
> > @@ -1,5 +1,5 @@
> > -/* Define where padding goes in struct semid_ds.  HPPA version.
> > -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> > +/* Sparc implementation of the semaphore struct semid_ds
> > +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> >     This file is part of the GNU C Library.
> >
> >     The GNU C Library is free software; you can redistribute it and/or
> > @@ -17,10 +17,18 @@
> >     <https://www.gnu.org/licenses/>.  */
> >
> >  #ifndef _SYS_SEM_H
> > -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> > +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
> >  #endif
> >
> > -#include <bits/timesize.h>
> > -
> > -#define __SEM_PAD_AFTER_TIME 0
> > -#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +/* Data structure describing a set of semaphores.  */
> > +struct semid_ds
> > +{
> > +  struct ipc_perm sem_perm;   /* operation permission struct */
> > +  __time_t sem_otime;  /* last semop() time */
> > +  __syscall_ulong_t __glibc_reserved1;
> > +  __time_t sem_ctime;  /* last time changed by semctl() */
> > +  __syscall_ulong_t __glibc_reserved2;
> > +  __syscall_ulong_t sem_nsems;    /* number of semaphores in set */
> > +  __syscall_ulong_t __glibc_reserved3;
> > +  __syscall_ulong_t __glibc_reserved4;
> > +};
> >
>
> Ok.
>

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/3] semctl: Remove the sem-pad.h file
  2020-04-17 20:38     ` Alistair Francis
@ 2020-04-20 20:18       ` Adhemerval Zanella
  2020-04-20 20:25         ` Alistair Francis
  0 siblings, 1 reply; 16+ messages in thread
From: Adhemerval Zanella @ 2020-04-20 20:18 UTC (permalink / raw)
  To: Alistair Francis; +Cc: GNU C Library



On 17/04/2020 17:38, Alistair Francis wrote:
> On Fri, Apr 17, 2020 at 12:03 PM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>>
>>
>> On 01/04/2020 13:53, Alistair Francis via Libc-alpha wrote:
>>> Remove the sem-pad.h file and instead have architectures override the
>>> struct semid_ds via the bits/types/struct_semid_ds.h file.
>>
>> Ok with the remarks below.
>>
>> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>

>>
>>> diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
>>> index ba0719e77a..659db85db8 100644
>>> --- a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
>>> +++ b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
>>> @@ -20,24 +20,27 @@
>>>  # error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
>>>  #endif
>>>
>>> -#if __SEM_PAD_BEFORE_TIME
>>> -# define __SEM_PAD_TIME(NAME, RES)        \
>>> -  __syscall_ulong_t __glibc_reserved ## RES; __time_t NAME
>>> -#elif __SEM_PAD_AFTER_TIME
>>> -# define __SEM_PAD_TIME(NAME, RES)        \
>>> -  __time_t NAME; __syscall_ulong_t __glibc_reserved ## RES
>>> -#else
>>> -# define __SEM_PAD_TIME(NAME, RES)    \
>>> -  __time_t NAME
>>> -#endif
>>> -
>>>  /* Data structure describing a set of semaphores.  */
>>> +#if __TIMESIZE == 32
>>> +struct semid_ds
>>> +{
>>> +  struct ipc_perm sem_perm;        /* operation permission struct */
>>> +  __time_t sem_otime;              /* last semop() time */
>>> +  __syscall_ulong_t __glibc_reserved1;
>>> +  __time_t sem_ctime;             /* last time changed by semctl() */
>>> +  __syscall_ulong_t __glibc_reserved2;
>>> +  __syscall_ulong_t sem_nsems;    /* number of semaphores in set */
>>> +  __syscall_ulong_t __glibc_reserved3;
>>> +  __syscall_ulong_t __glibc_reserved4;
>>> +};
>>> +#else
>>>  struct semid_ds
>>>  {
>>>    struct ipc_perm sem_perm;          /* operation permission struct */
>>> -  __SEM_PAD_TIME (sem_otime, 1);     /* last semop() time */
>>> -  __SEM_PAD_TIME (sem_ctime, 2);     /* last time changed by semctl() */
>>> +  __time_t sem_otime;                /* last semop() time */
>>> +  __time_t sem_ctime;                /* last time changed by semctl() */
>>
>> Shouldn't be __time64_t?
> 
> They should be the same as we are outside the __TIMESIZE == 32 define,
> time_t just seemed more generic.
> 
> Do you want me to change it?

Alright, it we can adjust once 64-bit time_t support is added on SysV.

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/3] semctl: Remove the sem-pad.h file
  2020-04-20 20:18       ` Adhemerval Zanella
@ 2020-04-20 20:25         ` Alistair Francis
  2020-04-21 14:03           ` Adhemerval Zanella
  0 siblings, 1 reply; 16+ messages in thread
From: Alistair Francis @ 2020-04-20 20:25 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library

On Mon, Apr 20, 2020 at 1:18 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 17/04/2020 17:38, Alistair Francis wrote:
> > On Fri, Apr 17, 2020 at 12:03 PM Adhemerval Zanella via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >>
> >>
> >>
> >> On 01/04/2020 13:53, Alistair Francis via Libc-alpha wrote:
> >>> Remove the sem-pad.h file and instead have architectures override the
> >>> struct semid_ds via the bits/types/struct_semid_ds.h file.
> >>
> >> Ok with the remarks below.
> >>
> >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> >>
>
> >>
> >>> diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> >>> index ba0719e77a..659db85db8 100644
> >>> --- a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> >>> +++ b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
> >>> @@ -20,24 +20,27 @@
> >>>  # error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
> >>>  #endif
> >>>
> >>> -#if __SEM_PAD_BEFORE_TIME
> >>> -# define __SEM_PAD_TIME(NAME, RES)        \
> >>> -  __syscall_ulong_t __glibc_reserved ## RES; __time_t NAME
> >>> -#elif __SEM_PAD_AFTER_TIME
> >>> -# define __SEM_PAD_TIME(NAME, RES)        \
> >>> -  __time_t NAME; __syscall_ulong_t __glibc_reserved ## RES
> >>> -#else
> >>> -# define __SEM_PAD_TIME(NAME, RES)    \
> >>> -  __time_t NAME
> >>> -#endif
> >>> -
> >>>  /* Data structure describing a set of semaphores.  */
> >>> +#if __TIMESIZE == 32
> >>> +struct semid_ds
> >>> +{
> >>> +  struct ipc_perm sem_perm;        /* operation permission struct */
> >>> +  __time_t sem_otime;              /* last semop() time */
> >>> +  __syscall_ulong_t __glibc_reserved1;
> >>> +  __time_t sem_ctime;             /* last time changed by semctl() */
> >>> +  __syscall_ulong_t __glibc_reserved2;
> >>> +  __syscall_ulong_t sem_nsems;    /* number of semaphores in set */
> >>> +  __syscall_ulong_t __glibc_reserved3;
> >>> +  __syscall_ulong_t __glibc_reserved4;
> >>> +};
> >>> +#else
> >>>  struct semid_ds
> >>>  {
> >>>    struct ipc_perm sem_perm;          /* operation permission struct */
> >>> -  __SEM_PAD_TIME (sem_otime, 1);     /* last semop() time */
> >>> -  __SEM_PAD_TIME (sem_ctime, 2);     /* last time changed by semctl() */
> >>> +  __time_t sem_otime;                /* last semop() time */
> >>> +  __time_t sem_ctime;                /* last time changed by semctl() */
> >>
> >> Shouldn't be __time64_t?
> >
> > They should be the same as we are outside the __TIMESIZE == 32 define,
> > time_t just seemed more generic.
> >
> > Do you want me to change it?
>
> Alright, it we can adjust once 64-bit time_t support is added on SysV.

Ok, I'll leave it as is.

Do you mind reviewing patch 3 as well? Then I can merge this series.

Alistair

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/3] semctl: Remove the sem-pad.h file
  2020-04-20 20:25         ` Alistair Francis
@ 2020-04-21 14:03           ` Adhemerval Zanella
  0 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2020-04-21 14:03 UTC (permalink / raw)
  To: Alistair Francis; +Cc: GNU C Library



On 20/04/2020 17:25, Alistair Francis wrote:
> On Mon, Apr 20, 2020 at 1:18 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 17/04/2020 17:38, Alistair Francis wrote:
>>> On Fri, Apr 17, 2020 at 12:03 PM Adhemerval Zanella via Libc-alpha
>>> <libc-alpha@sourceware.org> wrote:
>>>>
>>>>
>>>>
>>>> On 01/04/2020 13:53, Alistair Francis via Libc-alpha wrote:
>>>>> Remove the sem-pad.h file and instead have architectures override the
>>>>> struct semid_ds via the bits/types/struct_semid_ds.h file.
>>>>
>>>> Ok with the remarks below.
>>>>
>>>> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>>>
>>
>>>>
>>>>> diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
>>>>> index ba0719e77a..659db85db8 100644
>>>>> --- a/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
>>>>> +++ b/sysdeps/unix/sysv/linux/bits/types/struct_semid_ds.h
>>>>> @@ -20,24 +20,27 @@
>>>>>  # error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
>>>>>  #endif
>>>>>
>>>>> -#if __SEM_PAD_BEFORE_TIME
>>>>> -# define __SEM_PAD_TIME(NAME, RES)        \
>>>>> -  __syscall_ulong_t __glibc_reserved ## RES; __time_t NAME
>>>>> -#elif __SEM_PAD_AFTER_TIME
>>>>> -# define __SEM_PAD_TIME(NAME, RES)        \
>>>>> -  __time_t NAME; __syscall_ulong_t __glibc_reserved ## RES
>>>>> -#else
>>>>> -# define __SEM_PAD_TIME(NAME, RES)    \
>>>>> -  __time_t NAME
>>>>> -#endif
>>>>> -
>>>>>  /* Data structure describing a set of semaphores.  */
>>>>> +#if __TIMESIZE == 32
>>>>> +struct semid_ds
>>>>> +{
>>>>> +  struct ipc_perm sem_perm;        /* operation permission struct */
>>>>> +  __time_t sem_otime;              /* last semop() time */
>>>>> +  __syscall_ulong_t __glibc_reserved1;
>>>>> +  __time_t sem_ctime;             /* last time changed by semctl() */
>>>>> +  __syscall_ulong_t __glibc_reserved2;
>>>>> +  __syscall_ulong_t sem_nsems;    /* number of semaphores in set */
>>>>> +  __syscall_ulong_t __glibc_reserved3;
>>>>> +  __syscall_ulong_t __glibc_reserved4;
>>>>> +};
>>>>> +#else
>>>>>  struct semid_ds
>>>>>  {
>>>>>    struct ipc_perm sem_perm;          /* operation permission struct */
>>>>> -  __SEM_PAD_TIME (sem_otime, 1);     /* last semop() time */
>>>>> -  __SEM_PAD_TIME (sem_ctime, 2);     /* last time changed by semctl() */
>>>>> +  __time_t sem_otime;                /* last semop() time */
>>>>> +  __time_t sem_ctime;                /* last time changed by semctl() */
>>>>
>>>> Shouldn't be __time64_t?
>>>
>>> They should be the same as we are outside the __TIMESIZE == 32 define,
>>> time_t just seemed more generic.
>>>
>>> Do you want me to change it?
>>
>> Alright, it we can adjust once 64-bit time_t support is added on SysV.
> 
> Ok, I'll leave it as is.
> 
> Do you mind reviewing patch 3 as well? Then I can merge this series.

Yes, that's my plan for today.

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-04-01 16:53 ` [PATCH v5 3/3] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
@ 2020-04-21 20:52   ` Stepan Golosunov
  2020-04-22 21:25     ` Adhemerval Zanella
  2020-04-22 21:25   ` Adhemerval Zanella
  1 sibling, 1 reply; 16+ messages in thread
From: Stepan Golosunov @ 2020-04-21 20:52 UTC (permalink / raw)
  To: Alistair Francis; +Cc: libc-alpha

01.04.2020 в 09:53:08 -0700 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 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.
> 
> Due to allignment differences between 64-bit and 32-bit variables we
> also need to set nsems to ensure it's correct.
> ---
>  .../unix/sysv/linux/hppa/struct__semid_ds32.h | 32 +++++++++++++++++++
>  .../unix/sysv/linux/mips/struct__semid_ds32.h | 30 +++++++++++++++++
>  .../sysv/linux/powerpc/struct__semid_ds32.h   | 32 +++++++++++++++++++
>  sysdeps/unix/sysv/linux/semctl.c              | 30 ++++++++++++++---
>  .../sysv/linux/sparc/struct__semid_ds32.h     | 32 +++++++++++++++++++
>  sysdeps/unix/sysv/linux/struct__semid_ds32.h  | 32 +++++++++++++++++++
>  .../unix/sysv/linux/x86/struct__semid_ds32.h  | 32 +++++++++++++++++++
>  7 files changed, 216 insertions(+), 4 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/mips/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h

> diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
> index 30571af49f..b0e4d828ed 100644
> --- a/sysdeps/unix/sysv/linux/semctl.c
> +++ b/sysdeps/unix/sysv/linux/semctl.c
> @@ -22,6 +22,7 @@
>  #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.  */
> @@ -29,6 +30,9 @@ union semun
>  {
>    int val;			/* value for SETVAL */
>    struct semid_ds *buf;		/* buffer for IPC_STAT & IPC_SET */
> +#if __WORDSIZE == 32
> +  struct __semid_ds32 *buf32;   /* 32-bit buffer for IPC_STAT */
> +#endif

This won't compile on x32.

>    unsigned short int *array;	/* array for GETALL & SETALL */
>    struct seminfo *__buf;	/* buffer for IPC_INFO */
>  };
> @@ -44,13 +48,31 @@ 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
> +
> +#if (__WORDSIZE == 32 && __TIMESIZE == 64 \
> +     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
> +  if (ret == 0 && (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY))
> +    {
> +      struct semid_ds temp_ds = (struct semid_ds) {
> +        .sem_perm = arg.buf32->sem_perm,
> +        .sem_nsems = arg.buf32->sem_nsems,
> +        .sem_otime = (arg.buf32->sem_otime
> +                      | ((time_t) arg.buf32->sem_otime_high << 32)),
> +        .sem_ctime = (arg.buf32->sem_ctime
> +                      | ((time_t) arg.buf32->sem_ctime_high << 32)),
> +      };
> +      *arg.buf = temp_ds;
> +    }
> +#endif
> +  return ret;
>  }

You do not need 2 temporary variables (temp_ds and an anonymous one).

      ….buf = (struct semid_ds) { … };
or
      struct semid_ds temp_ds = { … };


And while I am not too familiar with aliasing rules this still seems
to violate them (accessing the same memory via 2 pointers of
incompatible types).  Making this an undefined behavior.

What I was suggesting is something like this:

union semun
{
  int val;
  union { struct semid_ds s, struct __semid_ds32 s32 } *buf;
  unsigned short int *array;
  struct seminfo *__buf;
};

Or even with an anonymous union member:

union __semid_ds {
  struct semid_ds;
#if …
  struct __semid_ds32 buf32;
#endif
}
union semun
{
  int val;
  union __semid_ds *buf;
  unsigned short int *array;
  struct seminfo *__buf;
};

This should allow to access both structures simultaneously without
local violations of aliasing rules.  (I think that aliasing violations
between different compilation units are normal in glibc.  While inside
a single file they can lead to miscompilation.)


> diff --git a/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
> new file mode 100644
> index 0000000000..4e4ab26661
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
> @@ -0,0 +1,32 @@
> +/* Sparc implementation of the semaphore struct __semid_ds32
> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifdef __i386__
> +/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
> + * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
> +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

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/3] semctl: Remove the sem-pad.h file
  2020-04-01 16:53 ` [PATCH v5 2/3] semctl: Remove the sem-pad.h file Alistair Francis
  2020-04-17 19:03   ` Adhemerval Zanella
@ 2020-04-22 19:22   ` Adhemerval Zanella
  1 sibling, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2020-04-22 19:22 UTC (permalink / raw)
  To: libc-alpha

Below a missing nit I just spot.

> diff --git a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h b/sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
> similarity index 52%
> rename from sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> rename to sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
> index ee0332325b..22f0645f85 100644
> --- a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/x86/bits/types/struct_semid_ds.h
> @@ -1,5 +1,5 @@
> -/* Define where padding goes in struct semid_ds.  HPPA version.
> -   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> +/* Sparc implementation of the semaphore struct semid_ds

s/Sparc/x86 and missing period.

> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -17,10 +17,18 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #ifndef _SYS_SEM_H
> -# error "Never use <bits/sem-pad.h> directly; include <sys/sem.h> instead."
> +# error "Never include <bits/types/struct_semid_ds.h> directly; use <sys/sem.h> instead."
>  #endif
>  
> -#include <bits/timesize.h>
> -
> -#define __SEM_PAD_AFTER_TIME 0
> -#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +/* Data structure describing a set of semaphores.  */
> +struct semid_ds
> +{
> +  struct ipc_perm sem_perm;   /* operation permission struct */
> +  __time_t sem_otime;  /* last semop() time */
> +  __syscall_ulong_t __glibc_reserved1;
> +  __time_t sem_ctime;  /* last time changed by semctl() */
> +  __syscall_ulong_t __glibc_reserved2;
> +  __syscall_ulong_t sem_nsems;    /* number of semaphores in set */
> +  __syscall_ulong_t __glibc_reserved3;
> +  __syscall_ulong_t __glibc_reserved4;
> +};
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-04-21 20:52   ` Stepan Golosunov
@ 2020-04-22 21:25     ` Adhemerval Zanella
  2020-04-28 21:59       ` Alistair Francis
  0 siblings, 1 reply; 16+ messages in thread
From: Adhemerval Zanella @ 2020-04-22 21:25 UTC (permalink / raw)
  To: libc-alpha



On 21/04/2020 17:52, Stepan Golosunov wrote:
> 01.04.2020 в 09:53:08 -0700 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 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.
>>
>> Due to allignment differences between 64-bit and 32-bit variables we
>> also need to set nsems to ensure it's correct.
>> ---
>>  .../unix/sysv/linux/hppa/struct__semid_ds32.h | 32 +++++++++++++++++++
>>  .../unix/sysv/linux/mips/struct__semid_ds32.h | 30 +++++++++++++++++
>>  .../sysv/linux/powerpc/struct__semid_ds32.h   | 32 +++++++++++++++++++
>>  sysdeps/unix/sysv/linux/semctl.c              | 30 ++++++++++++++---
>>  .../sysv/linux/sparc/struct__semid_ds32.h     | 32 +++++++++++++++++++
>>  sysdeps/unix/sysv/linux/struct__semid_ds32.h  | 32 +++++++++++++++++++
>>  .../unix/sysv/linux/x86/struct__semid_ds32.h  | 32 +++++++++++++++++++
>>  7 files changed, 216 insertions(+), 4 deletions(-)
>>  create mode 100644 sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h
>>  create mode 100644 sysdeps/unix/sysv/linux/mips/struct__semid_ds32.h
>>  create mode 100644 sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h
>>  create mode 100644 sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h
>>  create mode 100644 sysdeps/unix/sysv/linux/struct__semid_ds32.h
>>  create mode 100644 sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
> 
>> diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
>> index 30571af49f..b0e4d828ed 100644
>> --- a/sysdeps/unix/sysv/linux/semctl.c
>> +++ b/sysdeps/unix/sysv/linux/semctl.c
>> @@ -22,6 +22,7 @@
>>  #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.  */
>> @@ -29,6 +30,9 @@ union semun
>>  {
>>    int val;			/* value for SETVAL */
>>    struct semid_ds *buf;		/* buffer for IPC_STAT & IPC_SET */
>> +#if __WORDSIZE == 32
>> +  struct __semid_ds32 *buf32;   /* 32-bit buffer for IPC_STAT */
>> +#endif
> 
> This won't compile on x32.

In fact it will, since the incomplete type won't be actually used on x32
and c99 struct initialization does not catch this (at least this patch
set does build with gcc 9.3 for x32).  But I agree that it not correct 
to actually define it for x32.

> 
>>    unsigned short int *array;	/* array for GETALL & SETALL */
>>    struct seminfo *__buf;	/* buffer for IPC_INFO */
>>  };
>> @@ -44,13 +48,31 @@ 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
>> +
>> +#if (__WORDSIZE == 32 && __TIMESIZE == 64 \
>> +     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
>> +  if (ret == 0 && (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY))
>> +    {
>> +      struct semid_ds temp_ds = (struct semid_ds) {
>> +        .sem_perm = arg.buf32->sem_perm,
>> +        .sem_nsems = arg.buf32->sem_nsems,
>> +        .sem_otime = (arg.buf32->sem_otime
>> +                      | ((time_t) arg.buf32->sem_otime_high << 32)),
>> +        .sem_ctime = (arg.buf32->sem_ctime
>> +                      | ((time_t) arg.buf32->sem_ctime_high << 32)),
>> +      };
>> +      *arg.buf = temp_ds;
>> +    }
>> +#endif
>> +  return ret;
>>  }
> 
> You do not need 2 temporary variables (temp_ds and an anonymous one).
> 
>       ….buf = (struct semid_ds) { … };
> or
>       struct semid_ds temp_ds = { … };
> 
> 
> And while I am not too familiar with aliasing rules this still seems
> to violate them (accessing the same memory via 2 pointers of
> incompatible types).  Making this an undefined behavior.

It uses a temporary buffer exactly to avoid it (similar to mips64 
getdents64).

However, the way previous the patch redefined semid_ds for 
__TIMESIZE == 64 does not take in consideration the 64-bit __time_t 
alignment (it might be case where sem_perm force padding on next
element). It makes the glibc exported semid_ds incompatible with
kernel one, which is not an issues but prevent glibc to pass it
directly to kernel in the syscall.

And there is another issue where prior v4.14 (commit
4693916846269d633a3664586650dbfac2c5562f) Linux left the reserved
space (used later to the high bits of 64-bit times) untouched rather
than zeroed.  This means that the caller must zero the kernel passed
buffer.

So both issues prevents to use the user provided buffer directly in
the syscall.  So what I think it would more straightforward to use a
temporary to issue the syscall.

Something like:

  #define IPC_TIME64 \
   (__WORDSIZE == 32 && __TIMESIZE == 64 \
       && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))

  static int
  semctl_syscall (int semid, int semnum, int cmd, union semun arg)
  {
    int ret;
  #if IPC_TIME64
    struct __semid_ds32 tmp, *orig;
    if (cmd == IPC_SET)
      {
        tmp = (struct __semid_ds32) {
          .sem_perm  = arg.buf->sem_perm,
          .sem_otime = arg.buf->sem_otime,
          .sem_ctime = arg.buf->sem_ctime,
          .sem_nsems = arg.buf->sem_nsems,
        };
        orig = arg.buf;
        arg.buf = &tmp;
      }
  #endif

  #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
    ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
                               arg.array);
  #else
    ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64,
                               SEMCTL_ARG_ADDRESS (arg));
  #endif

  #if IPC_TIME64
    if (r >= 0
        && (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY))
      {
        arg.buf = orig;
        arg.buf->sem_perm  = tmp.sem_perm;
        arg.buf->sem_otime = tmp.sem_otime
			     | ((time_t) tmp.sem_otime_high << 32);
        arg.buf->sem_ctime = tmp.sem_ctime
			     | ((time_t) tmp.sem_ctime_high << 32);
        arg.buf->sem_nsems = tmp.sem_nsems;
      }
  #endif

(the first part where the user provided buffer might require some adjustments
once __TIMESIZE == 64 is being exported in public ABI).

> 
> What I was suggesting is something like this:
> 
> union semun
> {
>   int val;
>   union { struct semid_ds s, struct __semid_ds32 s32 } *buf;
>   unsigned short int *array;
>   struct seminfo *__buf;
> };
> 
> Or even with an anonymous union member:
> 
> union __semid_ds {
>   struct semid_ds;
> #if …
>   struct __semid_ds32 buf32;
> #endif
> }
> union semun
> {
>   int val;
>   union __semid_ds *buf;
>   unsigned short int *array;
>   struct seminfo *__buf;
> };
> 
> This should allow to access both structures simultaneously without
> local violations of aliasing rules.  (I think that aliasing violations
> between different compilation units are normal in glibc.  While inside
> a single file they can lead to miscompilation.)
> 
> 
>> diff --git a/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
>> new file mode 100644
>> index 0000000000..4e4ab26661
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
>> @@ -0,0 +1,32 @@
>> +/* Sparc implementation of the semaphore struct __semid_ds32
>> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <https://www.gnu.org/licenses/>.  */
>> +
>> +#ifdef __i386__
>> +/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
>> + * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
>> +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

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-04-01 16:53 ` [PATCH v5 3/3] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
  2020-04-21 20:52   ` Stepan Golosunov
@ 2020-04-22 21:25   ` Adhemerval Zanella
  1 sibling, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2020-04-22 21:25 UTC (permalink / raw)
  To: libc-alpha



On 01/04/2020 13:53, Alistair Francis via Libc-alpha 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.
> 
> Due to allignment differences between 64-bit and 32-bit variables we
> also need to set nsems to ensure it's correct.

s/allignment/alignment.

> ---
>  .../unix/sysv/linux/hppa/struct__semid_ds32.h | 32 +++++++++++++++++++
>  .../unix/sysv/linux/mips/struct__semid_ds32.h | 30 +++++++++++++++++
>  .../sysv/linux/powerpc/struct__semid_ds32.h   | 32 +++++++++++++++++++
>  sysdeps/unix/sysv/linux/semctl.c              | 30 ++++++++++++++---
>  .../sysv/linux/sparc/struct__semid_ds32.h     | 32 +++++++++++++++++++
>  sysdeps/unix/sysv/linux/struct__semid_ds32.h  | 32 +++++++++++++++++++
>  .../unix/sysv/linux/x86/struct__semid_ds32.h  | 32 +++++++++++++++++++
>  7 files changed, 216 insertions(+), 4 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/mips/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/struct__semid_ds32.h
>  create mode 100644 sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
> 
> diff --git a/sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h
> new file mode 100644
> index 0000000000..db78794e8c
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h
> @@ -0,0 +1,32 @@
> +/* HPPA implementation of the semaphore struct __semid_ds32

Missing period.

> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#if __WORDSIZE == 32
> +/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
> + * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */

I think on multi-line comments the lines should not have an extra '*'.

> +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;
> +};

Wrong identation.

> +#endif
> diff --git a/sysdeps/unix/sysv/linux/mips/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/mips/struct__semid_ds32.h
> new file mode 100644
> index 0000000000..d3eb47611c
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/mips/struct__semid_ds32.h
> @@ -0,0 +1,30 @@
> +/* MIPS implementation of the semaphore struct __semid_ds32

Missing period.

> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#if __WORDSIZE == 32
> +/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
> + * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
> +struct __semid_ds32 {
> +  struct ipc_perm sem_perm;              /* operation permission struct */
> +  __syscall_ulong_t   sem_otime;          /* last semop time */
> +  __syscall_ulong_t   sem_ctime;          /* last change time */
> +  __syscall_ulong_t   sem_nsems;         /* number of semaphores in set */
> +  __syscall_ulong_t   sem_otime_high;
> +  __syscall_ulong_t   sem_ctime_high;
> +};
> +#endif

Same comments from hppa variant.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h
> new file mode 100644
> index 0000000000..7ba6b852f1
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h
> @@ -0,0 +1,32 @@
> +/* PowerPC implementation of the semaphore struct __semid_ds32

Missing period.

> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#if __WORDSIZE == 32
> +/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
> + * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
> +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;
> +};
> +#endif

Same comments from hppa variant.

> diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
> index 30571af49f..b0e4d828ed 100644
> --- a/sysdeps/unix/sysv/linux/semctl.c
> +++ b/sysdeps/unix/sysv/linux/semctl.c
> @@ -22,6 +22,7 @@
>  #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.  */
> @@ -29,6 +30,9 @@ union semun
>  {
>    int val;			/* value for SETVAL */
>    struct semid_ds *buf;		/* buffer for IPC_STAT & IPC_SET */
> +#if __WORDSIZE == 32
> +  struct __semid_ds32 *buf32;   /* 32-bit buffer for IPC_STAT */
> +#endif
>    unsigned short int *array;	/* array for GETALL & SETALL */
>    struct seminfo *__buf;	/* buffer for IPC_INFO */
>  };
> @@ -44,13 +48,31 @@ 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
> +
> +#if (__WORDSIZE == 32 && __TIMESIZE == 64 \
> +     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
> +  if (ret == 0 && (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY))
> +    {
> +      struct semid_ds temp_ds = (struct semid_ds) {
> +        .sem_perm = arg.buf32->sem_perm,
> +        .sem_nsems = arg.buf32->sem_nsems,
> +        .sem_otime = (arg.buf32->sem_otime
> +                      | ((time_t) arg.buf32->sem_otime_high << 32)),
> +        .sem_ctime = (arg.buf32->sem_ctime
> +                      | ((time_t) arg.buf32->sem_ctime_high << 32)),
> +      };
> +      *arg.buf = temp_ds;
> +    }
> +#endif
> +  return ret;
>  }
>  
>  int
> diff --git a/sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h
> new file mode 100644
> index 0000000000..c39d65d710
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h
> @@ -0,0 +1,32 @@
> +/* Sparc implementation of the semaphore struct __semid_ds32

Missing period.

> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#if __WORDSIZE == 32
> +/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
> + * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
> +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;
> +};
> +#endif

Same comments from hppa variant.

> diff --git a/sysdeps/unix/sysv/linux/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/struct__semid_ds32.h
> new file mode 100644
> index 0000000000..c82bf01bf3
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/struct__semid_ds32.h
> @@ -0,0 +1,32 @@
> +/* Generic implementation of the semaphore struct __semid_ds32

Missing period.

> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#if __WORDSIZE == 32
> +/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
> + * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
> +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

Same comments from hppa variant.

> diff --git a/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
> new file mode 100644
> index 0000000000..4e4ab26661
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
> @@ -0,0 +1,32 @@
> +/* Sparc implementation of the semaphore struct __semid_ds32

s/Sparc/x86 and missing period.

> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifdef __i386__
> +/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
> + * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
> +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
> 


Same comments from hppa variant.

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-04-22 21:25     ` Adhemerval Zanella
@ 2020-04-28 21:59       ` Alistair Francis
  0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2020-04-28 21:59 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library

On Wed, Apr 22, 2020 at 2:26 PM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 21/04/2020 17:52, Stepan Golosunov wrote:
> > 01.04.2020 в 09:53:08 -0700 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 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.
> >>
> >> Due to allignment differences between 64-bit and 32-bit variables we
> >> also need to set nsems to ensure it's correct.
> >> ---
> >>  .../unix/sysv/linux/hppa/struct__semid_ds32.h | 32 +++++++++++++++++++
> >>  .../unix/sysv/linux/mips/struct__semid_ds32.h | 30 +++++++++++++++++
> >>  .../sysv/linux/powerpc/struct__semid_ds32.h   | 32 +++++++++++++++++++
> >>  sysdeps/unix/sysv/linux/semctl.c              | 30 ++++++++++++++---
> >>  .../sysv/linux/sparc/struct__semid_ds32.h     | 32 +++++++++++++++++++
> >>  sysdeps/unix/sysv/linux/struct__semid_ds32.h  | 32 +++++++++++++++++++
> >>  .../unix/sysv/linux/x86/struct__semid_ds32.h  | 32 +++++++++++++++++++
> >>  7 files changed, 216 insertions(+), 4 deletions(-)
> >>  create mode 100644 sysdeps/unix/sysv/linux/hppa/struct__semid_ds32.h
> >>  create mode 100644 sysdeps/unix/sysv/linux/mips/struct__semid_ds32.h
> >>  create mode 100644 sysdeps/unix/sysv/linux/powerpc/struct__semid_ds32.h
> >>  create mode 100644 sysdeps/unix/sysv/linux/sparc/struct__semid_ds32.h
> >>  create mode 100644 sysdeps/unix/sysv/linux/struct__semid_ds32.h
> >>  create mode 100644 sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
> >
> >> diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
> >> index 30571af49f..b0e4d828ed 100644
> >> --- a/sysdeps/unix/sysv/linux/semctl.c
> >> +++ b/sysdeps/unix/sysv/linux/semctl.c
> >> @@ -22,6 +22,7 @@
> >>  #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.  */
> >> @@ -29,6 +30,9 @@ union semun
> >>  {
> >>    int val;                  /* value for SETVAL */
> >>    struct semid_ds *buf;             /* buffer for IPC_STAT & IPC_SET */
> >> +#if __WORDSIZE == 32
> >> +  struct __semid_ds32 *buf32;   /* 32-bit buffer for IPC_STAT */
> >> +#endif
> >
> > This won't compile on x32.
>
> In fact it will, since the incomplete type won't be actually used on x32
> and c99 struct initialization does not catch this (at least this patch
> set does build with gcc 9.3 for x32).  But I agree that it not correct
> to actually define it for x32.
>
> >
> >>    unsigned short int *array;        /* array for GETALL & SETALL */
> >>    struct seminfo *__buf;    /* buffer for IPC_INFO */
> >>  };
> >> @@ -44,13 +48,31 @@ 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
> >> +
> >> +#if (__WORDSIZE == 32 && __TIMESIZE == 64 \
> >> +     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
> >> +  if (ret == 0 && (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY))
> >> +    {
> >> +      struct semid_ds temp_ds = (struct semid_ds) {
> >> +        .sem_perm = arg.buf32->sem_perm,
> >> +        .sem_nsems = arg.buf32->sem_nsems,
> >> +        .sem_otime = (arg.buf32->sem_otime
> >> +                      | ((time_t) arg.buf32->sem_otime_high << 32)),
> >> +        .sem_ctime = (arg.buf32->sem_ctime
> >> +                      | ((time_t) arg.buf32->sem_ctime_high << 32)),
> >> +      };
> >> +      *arg.buf = temp_ds;
> >> +    }
> >> +#endif
> >> +  return ret;
> >>  }
> >
> > You do not need 2 temporary variables (temp_ds and an anonymous one).
> >
> >       ….buf = (struct semid_ds) { … };
> > or
> >       struct semid_ds temp_ds = { … };
> >
> >
> > And while I am not too familiar with aliasing rules this still seems
> > to violate them (accessing the same memory via 2 pointers of
> > incompatible types).  Making this an undefined behavior.
>
> It uses a temporary buffer exactly to avoid it (similar to mips64
> getdents64).
>
> However, the way previous the patch redefined semid_ds for
> __TIMESIZE == 64 does not take in consideration the 64-bit __time_t
> alignment (it might be case where sem_perm force padding on next
> element). It makes the glibc exported semid_ds incompatible with
> kernel one, which is not an issues but prevent glibc to pass it
> directly to kernel in the syscall.
>
> And there is another issue where prior v4.14 (commit
> 4693916846269d633a3664586650dbfac2c5562f) Linux left the reserved
> space (used later to the high bits of 64-bit times) untouched rather
> than zeroed.  This means that the caller must zero the kernel passed
> buffer.
>
> So both issues prevents to use the user provided buffer directly in
> the syscall.  So what I think it would more straightforward to use a
> temporary to issue the syscall.
>
> Something like:
>
>   #define IPC_TIME64 \
>    (__WORDSIZE == 32 && __TIMESIZE == 64 \
>        && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
>
>   static int
>   semctl_syscall (int semid, int semnum, int cmd, union semun arg)
>   {
>     int ret;
>   #if IPC_TIME64
>     struct __semid_ds32 tmp, *orig;
>     if (cmd == IPC_SET)
>       {
>         tmp = (struct __semid_ds32) {
>           .sem_perm  = arg.buf->sem_perm,
>           .sem_otime = arg.buf->sem_otime,
>           .sem_ctime = arg.buf->sem_ctime,
>           .sem_nsems = arg.buf->sem_nsems,
>         };
>         orig = arg.buf;
>         arg.buf = &tmp;
>       }
>   #endif
>
>   #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
>     ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
>                                arg.array);
>   #else
>     ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64,
>                                SEMCTL_ARG_ADDRESS (arg));
>   #endif
>
>   #if IPC_TIME64
>     if (r >= 0
>         && (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY))
>       {
>         arg.buf = orig;
>         arg.buf->sem_perm  = tmp.sem_perm;
>         arg.buf->sem_otime = tmp.sem_otime
>                              | ((time_t) tmp.sem_otime_high << 32);
>         arg.buf->sem_ctime = tmp.sem_ctime
>                              | ((time_t) tmp.sem_ctime_high << 32);
>         arg.buf->sem_nsems = tmp.sem_nsems;
>       }
>   #endif
>
> (the first part where the user provided buffer might require some adjustments
> once __TIMESIZE == 64 is being exported in public ABI).

With a few changes I used what you posted above. Sending a new version now.

Thanks for the help.

Alistair

>
> >
> > What I was suggesting is something like this:
> >
> > union semun
> > {
> >   int val;
> >   union { struct semid_ds s, struct __semid_ds32 s32 } *buf;
> >   unsigned short int *array;
> >   struct seminfo *__buf;
> > };
> >
> > Or even with an anonymous union member:
> >
> > union __semid_ds {
> >   struct semid_ds;
> > #if …
> >   struct __semid_ds32 buf32;
> > #endif
> > }
> > union semun
> > {
> >   int val;
> >   union __semid_ds *buf;
> >   unsigned short int *array;
> >   struct seminfo *__buf;
> > };
> >
> > This should allow to access both structures simultaneously without
> > local violations of aliasing rules.  (I think that aliasing violations
> > between different compilation units are normal in glibc.  While inside
> > a single file they can lead to miscompilation.)
> >
> >
> >> diff --git a/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h b/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
> >> new file mode 100644
> >> index 0000000000..4e4ab26661
> >> --- /dev/null
> >> +++ b/sysdeps/unix/sysv/linux/x86/struct__semid_ds32.h
> >> @@ -0,0 +1,32 @@
> >> +/* Sparc implementation of the semaphore struct __semid_ds32
> >> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> >> +   This file is part of the GNU C Library.
> >> +
> >> +   The GNU C Library is free software; you can redistribute it and/or
> >> +   modify it under the terms of the GNU Lesser General Public
> >> +   License as published by the Free Software Foundation; either
> >> +   version 2.1 of the License, or (at your option) any later version.
> >> +
> >> +   The GNU C Library is distributed in the hope that it will be useful,
> >> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >> +   Lesser General Public License for more details.
> >> +
> >> +   You should have received a copy of the GNU Lesser General Public
> >> +   License along with the GNU C Library; if not, see
> >> +   <https://www.gnu.org/licenses/>.  */
> >> +
> >> +#ifdef __i386__
> >> +/* This is the "new" y2038 types defined after the 5.1 kernel. It allows
> >> + * the kernel to use {o,c}time{_high} values to support a 64-bit time_t.  */
> >> +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

^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-04-28 22:08 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-01 16:53 [PATCH v5 0/3] Support y2038 semctl_syscall() Alistair Francis
2020-04-01 16:53 ` [PATCH v5 1/3] bits/sem.h: Split out struct semid_ds Alistair Francis
2020-04-17 18:06   ` Adhemerval Zanella
2020-04-01 16:53 ` [PATCH v5 2/3] semctl: Remove the sem-pad.h file Alistair Francis
2020-04-17 19:03   ` Adhemerval Zanella
2020-04-17 20:38     ` Alistair Francis
2020-04-20 20:18       ` Adhemerval Zanella
2020-04-20 20:25         ` Alistair Francis
2020-04-21 14:03           ` Adhemerval Zanella
2020-04-22 19:22   ` Adhemerval Zanella
2020-04-01 16:53 ` [PATCH v5 3/3] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
2020-04-21 20:52   ` Stepan Golosunov
2020-04-22 21:25     ` Adhemerval Zanella
2020-04-28 21:59       ` Alistair Francis
2020-04-22 21:25   ` Adhemerval Zanella
2020-04-16 16:23 ` [PATCH v5 0/3] Support y2038 semctl_syscall() 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).