public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Support y2038 semctl_syscall()
@ 2020-03-26 16:37 Alistair Francis
  2020-03-26 16:37 ` [PATCH v4 1/3] bits/sem.h: Split out struct semid_ds Alistair Francis
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Alistair Francis @ 2020-03-26 16:37 UTC (permalink / raw)
  To: libc-alpha; +Cc: Vineet.Gupta1, 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.

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 +-------
 sysdeps/unix/sysv/linux/bits/semid_ds_t.h     | 61 +++++++++++++++++++
 sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h   | 26 --------
 .../unix/sysv/linux/hppa/bits/semid_ds_t.h    | 61 +++++++++++++++++++
 sysdeps/unix/sysv/linux/mips/bits/sem-pad.h   | 24 --------
 .../unix/sysv/linux/mips/bits/semid_ds_t.h    | 45 ++++++++++++++
 .../unix/sysv/linux/powerpc/bits/sem-pad.h    | 26 --------
 .../unix/sysv/linux/powerpc/bits/semid_ds_t.h | 61 +++++++++++++++++++
 sysdeps/unix/sysv/linux/semctl.c              | 24 ++++++--
 sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h  | 26 --------
 .../unix/sysv/linux/sparc/bits/semid_ds_t.h   | 61 +++++++++++++++++++
 sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    | 24 --------
 sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h | 49 +++++++++++++++
 15 files changed, 362 insertions(+), 188 deletions(-)
 delete mode 100644 sysdeps/unix/sysv/linux/bits/sem-pad.h
 create mode 100644 sysdeps/unix/sysv/linux/bits/semid_ds_t.h
 delete mode 100644 sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
 create mode 100644 sysdeps/unix/sysv/linux/hppa/bits/semid_ds_t.h
 delete mode 100644 sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
 create mode 100644 sysdeps/unix/sysv/linux/mips/bits/semid_ds_t.h
 delete mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/semid_ds_t.h
 delete mode 100644 sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
 create mode 100644 sysdeps/unix/sysv/linux/sparc/bits/semid_ds_t.h
 delete mode 100644 sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
 create mode 100644 sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h

-- 
2.26.0


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

* [PATCH v4 1/3] bits/sem.h: Split out struct semid_ds
  2020-03-26 16:37 [PATCH v4 0/3] Support y2038 semctl_syscall() Alistair Francis
@ 2020-03-26 16:37 ` Alistair Francis
  2020-03-26 18:59   ` Joseph Myers
  2020-03-26 16:37 ` [PATCH v4 2/3] semctl: Remove the sem-pad.h file Alistair Francis
  2020-03-26 16:37 ` [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
  2 siblings, 1 reply; 13+ messages in thread
From: Alistair Francis @ 2020-03-26 16:37 UTC (permalink / raw)
  To: libc-alpha; +Cc: Vineet.Gupta1, 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 +------------
 sysdeps/unix/sysv/linux/bits/semid_ds_t.h | 43 +++++++++++++++++++++++
 3 files changed, 45 insertions(+), 23 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/bits/semid_ds_t.h

diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 60dc5cf9e5..ddb2a54e57 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/semid_ds_t.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..5c2f6be4c3 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/semid_ds_t.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/semid_ds_t.h b/sysdeps/unix/sysv/linux/bits/semid_ds_t.h
new file mode 100644
index 0000000000..baed55ca8b
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/bits/semid_ds_t.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/semid_ds_t.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] 13+ messages in thread

* [PATCH v4 2/3] semctl: Remove the sem-pad.h file
  2020-03-26 16:37 [PATCH v4 0/3] Support y2038 semctl_syscall() Alistair Francis
  2020-03-26 16:37 ` [PATCH v4 1/3] bits/sem.h: Split out struct semid_ds Alistair Francis
@ 2020-03-26 16:37 ` Alistair Francis
  2020-03-26 16:37 ` [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
  2 siblings, 0 replies; 13+ messages in thread
From: Alistair Francis @ 2020-03-26 16:37 UTC (permalink / raw)
  To: libc-alpha; +Cc: Vineet.Gupta1, alistair23, Alistair Francis

Remove the sem-pad.h file and instead have architectures override the
struct semid_ds via the semid_ds_t.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 +-
 sysdeps/unix/sysv/linux/bits/semid_ds_t.h     | 29 ++++++------
 .../unix/sysv/linux/hppa/bits/semid_ds_t.h    | 46 +++++++++++++++++++
 sysdeps/unix/sysv/linux/mips/bits/sem-pad.h   | 24 ----------
 .../bits/sem-pad.h => mips/bits/semid_ds_t.h} | 20 +++++---
 .../unix/sysv/linux/powerpc/bits/sem-pad.h    | 26 -----------
 .../unix/sysv/linux/powerpc/bits/semid_ds_t.h | 46 +++++++++++++++++++
 .../unix/sysv/linux/sparc/bits/semid_ds_t.h   | 46 +++++++++++++++++++
 sysdeps/unix/sysv/linux/x86/bits/sem-pad.h    | 24 ----------
 .../bits/sem-pad.h => x86/bits/semid_ds_t.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/semid_ds_t.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/semid_ds_t.h} (56%)
 delete mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/semid_ds_t.h
 create mode 100644 sysdeps/unix/sysv/linux/sparc/bits/semid_ds_t.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/semid_ds_t.h} (53%)

diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index ddb2a54e57..af6dc61886 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 5c2f6be4c3..771ce00327 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/semid_ds_t.h>
 
 /* Flags for `semop'.  */
diff --git a/sysdeps/unix/sysv/linux/bits/semid_ds_t.h b/sysdeps/unix/sysv/linux/bits/semid_ds_t.h
index baed55ca8b..d9d902ed0d 100644
--- a/sysdeps/unix/sysv/linux/bits/semid_ds_t.h
+++ b/sysdeps/unix/sysv/linux/bits/semid_ds_t.h
@@ -20,24 +20,27 @@
 # error "Never include <bits/semid_ds_t.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/semid_ds_t.h b/sysdeps/unix/sysv/linux/hppa/bits/semid_ds_t.h
new file mode 100644
index 0000000000..39c0e53f38
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/hppa/bits/semid_ds_t.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/semid_ds_t.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/semid_ds_t.h
similarity index 56%
rename from sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
rename to sysdeps/unix/sysv/linux/mips/bits/semid_ds_t.h
index 5f4e214d12..1ab16492dd 100644
--- a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
+++ b/sysdeps/unix/sysv/linux/mips/bits/semid_ds_t.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/semid_ds_t.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/semid_ds_t.h b/sysdeps/unix/sysv/linux/powerpc/bits/semid_ds_t.h
new file mode 100644
index 0000000000..79b4cba939
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/bits/semid_ds_t.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/semid_ds_t.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/semid_ds_t.h b/sysdeps/unix/sysv/linux/sparc/bits/semid_ds_t.h
new file mode 100644
index 0000000000..f8de676e79
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/sparc/bits/semid_ds_t.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/semid_ds_t.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/semid_ds_t.h
similarity index 53%
rename from sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
rename to sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
index ee0332325b..42694069d5 100644
--- a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
+++ b/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.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/semid_ds_t.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] 13+ messages in thread

* [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-03-26 16:37 [PATCH v4 0/3] Support y2038 semctl_syscall() Alistair Francis
  2020-03-26 16:37 ` [PATCH v4 1/3] bits/sem.h: Split out struct semid_ds Alistair Francis
  2020-03-26 16:37 ` [PATCH v4 2/3] semctl: Remove the sem-pad.h file Alistair Francis
@ 2020-03-26 16:37 ` Alistair Francis
  2020-03-26 19:01   ` Joseph Myers
  2020-03-27 18:24   ` Stepan Golosunov
  2 siblings, 2 replies; 13+ messages in thread
From: Alistair Francis @ 2020-03-26 16:37 UTC (permalink / raw)
  To: libc-alpha; +Cc: Vineet.Gupta1, 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.
---
 sysdeps/unix/sysv/linux/bits/semid_ds_t.h     | 15 ++++++++++++
 .../unix/sysv/linux/hppa/bits/semid_ds_t.h    | 15 ++++++++++++
 .../unix/sysv/linux/mips/bits/semid_ds_t.h    | 13 ++++++++++
 .../unix/sysv/linux/powerpc/bits/semid_ds_t.h | 15 ++++++++++++
 sysdeps/unix/sysv/linux/semctl.c              | 24 +++++++++++++++----
 .../unix/sysv/linux/sparc/bits/semid_ds_t.h   | 15 ++++++++++++
 sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h | 15 ++++++++++++
 7 files changed, 108 insertions(+), 4 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/bits/semid_ds_t.h b/sysdeps/unix/sysv/linux/bits/semid_ds_t.h
index d9d902ed0d..b135301356 100644
--- a/sysdeps/unix/sysv/linux/bits/semid_ds_t.h
+++ b/sysdeps/unix/sysv/linux/bits/semid_ds_t.h
@@ -20,6 +20,21 @@
 # error "Never include <bits/semid_ds_t.h> directly; use <sys/sem.h> instead."
 #endif
 
+#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
+
 /* Data structure describing a set of semaphores.  */
 #if __TIMESIZE == 32
 struct semid_ds
diff --git a/sysdeps/unix/sysv/linux/hppa/bits/semid_ds_t.h b/sysdeps/unix/sysv/linux/hppa/bits/semid_ds_t.h
index 39c0e53f38..3613c5ec94 100644
--- a/sysdeps/unix/sysv/linux/hppa/bits/semid_ds_t.h
+++ b/sysdeps/unix/sysv/linux/hppa/bits/semid_ds_t.h
@@ -20,6 +20,21 @@
 # error "Never include <bits/semid_ds_t.h> directly; use <sys/sem.h> instead."
 #endif
 
+#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
+
 /* Data structure describing a set of semaphores.  */
 #if __TIMESIZE == 32
 struct semid_ds
diff --git a/sysdeps/unix/sysv/linux/mips/bits/semid_ds_t.h b/sysdeps/unix/sysv/linux/mips/bits/semid_ds_t.h
index 1ab16492dd..e26906a67f 100644
--- a/sysdeps/unix/sysv/linux/mips/bits/semid_ds_t.h
+++ b/sysdeps/unix/sysv/linux/mips/bits/semid_ds_t.h
@@ -20,6 +20,19 @@
 # error "Never include <bits/semid_ds_t.h> directly; use <sys/sem.h> instead."
 #endif
 
+#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
+
 /* Data structure describing a set of semaphores.  */
 struct semid_ds
 {
diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/semid_ds_t.h b/sysdeps/unix/sysv/linux/powerpc/bits/semid_ds_t.h
index 79b4cba939..ec2ff552eb 100644
--- a/sysdeps/unix/sysv/linux/powerpc/bits/semid_ds_t.h
+++ b/sysdeps/unix/sysv/linux/powerpc/bits/semid_ds_t.h
@@ -20,6 +20,21 @@
 # error "Never include <bits/semid_ds_t.h> directly; use <sys/sem.h> instead."
 #endif
 
+#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
+
 /* Data structure describing a set of semaphores.  */
 #if __TIMESIZE == 32
 struct semid_ds
diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
index 30571af49f..38c2fc6545 100644
--- a/sysdeps/unix/sysv/linux/semctl.c
+++ b/sysdeps/unix/sysv/linux/semctl.c
@@ -29,6 +29,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 +47,26 @@ 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
+  if (ret == 0 && cmd == IPC_STAT)
+    {
+      arg.buf->sem_nsems = arg.buf32->sem_nsems;
+      arg.buf->sem_otime = arg.buf32->sem_otime |
+                               ((time_t) arg.buf32->sem_otime_high << 32);
+      arg.buf->sem_ctime = arg.buf32->sem_ctime |
+                               ((time_t) arg.buf32->sem_ctime_high << 32);
+    }
+#endif
+  return ret;
 }
 
 int
diff --git a/sysdeps/unix/sysv/linux/sparc/bits/semid_ds_t.h b/sysdeps/unix/sysv/linux/sparc/bits/semid_ds_t.h
index f8de676e79..b08fb8a79e 100644
--- a/sysdeps/unix/sysv/linux/sparc/bits/semid_ds_t.h
+++ b/sysdeps/unix/sysv/linux/sparc/bits/semid_ds_t.h
@@ -20,6 +20,21 @@
 # error "Never include <bits/semid_ds_t.h> directly; use <sys/sem.h> instead."
 #endif
 
+#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
+
 /* Data structure describing a set of semaphores.  */
 #if __TIMESIZE == 32
 struct semid_ds
diff --git a/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h b/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
index 42694069d5..c7b9adce88 100644
--- a/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
+++ b/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
@@ -20,6 +20,21 @@
 # error "Never include <bits/semid_ds_t.h> directly; use <sys/sem.h> instead."
 #endif
 
+#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
+
 /* Data structure describing a set of semaphores.  */
 struct semid_ds
 {
-- 
2.26.0


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

* Re: [PATCH v4 1/3] bits/sem.h: Split out struct semid_ds
  2020-03-26 16:37 ` [PATCH v4 1/3] bits/sem.h: Split out struct semid_ds Alistair Francis
@ 2020-03-26 18:59   ` Joseph Myers
  2020-03-26 21:58     ` Alistair Francis
  0 siblings, 1 reply; 13+ messages in thread
From: Joseph Myers @ 2020-03-26 18:59 UTC (permalink / raw)
  To: Alistair Francis; +Cc: libc-alpha, Vineet.Gupta1

On Thu, 26 Mar 2020, 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.
> ---
>  sysdeps/unix/sysv/linux/Makefile          |  1 +
>  sysdeps/unix/sysv/linux/bits/sem.h        | 24 +------------
>  sysdeps/unix/sysv/linux/bits/semid_ds_t.h | 43 +++++++++++++++++++++++

That's not the naming convention.  A header that just defines a given type 
for use in other installed headers is bytes/types/<type_name>.h.  So 
bits/types/struct_semid_ds.h (there's no _t on this type name).

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-03-26 16:37 ` [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
@ 2020-03-26 19:01   ` Joseph Myers
  2020-03-26 22:11     ` Alistair Francis
  2020-03-27 18:24   ` Stepan Golosunov
  1 sibling, 1 reply; 13+ messages in thread
From: Joseph Myers @ 2020-03-26 19:01 UTC (permalink / raw)
  To: Alistair Francis; +Cc: libc-alpha, Vineet.Gupta1

On Thu, 26 Mar 2020, Alistair Francis via Libc-alpha wrote:

> +#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

As far as I can see, this type is not used in any public interface, just 
internally in the implementation.  Thus it should be defined in an 
internal header, not a bits/ installed header.

> +      arg.buf->sem_nsems = arg.buf32->sem_nsems;
> +      arg.buf->sem_otime = arg.buf32->sem_otime |
> +                               ((time_t) arg.buf32->sem_otime_high << 32);
> +      arg.buf->sem_ctime = arg.buf32->sem_ctime |
> +                               ((time_t) arg.buf32->sem_ctime_high << 32);

Split lines before not after operators, and make sure to include 
parentheses in such a case to ensure the operator at the start of the next 
line automatically goes in the correct column (see the GNU Coding 
Standards).

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH v4 1/3] bits/sem.h: Split out struct semid_ds
  2020-03-26 18:59   ` Joseph Myers
@ 2020-03-26 21:58     ` Alistair Francis
  0 siblings, 0 replies; 13+ messages in thread
From: Alistair Francis @ 2020-03-26 21:58 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Alistair Francis, Vineet Gupta, GNU C Library

On Thu, Mar 26, 2020 at 11:59 AM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 26 Mar 2020, 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.
> > ---
> >  sysdeps/unix/sysv/linux/Makefile          |  1 +
> >  sysdeps/unix/sysv/linux/bits/sem.h        | 24 +------------
> >  sysdeps/unix/sysv/linux/bits/semid_ds_t.h | 43 +++++++++++++++++++++++
>
> That's not the naming convention.  A header that just defines a given type
> for use in other installed headers is bytes/types/<type_name>.h.  So
> bits/types/struct_semid_ds.h (there's no _t on this type name).

Fixed!

Alistair

>
> --
> Joseph S. Myers
> joseph@codesourcery.com

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

* Re: [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-03-26 19:01   ` Joseph Myers
@ 2020-03-26 22:11     ` Alistair Francis
  2020-03-26 22:52       ` Joseph Myers
  0 siblings, 1 reply; 13+ messages in thread
From: Alistair Francis @ 2020-03-26 22:11 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Alistair Francis, Vineet Gupta, GNU C Library

On Thu, Mar 26, 2020 at 12:01 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 26 Mar 2020, Alistair Francis via Libc-alpha wrote:
>
> > +#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
>
> As far as I can see, this type is not used in any public interface, just
> internally in the implementation.  Thus it should be defined in an
> internal header, not a bits/ installed header.

It is only user internally.

The problem is I couldn't find a place to put it where architectures
can override it. Is there a place I'm missing where I could put it?

>
> > +      arg.buf->sem_nsems = arg.buf32->sem_nsems;
> > +      arg.buf->sem_otime = arg.buf32->sem_otime |
> > +                               ((time_t) arg.buf32->sem_otime_high << 32);
> > +      arg.buf->sem_ctime = arg.buf32->sem_ctime |
> > +                               ((time_t) arg.buf32->sem_ctime_high << 32);
>
> Split lines before not after operators, and make sure to include
> parentheses in such a case to ensure the operator at the start of the next
> line automatically goes in the correct column (see the GNU Coding
> Standards).

Fixed.

Alistair

>
> --
> Joseph S. Myers
> joseph@codesourcery.com

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

* Re: [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-03-26 22:11     ` Alistair Francis
@ 2020-03-26 22:52       ` Joseph Myers
  0 siblings, 0 replies; 13+ messages in thread
From: Joseph Myers @ 2020-03-26 22:52 UTC (permalink / raw)
  To: Alistair Francis; +Cc: GNU C Library, Vineet Gupta, Alistair Francis

On Thu, 26 Mar 2020, Alistair Francis via Libc-alpha wrote:

> The problem is I couldn't find a place to put it where architectures
> can override it. Is there a place I'm missing where I could put it?

Any .h or .c file in the glibc sources can be overridden for a particular 
configuration simply by putting a copy in a higher-priority sysdeps 
directory.  (But watch out for code using #include with "" instead of <>; 
if a .h or .c file is included like that, sysdeps overrides of the 
included file may not be effective.  In general, use <> not "" with 
#include in glibc to avoid that issue.)

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-03-26 16:37 ` [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
  2020-03-26 19:01   ` Joseph Myers
@ 2020-03-27 18:24   ` Stepan Golosunov
  2020-03-27 19:45     ` Alistair Francis
  1 sibling, 1 reply; 13+ messages in thread
From: Stepan Golosunov @ 2020-03-27 18:24 UTC (permalink / raw)
  To: Alistair Francis; +Cc: libc-alpha

26.03.2020 в 09:37:24 -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.
> ---
>  sysdeps/unix/sysv/linux/bits/semid_ds_t.h     | 15 ++++++++++++
>  .../unix/sysv/linux/hppa/bits/semid_ds_t.h    | 15 ++++++++++++
>  .../unix/sysv/linux/mips/bits/semid_ds_t.h    | 13 ++++++++++
>  .../unix/sysv/linux/powerpc/bits/semid_ds_t.h | 15 ++++++++++++
>  sysdeps/unix/sysv/linux/semctl.c              | 24 +++++++++++++++----
>  .../unix/sysv/linux/sparc/bits/semid_ds_t.h   | 15 ++++++++++++
>  sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h | 15 ++++++++++++
>  7 files changed, 108 insertions(+), 4 deletions(-)
> 

> --- a/sysdeps/unix/sysv/linux/semctl.c
> +++ b/sysdeps/unix/sysv/linux/semctl.c
> @@ -29,6 +29,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 +47,26 @@ 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

This condition is wrong for x32 unless someone invents __semid_ds32
with 32-bit time fields there.

> +  if (ret == 0 && cmd == IPC_STAT)

Is cmd == IPC_STAT the only case when conversion is needed?
Brief glance at kernel sources suggests that SEM_STAT and SEM_STAT_ANY
are also relevant.

> +    {
> +      arg.buf->sem_nsems = arg.buf32->sem_nsems;
> +      arg.buf->sem_otime = arg.buf32->sem_otime |
> +                               ((time_t) arg.buf32->sem_otime_high << 32);
> +      arg.buf->sem_ctime = arg.buf32->sem_ctime |
> +                               ((time_t) arg.buf32->sem_ctime_high << 32);
> +    }

Note that this is supposed to do nothing on little-endian asm-generic
architectures (thus hiding bugs for big-endian ones).
It's also not obvious why it attempts to copy sem_nsems (if sem_nsems
ever changes it's location the assignment will likely clobber some of the
sem{o,c}time{,_high}).

> +#endif
> +  return ret;
>  }
>  
>  int

> --- a/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
> +++ b/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
> @@ -20,6 +20,21 @@
>  # error "Never include <bits/semid_ds_t.h> directly; use <sys/sem.h> instead."
>  #endif
>  
> +#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

This is wrong for x32 but might work by accident (as long as kernel
does not use its __unused1 and __unused2 fields).

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

* Re: [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-03-27 18:24   ` Stepan Golosunov
@ 2020-03-27 19:45     ` Alistair Francis
  2020-03-28 10:28       ` Stepan Golosunov
  0 siblings, 1 reply; 13+ messages in thread
From: Alistair Francis @ 2020-03-27 19:45 UTC (permalink / raw)
  To: Stepan Golosunov; +Cc: Alistair Francis, GNU C Library

On Fri, Mar 27, 2020 at 11:25 AM Stepan Golosunov
<stepan@golosunov.pp.ru> wrote:
>
> 26.03.2020 в 09:37:24 -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.
> > ---
> >  sysdeps/unix/sysv/linux/bits/semid_ds_t.h     | 15 ++++++++++++
> >  .../unix/sysv/linux/hppa/bits/semid_ds_t.h    | 15 ++++++++++++
> >  .../unix/sysv/linux/mips/bits/semid_ds_t.h    | 13 ++++++++++
> >  .../unix/sysv/linux/powerpc/bits/semid_ds_t.h | 15 ++++++++++++
> >  sysdeps/unix/sysv/linux/semctl.c              | 24 +++++++++++++++----
> >  .../unix/sysv/linux/sparc/bits/semid_ds_t.h   | 15 ++++++++++++
> >  sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h | 15 ++++++++++++
> >  7 files changed, 108 insertions(+), 4 deletions(-)
> >
>
> > --- a/sysdeps/unix/sysv/linux/semctl.c
> > +++ b/sysdeps/unix/sysv/linux/semctl.c
> > @@ -29,6 +29,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 +47,26 @@ 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
>
> This condition is wrong for x32 unless someone invents __semid_ds32
> with 32-bit time fields there.

Ah, good point. I guess there needs to be a && !defined(__ILP32__)
added to this.

>
> > +  if (ret == 0 && cmd == IPC_STAT)
>
> Is cmd == IPC_STAT the only case when conversion is needed?
> Brief glance at kernel sources suggests that SEM_STAT and SEM_STAT_ANY
> are also relevant.

I can double check.

>
> > +    {
> > +      arg.buf->sem_nsems = arg.buf32->sem_nsems;
> > +      arg.buf->sem_otime = arg.buf32->sem_otime |
> > +                               ((time_t) arg.buf32->sem_otime_high << 32);
> > +      arg.buf->sem_ctime = arg.buf32->sem_ctime |
> > +                               ((time_t) arg.buf32->sem_ctime_high << 32);
> > +    }
>
> Note that this is supposed to do nothing on little-endian asm-generic
> architectures (thus hiding bugs for big-endian ones).
> It's also not obvious why it attempts to copy sem_nsems (if sem_nsems
> ever changes it's location the assignment will likely clobber some of the
> sem{o,c}time{,_high}).

This is somewhat explained in the commit message.

For RV32 (and probably others) due to alignment constraints being
different between 32 and 64-bit types sem_nsems is actually at a
different offset in bug32 then buf. The kernel fills it out at the
alignment of buf32 so we need to move it to the alignment of buf.

The same issues applies to both types, so this isn't just for
big-endian machines.

>
> > +#endif
> > +  return ret;
> >  }
> >
> >  int
>
> > --- a/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
> > +++ b/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
> > @@ -20,6 +20,21 @@
> >  # error "Never include <bits/semid_ds_t.h> directly; use <sys/sem.h> instead."
> >  #endif
> >
> > +#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
>
> This is wrong for x32 but might work by accident (as long as kernel
> does not use its __unused1 and __unused2 fields).

Do you know what it should be?

I see this struct in: arch/x86/include/uapi/asm/sembuf.h

Alistair

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

* Re: [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-03-27 19:45     ` Alistair Francis
@ 2020-03-28 10:28       ` Stepan Golosunov
  2020-04-01 16:16         ` Alistair Francis
  0 siblings, 1 reply; 13+ messages in thread
From: Stepan Golosunov @ 2020-03-28 10:28 UTC (permalink / raw)
  To: Alistair Francis; +Cc: Alistair Francis, GNU C Library

27.03.2020 в 12:45:23 -0700 Alistair Francis написал:
> On Fri, Mar 27, 2020 at 11:25 AM Stepan Golosunov
> <stepan@golosunov.pp.ru> wrote:
> >
> > 26.03.2020 в 09:37:24 -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.
> > > ---
> > >  sysdeps/unix/sysv/linux/bits/semid_ds_t.h     | 15 ++++++++++++
> > >  .../unix/sysv/linux/hppa/bits/semid_ds_t.h    | 15 ++++++++++++
> > >  .../unix/sysv/linux/mips/bits/semid_ds_t.h    | 13 ++++++++++
> > >  .../unix/sysv/linux/powerpc/bits/semid_ds_t.h | 15 ++++++++++++
> > >  sysdeps/unix/sysv/linux/semctl.c              | 24 +++++++++++++++----
> > >  .../unix/sysv/linux/sparc/bits/semid_ds_t.h   | 15 ++++++++++++
> > >  sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h | 15 ++++++++++++
> > >  7 files changed, 108 insertions(+), 4 deletions(-)
> > >
> >
> > > --- a/sysdeps/unix/sysv/linux/semctl.c
> > > +++ b/sysdeps/unix/sysv/linux/semctl.c
> > > @@ -29,6 +29,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 +47,26 @@ 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
> >
> > This condition is wrong for x32 unless someone invents __semid_ds32
> > with 32-bit time fields there.
> 
> Ah, good point. I guess there needs to be a && !defined(__ILP32__)
> added to this.

The ususal condition for 32-bit architectures without x32 seems to be

#if (__WORDSIZE == 32 \
     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))


> >
> > > +  if (ret == 0 && cmd == IPC_STAT)
> >
> > Is cmd == IPC_STAT the only case when conversion is needed?
> > Brief glance at kernel sources suggests that SEM_STAT and SEM_STAT_ANY
> > are also relevant.
> 
> I can double check.
> 
> >
> > > +    {
> > > +      arg.buf->sem_nsems = arg.buf32->sem_nsems;
> > > +      arg.buf->sem_otime = arg.buf32->sem_otime |
> > > +                               ((time_t) arg.buf32->sem_otime_high << 32);
> > > +      arg.buf->sem_ctime = arg.buf32->sem_ctime |
> > > +                               ((time_t) arg.buf32->sem_ctime_high << 32);
> > > +    }
> >
> > Note that this is supposed to do nothing on little-endian asm-generic
> > architectures (thus hiding bugs for big-endian ones).
> > It's also not obvious why it attempts to copy sem_nsems (if sem_nsems
> > ever changes it's location the assignment will likely clobber some of the
> > sem{o,c}time{,_high}).
> 
> This is somewhat explained in the commit message.
> 
> For RV32 (and probably others) due to alignment constraints being
> different between 32 and 64-bit types sem_nsems is actually at a
> different offset in bug32 then buf. The kernel fills it out at the
> alignment of buf32 so we need to move it to the alignment of buf.
> 
> The same issues applies to both types, so this isn't just for
> big-endian machines.

But if there is padding before sem_otime added doesn't assignment to
arg.buf->sem_otime clobber arg.buf32->sem_ctime?

(And the situation will be even more complex when implementing y2038
support for older architectures like mips.)

This also seems to violate strict aliasing rules.

I guess it could be solved with something like

arg.buf->s = (struct semid_ds) {
  .sem_perm = arg.buf->s32.sem_perm,
  .sem_nsems = arg.buf->s32.sem_nsems,
  .sem_otime = arg.buf->s32.sem_otime | …
  …
}


> >
> > > +#endif
> > > +  return ret;
> > >  }
> > >
> > >  int
> >
> > > --- a/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
> > > +++ b/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
> > > @@ -20,6 +20,21 @@
> > >  # error "Never include <bits/semid_ds_t.h> directly; use <sys/sem.h> instead."
> > >  #endif
> > >
> > > +#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
> >
> > This is wrong for x32 but might work by accident (as long as kernel
> > does not use its __unused1 and __unused2 fields).
> 
> Do you know what it should be?
> 
> I see this struct in: arch/x86/include/uapi/asm/sembuf.h

I think the condition should be changed to

#ifdef __i386__

x32 does not need __semid_ds32.  If one was needed I guess it would
look like

struct __semid_ds32
{
  struct ipc_perm sem_perm;
  uint32_t  __glibc_reserved0;
  uint32_t sem_otime;
  int32_t sem_otime_high;
  __syscall_ulong_t __glibc_reserved1;
  uint32_t sem_ctime;
  int32_t sem_ctime_high;
  __syscall_ulong_t __glibc_reserved2;
  __syscall_ulong_t sem_nsems;
  __syscall_ulong_t __glibc_reserved3;
  __syscall_ulong_t __glibc_reserved4;
};

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

* Re: [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall
  2020-03-28 10:28       ` Stepan Golosunov
@ 2020-04-01 16:16         ` Alistair Francis
  0 siblings, 0 replies; 13+ messages in thread
From: Alistair Francis @ 2020-04-01 16:16 UTC (permalink / raw)
  To: Stepan Golosunov; +Cc: Alistair Francis, GNU C Library

On Sat, Mar 28, 2020 at 3:28 AM Stepan Golosunov <stepan@golosunov.pp.ru> wrote:
>
> 27.03.2020 в 12:45:23 -0700 Alistair Francis написал:
> > On Fri, Mar 27, 2020 at 11:25 AM Stepan Golosunov
> > <stepan@golosunov.pp.ru> wrote:
> > >
> > > 26.03.2020 в 09:37:24 -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.
> > > > ---
> > > >  sysdeps/unix/sysv/linux/bits/semid_ds_t.h     | 15 ++++++++++++
> > > >  .../unix/sysv/linux/hppa/bits/semid_ds_t.h    | 15 ++++++++++++
> > > >  .../unix/sysv/linux/mips/bits/semid_ds_t.h    | 13 ++++++++++
> > > >  .../unix/sysv/linux/powerpc/bits/semid_ds_t.h | 15 ++++++++++++
> > > >  sysdeps/unix/sysv/linux/semctl.c              | 24 +++++++++++++++----
> > > >  .../unix/sysv/linux/sparc/bits/semid_ds_t.h   | 15 ++++++++++++
> > > >  sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h | 15 ++++++++++++
> > > >  7 files changed, 108 insertions(+), 4 deletions(-)
> > > >
> > >
> > > > --- a/sysdeps/unix/sysv/linux/semctl.c
> > > > +++ b/sysdeps/unix/sysv/linux/semctl.c
> > > > @@ -29,6 +29,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 +47,26 @@ 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
> > >
> > > This condition is wrong for x32 unless someone invents __semid_ds32
> > > with 32-bit time fields there.
> >
> > Ah, good point. I guess there needs to be a && !defined(__ILP32__)
> > added to this.
>
> The ususal condition for 32-bit architectures without x32 seems to be
>
> #if (__WORDSIZE == 32 \
>      && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))

That works, I have updated this.

>
>
> > >
> > > > +  if (ret == 0 && cmd == IPC_STAT)
> > >
> > > Is cmd == IPC_STAT the only case when conversion is needed?
> > > Brief glance at kernel sources suggests that SEM_STAT and SEM_STAT_ANY
> > > are also relevant.
> >
> > I can double check.

You are right, I am checking those other two conditions.

> >
> > >
> > > > +    {
> > > > +      arg.buf->sem_nsems = arg.buf32->sem_nsems;
> > > > +      arg.buf->sem_otime = arg.buf32->sem_otime |
> > > > +                               ((time_t) arg.buf32->sem_otime_high << 32);
> > > > +      arg.buf->sem_ctime = arg.buf32->sem_ctime |
> > > > +                               ((time_t) arg.buf32->sem_ctime_high << 32);
> > > > +    }
> > >
> > > Note that this is supposed to do nothing on little-endian asm-generic
> > > architectures (thus hiding bugs for big-endian ones).
> > > It's also not obvious why it attempts to copy sem_nsems (if sem_nsems
> > > ever changes it's location the assignment will likely clobber some of the
> > > sem{o,c}time{,_high}).
> >
> > This is somewhat explained in the commit message.
> >
> > For RV32 (and probably others) due to alignment constraints being
> > different between 32 and 64-bit types sem_nsems is actually at a
> > different offset in bug32 then buf. The kernel fills it out at the
> > alignment of buf32 so we need to move it to the alignment of buf.
> >
> > The same issues applies to both types, so this isn't just for
> > big-endian machines.
>
> But if there is padding before sem_otime added doesn't assignment to
> arg.buf->sem_otime clobber arg.buf32->sem_ctime?
>
> (And the situation will be even more complex when implementing y2038
> support for older architectures like mips.)
>
> This also seems to violate strict aliasing rules.
>
> I guess it could be solved with something like
>
> arg.buf->s = (struct semid_ds) {
>   .sem_perm = arg.buf->s32.sem_perm,
>   .sem_nsems = arg.buf->s32.sem_nsems,
>   .sem_otime = arg.buf->s32.sem_otime | …
>   …
> }

Good idea, I have updated it to create a new struct.

>
>
> > >
> > > > +#endif
> > > > +  return ret;
> > > >  }
> > > >
> > > >  int
> > >
> > > > --- a/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
> > > > +++ b/sysdeps/unix/sysv/linux/x86/bits/semid_ds_t.h
> > > > @@ -20,6 +20,21 @@
> > > >  # error "Never include <bits/semid_ds_t.h> directly; use <sys/sem.h> instead."
> > > >  #endif
> > > >
> > > > +#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
> > >
> > > This is wrong for x32 but might work by accident (as long as kernel
> > > does not use its __unused1 and __unused2 fields).
> >
> > Do you know what it should be?
> >
> > I see this struct in: arch/x86/include/uapi/asm/sembuf.h
>
> I think the condition should be changed to
>
> #ifdef __i386__
>
> x32 does not need __semid_ds32.  If one was needed I guess it would
> look like

I have changed the define and I don't define a struct __semid_ds32 for x32.


Alistair

>
> struct __semid_ds32
> {
>   struct ipc_perm sem_perm;
>   uint32_t  __glibc_reserved0;
>   uint32_t sem_otime;
>   int32_t sem_otime_high;
>   __syscall_ulong_t __glibc_reserved1;
>   uint32_t sem_ctime;
>   int32_t sem_ctime_high;
>   __syscall_ulong_t __glibc_reserved2;
>   __syscall_ulong_t sem_nsems;
>   __syscall_ulong_t __glibc_reserved3;
>   __syscall_ulong_t __glibc_reserved4;
> };

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

end of thread, other threads:[~2020-04-01 16:24 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26 16:37 [PATCH v4 0/3] Support y2038 semctl_syscall() Alistair Francis
2020-03-26 16:37 ` [PATCH v4 1/3] bits/sem.h: Split out struct semid_ds Alistair Francis
2020-03-26 18:59   ` Joseph Myers
2020-03-26 21:58     ` Alistair Francis
2020-03-26 16:37 ` [PATCH v4 2/3] semctl: Remove the sem-pad.h file Alistair Francis
2020-03-26 16:37 ` [PATCH v4 3/3] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
2020-03-26 19:01   ` Joseph Myers
2020-03-26 22:11     ` Alistair Francis
2020-03-26 22:52       ` Joseph Myers
2020-03-27 18:24   ` Stepan Golosunov
2020-03-27 19:45     ` Alistair Francis
2020-03-28 10:28       ` Stepan Golosunov
2020-04-01 16:16         ` 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).