[PATCH v2 0/6] Always use long time_t for certain syscalls

[PATCH v2 0/6] Always use long time_t for certain syscalls

[Alistair Francis]

On y2038 safe 32-bit systems the Linux kernel expects itimerval
and rusage to use a 32-bit time_t, even though the other time_t's
are 64-bit.

This series converts getitimer, setitimer, getrusage and wait4 to be
both y2038 safe and always pass a long time_t. On 32 and 64-bit systems
we will pass a long time to the kernel (no matter the time_t size). This is
no change for 64-bit architectures or 32-bit architectures with a 32-bit time_t.

This follows the standard y2038 conversion so that we don't break
backwards compatibility but we expose a 64-bit version for y2038 safe
architectrures (like RV32).

Alistair Francis (6):
  sysv/linux: Rename alpha functions to be alpha specific
  time: Add a timeval with a long tv_sec and tv_usec
  time: Add a __itimerval64 struct
  linux: Use long time_t __getitimer/__setitimer
  resource: Add a __rusage64 struct
  linux: Use long time_t for wait4/getrusage

 include/sys/resource.h                        | 120 ++++++++++++++++++
 include/time.h                                |  64 ++++++++++
 .../{tv32-compat.h => alpha-tv32-compat.h}    |  16 +--
 sysdeps/unix/sysv/linux/alpha/osf_adjtime.c   |  10 +-
 sysdeps/unix/sysv/linux/alpha/osf_getitimer.c |   6 +-
 sysdeps/unix/sysv/linux/alpha/osf_getrusage.c |   4 +-
 .../unix/sysv/linux/alpha/osf_gettimeofday.c  |   4 +-
 sysdeps/unix/sysv/linux/alpha/osf_setitimer.c |  10 +-
 .../unix/sysv/linux/alpha/osf_settimeofday.c  |   4 +-
 sysdeps/unix/sysv/linux/alpha/osf_utimes.c    |   6 +-
 sysdeps/unix/sysv/linux/alpha/osf_wait4.c     |   4 +-
 sysdeps/unix/sysv/linux/getitimer.c           |  54 ++++++++
 sysdeps/unix/sysv/linux/getrusage.c           |  53 ++++++++
 sysdeps/unix/sysv/linux/setitimer.c           |  89 +++++++++++++
 sysdeps/unix/sysv/linux/tv32-compat.h         |  82 ++++++++++++
 sysdeps/unix/sysv/linux/wait4.c               |  40 +++++-
 16 files changed, 530 insertions(+), 36 deletions(-)
 rename sysdeps/unix/sysv/linux/alpha/{tv32-compat.h => alpha-tv32-compat.h} (88%)
 create mode 100644 sysdeps/unix/sysv/linux/getitimer.c
 create mode 100644 sysdeps/unix/sysv/linux/getrusage.c
 create mode 100644 sysdeps/unix/sysv/linux/setitimer.c
 create mode 100644 sysdeps/unix/sysv/linux/tv32-compat.h

-- 
2.25.0

[PATCH v2 2/6] time: Add a timeval with a long tv_sec and tv_usec

[Alistair Francis]

On y2038 safe 32-bit systems the Linux kernel expects itimerval to
use a 32-bit time_t, even though the other time_t's are 64-bit. To
address this let's add a __timeval32 struct to be used internally.
---
 include/time.h | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/include/time.h b/include/time.h
index 73f66277ac..f22abc13f4 100644
--- a/include/time.h
+++ b/include/time.h
@@ -400,6 +400,47 @@ timespec64_to_timeval64 (const struct __timespec64 ts64)
   return tv64;
 }
 
+/* A version of 'struct timeval' with `long` time_t
+   and suseconds_t.  */
+struct __timeval32
+{
+  long tv_sec;         /* Seconds.  */
+  long tv_usec;        /* Microseconds.  */
+};
+
+/* Conversion functions for converting to/from __timeval32
+.  If the seconds field of a __timeval32 would
+   overflow, they write { INT32_MAX, 999999 } to the output.  */
+static inline struct __timeval64
+valid_timeval32_to_timeval64 (const struct __timeval32 tv)
+{
+  return (struct __timeval64) { tv.tv_sec, tv.tv_usec };
+}
+
+static inline struct __timeval32
+valid_timeval64_to_timeval32 (const struct __timeval64 tv64)
+{
+  return (struct __timeval32) { tv64.tv_sec, tv64.tv_usec };
+}
+
+static inline struct timeval
+valid_timeval32_to_timeval (const struct __timeval32 tv)
+{
+  return (struct timeval) { tv.tv_sec, tv.tv_usec };
+}
+
+static inline struct timespec
+valid_timeval32_to_timespec (const struct __timeval32 tv)
+{
+  return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 };
+}
+
+static inline struct __timeval32
+valid_timespec_to_timeval32 (const struct timespec ts)
+{
+  return (struct __timeval32) { (time_t) ts.tv_sec, ts.tv_nsec / 1000 };
+}
+
 /* Check if a value is in the valid nanoseconds range. Return true if
    it is, false otherwise.  */
 static inline bool
-- 
2.25.0

Re: [PATCH v2 2/6] time: Add a timeval with a long tv_sec and tv_usec

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2390 bytes --]

On Mon, 10 Feb 2020 09:43:21 -0800
Alistair Francis <alistair.francis@wdc.com> wrote:

> On y2038 safe 32-bit systems the Linux kernel expects itimerval to
> use a 32-bit time_t, even though the other time_t's are 64-bit. To
> address this let's add a __timeval32 struct to be used internally.
> ---
>  include/time.h | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/include/time.h b/include/time.h
> index 73f66277ac..f22abc13f4 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -400,6 +400,47 @@ timespec64_to_timeval64 (const struct
> __timespec64 ts64) return tv64;
>  }
>  
> +/* A version of 'struct timeval' with `long` time_t
> +   and suseconds_t.  */
> +struct __timeval32
> +{
> +  long tv_sec;         /* Seconds.  */
> +  long tv_usec;        /* Microseconds.  */
> +};
> +
> +/* Conversion functions for converting to/from __timeval32
> +.  If the seconds field of a __timeval32 would
> +   overflow, they write { INT32_MAX, 999999 } to the output.  */

Please update the above comment.

> +static inline struct __timeval64
> +valid_timeval32_to_timeval64 (const struct __timeval32 tv)
> +{
> +  return (struct __timeval64) { tv.tv_sec, tv.tv_usec };
> +}
> +
> +static inline struct __timeval32
> +valid_timeval64_to_timeval32 (const struct __timeval64 tv64)
> +{
> +  return (struct __timeval32) { tv64.tv_sec, tv64.tv_usec };
> +}
> +
> +static inline struct timeval
> +valid_timeval32_to_timeval (const struct __timeval32 tv)
> +{
> +  return (struct timeval) { tv.tv_sec, tv.tv_usec };
> +}
> +
> +static inline struct timespec
> +valid_timeval32_to_timespec (const struct __timeval32 tv)
> +{
> +  return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 };
> +}
> +
> +static inline struct __timeval32
> +valid_timespec_to_timeval32 (const struct timespec ts)
> +{
> +  return (struct __timeval32) { (time_t) ts.tv_sec, ts.tv_nsec /
> 1000 }; +}
> +
>  /* Check if a value is in the valid nanoseconds range. Return true if
>     it is, false otherwise.  */
>  static inline bool




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 3/6] time: Add a __itimerval64 struct

[Alistair Francis]

Add a __itimerval64 which always uses a 64-bit time_t.

Reviewed-by: Lukasz Majewski <lukma@denx.de>
---
 include/time.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/time.h b/include/time.h
index f22abc13f4..684dabba9c 100644
--- a/include/time.h
+++ b/include/time.h
@@ -108,6 +108,17 @@ struct __timeval64
 };
 #endif
 
+#if __TIMESIZE == 64
+# define __itimerval64 itimerval
+#else
+/* The glibc's internal representation of the struct itimerval.  */
+struct __itimerval64
+{
+  struct __timeval64 it_interval;
+  struct __timeval64 it_value;
+};
+#endif
+
 #if __TIMESIZE == 64
 # define __ctime64 ctime
 #else
-- 
2.25.0

[PATCH v2 4/6] linux: Use long time_t __getitimer/__setitimer

[Alistair Francis]

The Linux kernel expects itimerval to use a 32-bit time_t, even on archs
with a 64-bit time_t (like RV32). To address this let's convert
itimerval to/from 32-bit and 64-bit to ensure the kernel always gets
a 32-bit time_t.

While we are converting these functions let's also convert them to be
the y2038 safe versions. This means there is a *64 function that is
called by a backwards compatible wrapper.
---
 include/time.h                        | 12 ++++
 sysdeps/unix/sysv/linux/getitimer.c   | 54 ++++++++++++++++
 sysdeps/unix/sysv/linux/setitimer.c   | 89 +++++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/tv32-compat.h | 35 +++++++++++
 4 files changed, 190 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/getitimer.c
 create mode 100644 sysdeps/unix/sysv/linux/setitimer.c
 create mode 100644 sysdeps/unix/sysv/linux/tv32-compat.h

diff --git a/include/time.h b/include/time.h
index 684dabba9c..52b1aba70e 100644
--- a/include/time.h
+++ b/include/time.h
@@ -6,6 +6,7 @@
 # include <bits/types/locale_t.h>
 # include <stdbool.h>
 # include <time/mktime-internal.h>
+# include <sys/time.h>
 # include <endian.h>
 # include <time-clockid.h>
 # include <sys/time.h>
@@ -119,6 +120,17 @@ struct __itimerval64
 };
 #endif
 
+#if __TIMESIZE == 64
+# define __getitimer64 __getitimer
+# define __setitimer64 __setitimer
+#else
+extern int __getitimer64 (enum __itimer_which __which,
+                          struct __itimerval64 *__value);
+extern int __setitimer64 (enum __itimer_which __which,
+                          const struct __itimerval64 *__restrict __new,
+                          struct __itimerval64 *__restrict __old);
+#endif
+
 #if __TIMESIZE == 64
 # define __ctime64 ctime
 #else
diff --git a/sysdeps/unix/sysv/linux/getitimer.c b/sysdeps/unix/sysv/linux/getitimer.c
new file mode 100644
index 0000000000..28a3e31126
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/getitimer.c
@@ -0,0 +1,54 @@
+/* getitimer -- Get the state of an interval timer.  Linux/tv32 version.
+   Copyright (C) 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <time.h>
+#include <sys/time.h>
+#include <sysdep.h>
+#include <tv32-compat.h>
+
+int
+__getitimer64 (__itimer_which_t which, struct __itimerval64 *curr_value)
+{
+  struct __itimerval32 curr_value_32;
+  if (INLINE_SYSCALL_CALL (getitimer, which, &curr_value_32) == -1)
+    return -1;
+
+  /* Write all fields of 'curr_value' regardless of overflow.  */
+  curr_value->it_interval
+    = valid_timeval32_to_timeval64 (curr_value_32.it_interval);
+  curr_value->it_value
+    = valid_timeval32_to_timeval64 (curr_value_32.it_value);
+  return 0;
+}
+
+
+#if __TIMESIZE != 64
+int
+__getitimer (__itimer_which_t which, struct itimerval *curr_value)
+{
+  struct __itimerval64 val64;
+
+  val64.it_interval
+    = valid_timeval_to_timeval64 (curr_value->it_interval);
+  val64.it_value
+    = valid_timeval_to_timeval64 (curr_value->it_value);
+
+  return __getitimer64 (which, &val64);
+}
+#endif
+weak_alias (__getitimer, getitimer)
diff --git a/sysdeps/unix/sysv/linux/setitimer.c b/sysdeps/unix/sysv/linux/setitimer.c
new file mode 100644
index 0000000000..99b40b3db2
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/setitimer.c
@@ -0,0 +1,89 @@
+/* setitimer -- Set the state of an interval timer.  Linux/tv32 version.
+   Copyright (C) 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <time.h>
+#include <sys/time.h>
+#include <sysdep.h>
+#include <tv32-compat.h>
+
+int
+__setitimer64 (__itimer_which_t which,
+               const struct __itimerval64 *restrict new_value,
+               struct __itimerval64 *restrict old_value)
+{
+  struct __itimerval32 new_value_32;
+
+  if (! in_time_t_range (new_value->it_interval.tv_sec))
+  {
+    __set_errno (EOVERFLOW);
+    return -1;
+  }
+  new_value_32.it_interval
+    = valid_timeval64_to_timeval32 (new_value->it_interval);
+
+  if (! in_time_t_range (new_value->it_value.tv_sec))
+  {
+    __set_errno (EOVERFLOW);
+    return -1;
+  }
+  new_value_32.it_value
+    = valid_timeval64_to_timeval32 (new_value->it_value);
+
+  if (old_value == NULL)
+    return INLINE_SYSCALL_CALL (setitimer, which, &new_value_32, NULL);
+
+  struct __itimerval32 old_value_32;
+  if (INLINE_SYSCALL_CALL (setitimer, which, &new_value_32, &old_value_32) == -1)
+    return -1;
+
+  /* Write all fields of 'old_value' regardless of overflow.  */
+  old_value->it_interval
+     = valid_timeval32_to_timeval64 (old_value_32.it_interval);
+  old_value->it_value
+     = valid_timeval32_to_timeval64 (old_value_32.it_value);
+  return 0;
+}
+
+#if __TIMESIZE != 64
+int
+__setitimer (__itimer_which_t which,
+             const struct itimerval *restrict new_value,
+             struct itimerval *restrict old_value)
+{
+  int ret;
+  struct __itimerval64 new64, old64;
+
+  new64.it_interval
+    = valid_timeval_to_timeval64 (new_value->it_interval);
+  new64.it_value
+    = valid_timeval_to_timeval64 (new_value->it_value);
+
+  ret = __setitimer64 (which, &new64, &old64);
+
+  if (ret != 0)
+    return ret;
+
+  old_value->it_interval
+    = valid_timeval64_to_timeval (old64.it_interval);
+  old_value->it_value
+    = valid_timeval64_to_timeval (old64.it_value);
+
+  return ret;
+}
+#endif
+weak_alias (__setitimer, setitimer)
diff --git a/sysdeps/unix/sysv/linux/tv32-compat.h b/sysdeps/unix/sysv/linux/tv32-compat.h
new file mode 100644
index 0000000000..4eb6f216ea
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tv32-compat.h
@@ -0,0 +1,35 @@
+/* Compatibility definitions for `struct timeval' with 32-bit time_t.
+   Copyright (C) 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
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _TV32_COMPAT_H
+#define _TV32_COMPAT_H 1
+
+#include <features.h>
+
+#include <bits/types.h>
+#include <bits/types/time_t.h>
+#include <bits/types/struct_timeval.h>
+
+/* Structures containing 'struct timeval' with 32-bit time_t.  */
+struct __itimerval32
+{
+  struct __timeval32 it_interval;
+  struct __timeval32 it_value;
+};
+
+#endif /* tv32-compat.h */
-- 
2.25.0

Re: [PATCH v2 4/6] linux: Use long time_t __getitimer/__setitimer

[Vineet Gupta]

Hi Alistair,

On 2/10/20 9:43 AM, Alistair Francis wrote:
> The Linux kernel expects itimerval to use a 32-bit time_t, even on archs
> with a 64-bit time_t (like RV32). To address this let's convert
> itimerval to/from 32-bit and 64-bit to ensure the kernel always gets
> a 32-bit time_t.
> 
> While we are converting these functions let's also convert them to be
> the y2038 safe versions. This means there is a *64 function that is
> called by a backwards compatible wrapper.
> ---

> +
> +int
> +__setitimer64 (__itimer_which_t which,
> +               const struct __itimerval64 *restrict new_value,
> +               struct __itimerval64 *restrict old_value)
> +{
> +  struct __itimerval32 new_value_32;
> +
> +  if (! in_time_t_range (new_value->it_interval.tv_sec))
> +  {
> +    __set_errno (EOVERFLOW);
> +    return -1;
> +  }
> +  new_value_32.it_interval
> +    = valid_timeval64_to_timeval32 (new_value->it_interval);
> +
> +  if (! in_time_t_range (new_value->it_value.tv_sec))
> +  {
> +    __set_errno (EOVERFLOW);
> +    return -1;
> +  }
> +  new_value_32.it_value
> +    = valid_timeval64_to_timeval32 (new_value->it_value);
> +
> +  if (old_value == NULL)
> +    return INLINE_SYSCALL_CALL (setitimer, which, &new_value_32, NULL);
> +
> +  struct __itimerval32 old_value_32;
> +  if (INLINE_SYSCALL_CALL (setitimer, which, &new_value_32, &old_value_32) == -1)
> +    return -1;
> +
> +  /* Write all fields of 'old_value' regardless of overflow.  */
> +  old_value->it_interval
> +     = valid_timeval32_to_timeval64 (old_value_32.it_interval);
> +  old_value->it_value
> +     = valid_timeval32_to_timeval64 (old_value_32.it_value);
> +  return 0;
> +}
> +
> +#if __TIMESIZE != 64
> +int
> +__setitimer (__itimer_which_t which,
> +             const struct itimerval *restrict new_value,
> +             struct itimerval *restrict old_value)
> +{
> +  int ret;
> +  struct __itimerval64 new64, old64;
> +
> +  new64.it_interval
> +    = valid_timeval_to_timeval64 (new_value->it_interval);
> +  new64.it_value
> +    = valid_timeval_to_timeval64 (new_value->it_value);
> +
> +  ret = __setitimer64 (which, &new64, &old64);
> +
> +  if (ret != 0)
> +    return ret;

I tested ARC port over your v1 next branch and it works fine in general. I still
had 32-bit time_t so you have some more test coverage ;-)

The glibc testsuite had some new failures, some of them are coming from the
unchecked @old_value dereference (which would not hit for 64-bit time_t).

Care to fix it please.

> +
> +  old_value->it_interval
> +    = valid_timeval64_to_timeval (old64.it_interval);
> +  old_value->it_value
> +    = valid_timeval64_to_timeval (old64.it_value);
> +
> +  return ret;
> +}
> +#endif
> +weak_alias (__setitimer, setitimer)
Thx,
-Vineet

Re: [PATCH v2 4/6] linux: Use long time_t __getitimer/__setitimer

[Alistair Francis]

On Tue, Feb 11, 2020 at 12:02 PM Vineet Gupta <vineetg76@gmail.com> wrote:
>
> Hi Alistair,
>
> On 2/10/20 9:43 AM, Alistair Francis wrote:
> > The Linux kernel expects itimerval to use a 32-bit time_t, even on archs
> > with a 64-bit time_t (like RV32). To address this let's convert
> > itimerval to/from 32-bit and 64-bit to ensure the kernel always gets
> > a 32-bit time_t.
> >
> > While we are converting these functions let's also convert them to be
> > the y2038 safe versions. This means there is a *64 function that is
> > called by a backwards compatible wrapper.
> > ---
>
> > +
> > +int
> > +__setitimer64 (__itimer_which_t which,
> > +               const struct __itimerval64 *restrict new_value,
> > +               struct __itimerval64 *restrict old_value)
> > +{
> > +  struct __itimerval32 new_value_32;
> > +
> > +  if (! in_time_t_range (new_value->it_interval.tv_sec))
> > +  {
> > +    __set_errno (EOVERFLOW);
> > +    return -1;
> > +  }
> > +  new_value_32.it_interval
> > +    = valid_timeval64_to_timeval32 (new_value->it_interval);
> > +
> > +  if (! in_time_t_range (new_value->it_value.tv_sec))
> > +  {
> > +    __set_errno (EOVERFLOW);
> > +    return -1;
> > +  }
> > +  new_value_32.it_value
> > +    = valid_timeval64_to_timeval32 (new_value->it_value);
> > +
> > +  if (old_value == NULL)
> > +    return INLINE_SYSCALL_CALL (setitimer, which, &new_value_32, NULL);
> > +
> > +  struct __itimerval32 old_value_32;
> > +  if (INLINE_SYSCALL_CALL (setitimer, which, &new_value_32, &old_value_32) == -1)
> > +    return -1;
> > +
> > +  /* Write all fields of 'old_value' regardless of overflow.  */
> > +  old_value->it_interval
> > +     = valid_timeval32_to_timeval64 (old_value_32.it_interval);
> > +  old_value->it_value
> > +     = valid_timeval32_to_timeval64 (old_value_32.it_value);
> > +  return 0;
> > +}
> > +
> > +#if __TIMESIZE != 64
> > +int
> > +__setitimer (__itimer_which_t which,
> > +             const struct itimerval *restrict new_value,
> > +             struct itimerval *restrict old_value)
> > +{
> > +  int ret;
> > +  struct __itimerval64 new64, old64;
> > +
> > +  new64.it_interval
> > +    = valid_timeval_to_timeval64 (new_value->it_interval);
> > +  new64.it_value
> > +    = valid_timeval_to_timeval64 (new_value->it_value);
> > +
> > +  ret = __setitimer64 (which, &new64, &old64);
> > +
> > +  if (ret != 0)
> > +    return ret;
>
> I tested ARC port over your v1 next branch and it works fine in general. I still
> had 32-bit time_t so you have some more test coverage ;-)
>
> The glibc testsuite had some new failures, some of them are coming from the
> unchecked @old_value dereference (which would not hit for 64-bit time_t).
>
> Care to fix it please.

Fixed! Thanks for testing!

Alistair

>
> > +
> > +  old_value->it_interval
> > +    = valid_timeval64_to_timeval (old64.it_interval);
> > +  old_value->it_value
> > +    = valid_timeval64_to_timeval (old64.it_value);
> > +
> > +  return ret;
> > +}
> > +#endif
> > +weak_alias (__setitimer, setitimer)
> Thx,
> -Vineet

[PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Alistair Francis]

These functions are alpha specifc, rename them to be clear.

Let's also rename the header file from tv32-compat.h to
alpha-tv32-compat.h. This is to avoid conflicts with the one we will
introduce later.
---
 .../alpha/{tv32-compat.h => alpha-tv32-compat.h} | 16 ++++++++--------
 sysdeps/unix/sysv/linux/alpha/osf_adjtime.c      | 10 +++++-----
 sysdeps/unix/sysv/linux/alpha/osf_getitimer.c    |  6 +++---
 sysdeps/unix/sysv/linux/alpha/osf_getrusage.c    |  4 ++--
 sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c |  4 ++--
 sysdeps/unix/sysv/linux/alpha/osf_setitimer.c    | 10 +++++-----
 sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c |  4 ++--
 sysdeps/unix/sysv/linux/alpha/osf_utimes.c       |  6 +++---
 sysdeps/unix/sysv/linux/alpha/osf_wait4.c        |  4 ++--
 9 files changed, 32 insertions(+), 32 deletions(-)
 rename sysdeps/unix/sysv/linux/alpha/{tv32-compat.h => alpha-tv32-compat.h} (88%)

diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h b/sysdeps/unix/sysv/linux/alpha/alpha-tv32-compat.h
similarity index 88%
rename from sysdeps/unix/sysv/linux/alpha/tv32-compat.h
rename to sysdeps/unix/sysv/linux/alpha/alpha-tv32-compat.h
index 8e34ed1c1b..3073005c65 100644
--- a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
+++ b/sysdeps/unix/sysv/linux/alpha/alpha-tv32-compat.h
@@ -70,13 +70,13 @@ struct rusage32
    overflow, they write { INT32_MAX, TV_USEC_MAX } to the output.  */
 
 static inline struct timeval
-valid_timeval32_to_timeval (const struct timeval32 tv)
+alpha_valid_timeval32_to_timeval (const struct timeval32 tv)
 {
   return (struct timeval) { tv.tv_sec, tv.tv_usec };
 }
 
 static inline struct timeval32
-valid_timeval_to_timeval32 (const struct timeval tv64)
+alpha_valid_timeval_to_timeval32 (const struct timeval tv64)
 {
   if (__glibc_unlikely (tv64.tv_sec > (time_t) INT32_MAX))
     return (struct timeval32) { INT32_MAX, TV_USEC_MAX};
@@ -84,27 +84,27 @@ valid_timeval_to_timeval32 (const struct timeval tv64)
 }
 
 static inline struct timespec
-valid_timeval32_to_timespec (const struct timeval32 tv)
+alpha_valid_timeval32_to_timespec (const struct timeval32 tv)
 {
   return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 };
 }
 
 static inline struct timeval32
-valid_timespec_to_timeval32 (const struct timespec ts)
+alpha_valid_timespec_to_timeval32 (const struct timespec ts)
 {
   return (struct timeval32) { (time_t) ts.tv_sec, ts.tv_nsec / 1000 };
 }
 
 static inline void
-rusage64_to_rusage32 (struct rusage32 *restrict r32,
+alpha_rusage64_to_rusage32 (struct rusage32 *restrict r32,
                       const struct rusage *restrict r64)
 {
   /* Make sure the entire output structure is cleared, including
      padding and reserved fields.  */
   memset (r32, 0, sizeof *r32);
 
-  r32->ru_utime    = valid_timeval_to_timeval32 (r64->ru_utime);
-  r32->ru_stime    = valid_timeval_to_timeval32 (r64->ru_stime);
+  r32->ru_utime    = alpha_valid_timeval_to_timeval32 (r64->ru_utime);
+  r32->ru_stime    = alpha_valid_timeval_to_timeval32 (r64->ru_stime);
   r32->ru_maxrss   = r64->ru_maxrss;
   r32->ru_ixrss    = r64->ru_ixrss;
   r32->ru_idrss    = r64->ru_idrss;
@@ -121,4 +121,4 @@ rusage64_to_rusage32 (struct rusage32 *restrict r32,
   r32->ru_nivcsw   = r64->ru_nivcsw;
 }
 
-#endif /* tv32-compat.h */
+#endif /* alpha-tv32-compat.h */
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
index 9825a4734d..f0a1123639 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
@@ -22,7 +22,7 @@
 
 #include <sys/time.h>
 #include <sys/timex.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
 
 struct timex32 {
 	unsigned int modes;	/* mode selector */
@@ -57,13 +57,13 @@ int
 attribute_compat_text_section
 __adjtime_tv32 (const struct timeval32 *itv, struct timeval32 *otv)
 {
-  struct timeval itv64 = valid_timeval32_to_timeval (*itv);
+  struct timeval itv64 = alpha_valid_timeval32_to_timeval (*itv);
   struct timeval otv64;
 
   if (__adjtime (&itv64, &otv64) == -1)
     return -1;
 
-  *otv = valid_timeval_to_timeval32 (otv64);
+  *otv = alpha_valid_timeval_to_timeval32 (otv64);
   return 0;
 }
 
@@ -91,7 +91,7 @@ __adjtimex_tv32 (struct timex32 *tx)
   tx64.calcnt    = tx->calcnt;
   tx64.errcnt    = tx->errcnt;
   tx64.stbcnt    = tx->stbcnt;
-  tx64.time      = valid_timeval32_to_timeval (tx->time);
+  tx64.time      = alpha_valid_timeval32_to_timeval (tx->time);
 
   int status = __adjtimex (&tx64);
   if (status < 0)
@@ -116,7 +116,7 @@ __adjtimex_tv32 (struct timex32 *tx)
   tx->calcnt    = tx64.calcnt;
   tx->errcnt    = tx64.errcnt;
   tx->stbcnt    = tx64.stbcnt;
-  tx->time      = valid_timeval_to_timeval32 (tx64.time);
+  tx->time      = alpha_valid_timeval_to_timeval32 (tx64.time);
 
   return status;
 }
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
index e9de2b287b..204d4ba796 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
@@ -21,7 +21,7 @@
 #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
 
 #include <sys/time.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
 
 int
 attribute_compat_text_section
@@ -33,9 +33,9 @@ __getitimer_tv32 (int which, struct itimerval32 *curr_value)
 
   /* Write all fields of 'curr_value' regardless of overflow.  */
   curr_value->it_interval
-    = valid_timeval_to_timeval32 (curr_value_64.it_interval);
+    = alpha_valid_timeval_to_timeval32 (curr_value_64.it_interval);
   curr_value->it_value
-    = valid_timeval_to_timeval32 (curr_value_64.it_value);
+    = alpha_valid_timeval_to_timeval32 (curr_value_64.it_value);
   return 0;
 }
 
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_getrusage.c b/sysdeps/unix/sysv/linux/alpha/osf_getrusage.c
index 74c6fb49aa..be81994654 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_getrusage.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_getrusage.c
@@ -22,7 +22,7 @@
 
 #include <sys/time.h>
 #include <sys/resource.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
 
 int
 __getrusage_tv32 (int who, struct rusage32 *usage32)
@@ -31,7 +31,7 @@ __getrusage_tv32 (int who, struct rusage32 *usage32)
   if (__getrusage (who, &usage64) == -1)
     return -1;
 
-  rusage64_to_rusage32 (usage32, &usage64);
+  alpha_rusage64_to_rusage32 (usage32, &usage64);
   return 0;
 }
 
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
index df7f06765b..9ffda2fde3 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
@@ -23,7 +23,7 @@
 #include <string.h>
 #include <time.h>
 #include <sys/time.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
 
 /* Get the current time of day and timezone information putting it
    into *TV and *TZ.  */
@@ -38,7 +38,7 @@ __gettimeofday_tv32 (struct timeval32 *restrict tv32, void *restrict tz)
   struct timespec ts;
   __clock_gettime (CLOCK_REALTIME, &ts);
 
-  *tv32 = valid_timespec_to_timeval32 (ts);
+  *tv32 = alpha_valid_timespec_to_timeval32 (ts);
   return 0;
 }
 
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
index 7df2d1b71c..726dfc8b0e 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
@@ -21,7 +21,7 @@
 #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
 
 #include <sys/time.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
 
 int
 attribute_compat_text_section
@@ -30,9 +30,9 @@ __setitimer_tv32 (int which, const struct itimerval32 *restrict new_value,
 {
   struct itimerval new_value_64;
   new_value_64.it_interval
-    = valid_timeval32_to_timeval (new_value->it_interval);
+    = alpha_valid_timeval32_to_timeval (new_value->it_interval);
   new_value_64.it_value
-    = valid_timeval32_to_timeval (new_value->it_value);
+    = alpha_valid_timeval32_to_timeval (new_value->it_value);
 
   if (old_value == NULL)
     return __setitimer (which, &new_value_64, NULL);
@@ -43,9 +43,9 @@ __setitimer_tv32 (int which, const struct itimerval32 *restrict new_value,
 
   /* Write all fields of 'old_value' regardless of overflow.  */
   old_value->it_interval
-     = valid_timeval_to_timeval32 (old_value_64.it_interval);
+     = alpha_valid_timeval_to_timeval32 (old_value_64.it_interval);
   old_value->it_value
-     = valid_timeval_to_timeval32 (old_value_64.it_value);
+     = alpha_valid_timeval_to_timeval32 (old_value_64.it_value);
   return 0;
 }
 
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c b/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c
index 6e17a95a47..044363e079 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c
@@ -23,7 +23,7 @@
 #include <sys/time.h>
 #include <time.h>
 #include <errno.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
 
 /* Set the current time of day and timezone information.
    This call is restricted to the super-user.  */
@@ -42,7 +42,7 @@ __settimeofday_tv32 (const struct timeval32 *tv32,
       return __settimezone (tz);
     }
 
-  struct timespec ts = valid_timeval32_to_timespec (*tv32);
+  struct timespec ts = alpha_valid_timeval32_to_timespec (*tv32);
   return __clock_settime (CLOCK_REALTIME, &ts);
 }
 
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
index 6c3fad0132..8ad9fb567c 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
@@ -21,15 +21,15 @@
 #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
 
 #include <sys/time.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
 
 int
 attribute_compat_text_section
 __utimes_tv32 (const char *filename, const struct timeval32 times32[2])
 {
   struct timeval times[2];
-  times[0] = valid_timeval32_to_timeval (times32[0]);
-  times[1] = valid_timeval32_to_timeval (times32[1]);
+  times[0] = alpha_valid_timeval32_to_timeval (times32[0]);
+  times[1] = alpha_valid_timeval32_to_timeval (times32[1]);
   return __utimes (filename, times);
 }
 
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_wait4.c b/sysdeps/unix/sysv/linux/alpha/osf_wait4.c
index 6af8347871..c664e8e93f 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_wait4.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_wait4.c
@@ -23,7 +23,7 @@
 #include <sys/time.h>
 #include <sys/resource.h>
 #include <sys/wait.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
 
 pid_t
 attribute_compat_text_section
@@ -33,7 +33,7 @@ __wait4_tv32 (pid_t pid, int *status, int options, struct rusage32 *usage32)
   pid_t child = __wait4 (pid, status, options, &usage64);
 
   if (child >= 0 && usage32 != NULL)
-    rusage64_to_rusage32 (usage32, &usage64);
+    alpha_rusage64_to_rusage32 (usage32, &usage64);
   return child;
 }
 
-- 
2.25.0

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Zack Weinberg]

On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> These functions are alpha specifc, rename them to be clear.

These functions were meant to be generic.  I put the file in
s/u/s/l/alpha because it was only *used* on Alpha, at the time.

What's stopping you from using these functions on all architectures?

zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Alistair Francis]

 On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
>
> On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > These functions are alpha specifc, rename them to be clear.
>
> These functions were meant to be generic.  I put the file in
> s/u/s/l/alpha because it was only *used* on Alpha, at the time.
>
> What's stopping you from using these functions on all architectures?

The Alpha functions use 32-bit versions of the timeval struct for a
64-bit architecture. For all other architectures we want to use the
word size version (regardless of time_t size) for the timeval struct.
That's the main difference.

If Alpha didn't have this requirement we could then just remove the
Alpha specific overrides after this series and use all of the new
generic y2038 infrastructure.

Alistair

>
> zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Zack Weinberg]

On Mon, Feb 10, 2020 at 4:25 PM Alistair Francis <alistair23@gmail.com> wrote:
>  On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
> >
> > On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
> > <alistair.francis@wdc.com> wrote:
> > >
> > > These functions are alpha specifc, rename them to be clear.
> >
> > These functions were meant to be generic.  I put the file in
> > s/u/s/l/alpha because it was only *used* on Alpha, at the time.
> >
> > What's stopping you from using these functions on all architectures?
>
> The Alpha functions use 32-bit versions of the timeval struct for a
> 64-bit architecture. For all other architectures we want to use the
> word size version (regardless of time_t size) for the timeval struct.
> That's the main difference.

I'm sorry, I haven't been following the discussion all that closely.
Why is it necessary to use a 32-bit timeval struct (with 32-bit
time_t, I presume) *only* on Alpha?  Is there no way we can smooth
over this difference so that, as you say,

> If Alpha didn't have this requirement we could then just remove the
> Alpha specific overrides after this series and use all of the new
> generic y2038 infrastructure.

Also, even if we really are stuck with an Alpha-specific struct
timeval32, I don't see why that affects the definitions of the
conversion functions?

zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Adhemerval Zanella]

On 10/02/2020 18:35, Zack Weinberg wrote:
> On Mon, Feb 10, 2020 at 4:25 PM Alistair Francis <alistair23@gmail.com> wrote:
>>  On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
>>>
>>> On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
>>> <alistair.francis@wdc.com> wrote:
>>>>
>>>> These functions are alpha specifc, rename them to be clear.
>>>
>>> These functions were meant to be generic.  I put the file in
>>> s/u/s/l/alpha because it was only *used* on Alpha, at the time.
>>>
>>> What's stopping you from using these functions on all architectures?
>>
>> The Alpha functions use 32-bit versions of the timeval struct for a
>> 64-bit architecture. For all other architectures we want to use the
>> word size version (regardless of time_t size) for the timeval struct.
>> That's the main difference.
> 
> I'm sorry, I haven't been following the discussion all that closely.
> Why is it necessary to use a 32-bit timeval struct (with 32-bit
> time_t, I presume) *only* on Alpha?  Is there no way we can smooth
> over this difference so that, as you say,

Alpha is an outlier here, it is the only 64-bit architecture that did
the 32 bit to 64 bit transition some time ago.  All the generic code
assumes that 64-bit architectures always have 64-bit time_t
(__ASSUME_TIME64_SYSCALLS). 

And the old interface is not exported and *only* meant to be used in
compat symbols, different than the current way of handling y2038 
prof code (where the user might still define the ABI with _TIME_SIZE).

That's why I think alpha compat code should be compartmentalized on
alpha sysdep folder.

> 
>> If Alpha didn't have this requirement we could then just remove the
>> Alpha specific overrides after this series and use all of the new
>> generic y2038 infrastructure.
> 
> Also, even if we really are stuck with an Alpha-specific struct
> timeval32, I don't see why that affects the definitions of the
> conversion functions?
> 
> zw
> 

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Alistair Francis]

On Mon, Feb 10, 2020 at 1:53 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 10/02/2020 18:35, Zack Weinberg wrote:
> > On Mon, Feb 10, 2020 at 4:25 PM Alistair Francis <alistair23@gmail.com> wrote:
> >>  On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
> >>>
> >>> On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
> >>> <alistair.francis@wdc.com> wrote:
> >>>>
> >>>> These functions are alpha specifc, rename them to be clear.
> >>>
> >>> These functions were meant to be generic.  I put the file in
> >>> s/u/s/l/alpha because it was only *used* on Alpha, at the time.
> >>>
> >>> What's stopping you from using these functions on all architectures?
> >>
> >> The Alpha functions use 32-bit versions of the timeval struct for a
> >> 64-bit architecture. For all other architectures we want to use the
> >> word size version (regardless of time_t size) for the timeval struct.
> >> That's the main difference.
> >
> > I'm sorry, I haven't been following the discussion all that closely.
> > Why is it necessary to use a 32-bit timeval struct (with 32-bit
> > time_t, I presume) *only* on Alpha?  Is there no way we can smooth
> > over this difference so that, as you say,
>
> Alpha is an outlier here, it is the only 64-bit architecture that did
> the 32 bit to 64 bit transition some time ago.  All the generic code
> assumes that 64-bit architectures always have 64-bit time_t
> (__ASSUME_TIME64_SYSCALLS).
>
> And the old interface is not exported and *only* meant to be used in
> compat symbols, different than the current way of handling y2038
> prof code (where the user might still define the ABI with _TIME_SIZE).
>
> That's why I think alpha compat code should be compartmentalized on
> alpha sysdep folder.

Agreed. That is the goal with this patch.

This patch moves and renames the Alpha functions to make sure there is
no conflict with any of the new work.

Alistair

>
> >
> >> If Alpha didn't have this requirement we could then just remove the
> >> Alpha specific overrides after this series and use all of the new
> >> generic y2038 infrastructure.
> >
> > Also, even if we really are stuck with an Alpha-specific struct
> > timeval32, I don't see why that affects the definitions of the
> > conversion functions?
> >
> > zw
> >

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Zack Weinberg]

On Mon, Feb 10, 2020 at 4:53 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 10/02/2020 18:35, Zack Weinberg wrote:
> > On Mon, Feb 10, 2020 at 4:25 PM Alistair Francis <alistair23@gmail.com> wrote:
> >>  On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
> >>>
> >>> On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
> >>> <alistair.francis@wdc.com> wrote:
> >>>>
> >>>> These functions are alpha specifc, rename them to be clear.
> >>>
> >>> These functions were meant to be generic.  I put the file in
> >>> s/u/s/l/alpha because it was only *used* on Alpha, at the time.
> >>>
> >>> What's stopping you from using these functions on all architectures?
> >>
> >> The Alpha functions use 32-bit versions of the timeval struct for a
> >> 64-bit architecture. For all other architectures we want to use the
> >> word size version (regardless of time_t size) for the timeval struct.
> >> That's the main difference.
> >
> > I'm sorry, I haven't been following the discussion all that closely.
> > Why is it necessary to use a 32-bit timeval struct (with 32-bit
> > time_t, I presume) *only* on Alpha?  Is there no way we can smooth
> > over this difference so that, as you say,
>
> Alpha is an outlier here, it is the only 64-bit architecture that did
> the 32 bit to 64 bit transition some time ago.  All the generic code
> assumes that 64-bit architectures always have 64-bit time_t
> (__ASSUME_TIME64_SYSCALLS).
>
> And the old interface is not exported and *only* meant to be used in
> compat symbols, different than the current way of handling y2038
> prof code (where the user might still define the ABI with _TIME_SIZE).
>
> That's why I think alpha compat code should be compartmentalized on
> alpha sysdep folder.

This doesn't address any of my concerns.  It should not be necessary
to duplicate an *internal header* full of functions whose operation
is, or ought to be, completely generic, just because the exposed API
is different on Alpha.

zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Adhemerval Zanella]

On 10/02/2020 22:10, Zack Weinberg wrote:
> On Mon, Feb 10, 2020 at 4:53 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> On 10/02/2020 18:35, Zack Weinberg wrote:
>>> On Mon, Feb 10, 2020 at 4:25 PM Alistair Francis <alistair23@gmail.com> wrote:
>>>>  On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
>>>>>
>>>>> On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
>>>>> <alistair.francis@wdc.com> wrote:
>>>>>>
>>>>>> These functions are alpha specifc, rename them to be clear.
>>>>>
>>>>> These functions were meant to be generic.  I put the file in
>>>>> s/u/s/l/alpha because it was only *used* on Alpha, at the time.
>>>>>
>>>>> What's stopping you from using these functions on all architectures?
>>>>
>>>> The Alpha functions use 32-bit versions of the timeval struct for a
>>>> 64-bit architecture. For all other architectures we want to use the
>>>> word size version (regardless of time_t size) for the timeval struct.
>>>> That's the main difference.
>>>
>>> I'm sorry, I haven't been following the discussion all that closely.
>>> Why is it necessary to use a 32-bit timeval struct (with 32-bit
>>> time_t, I presume) *only* on Alpha?  Is there no way we can smooth
>>> over this difference so that, as you say,
>>
>> Alpha is an outlier here, it is the only 64-bit architecture that did
>> the 32 bit to 64 bit transition some time ago.  All the generic code
>> assumes that 64-bit architectures always have 64-bit time_t
>> (__ASSUME_TIME64_SYSCALLS).
>>
>> And the old interface is not exported and *only* meant to be used in
>> compat symbols, different than the current way of handling y2038
>> prof code (where the user might still define the ABI with _TIME_SIZE).
>>
>> That's why I think alpha compat code should be compartmentalized on
>> alpha sysdep folder.
> 
> This doesn't address any of my concerns.  It should not be necessary
> to duplicate an *internal header* full of functions whose operation
> is, or ought to be, completely generic, just because the exposed API
> is different on Alpha.

The 32-bit timeval struct are alpha specific in a sense that no other
64-bit architectures have 32 time_t. 

We could certainly make it generic and add even more internally 
pre-processor magic to fit alpha code in generic definitions, but I
think it is really a worthless complication.  It is a legacy API, 
and it is highly unlikely that any other port will use such code.

I tend to see that generic code, such as the syscall consolidation
I have done over the years, should be defined when either you have
multiples ABIs that use it or if is the default for newer ports
(such as generic kABI).  Outliers implementations, such osf alpha
syscalls, should be secluded in alpha-only code.

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Zack Weinberg]

On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 10/02/2020 22:10, Zack Weinberg wrote:
> >
> > This doesn't address any of my concerns.  It should not be necessary
> > to duplicate an *internal header* full of functions whose operation
> > is, or ought to be, completely generic, just because the exposed API
> > is different on Alpha.
>
> The 32-bit timeval struct are alpha specific in a sense that no other
> 64-bit architectures have 32 time_t.
>
> We could certainly make it generic and add even more internally
> pre-processor magic to fit alpha code in generic definitions, but I
> think it is really a worthless complication.  It is a legacy API,
> and it is highly unlikely that any other port will use such code.

I think we're talking past each other.  This patch is about
tv32-compat.h.  tv32-compat.h contains conversion functions between
e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
time_t.  I still don't see any reason why these conversion functions
need to be different on Alpha than they are on the 32-bit
architectures.

Alpha is unusual in that certain legacy code paths need to call these
conversion functions, but *what the functions do* should be identical
to what they do on 32-bit architectures.  There should be no need for
preprocessor magic or anything.

zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Alistair Francis]

On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
>
> On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
> > On 10/02/2020 22:10, Zack Weinberg wrote:
> > >
> > > This doesn't address any of my concerns.  It should not be necessary
> > > to duplicate an *internal header* full of functions whose operation
> > > is, or ought to be, completely generic, just because the exposed API
> > > is different on Alpha.
> >
> > The 32-bit timeval struct are alpha specific in a sense that no other
> > 64-bit architectures have 32 time_t.
> >
> > We could certainly make it generic and add even more internally
> > pre-processor magic to fit alpha code in generic definitions, but I
> > think it is really a worthless complication.  It is a legacy API,
> > and it is highly unlikely that any other port will use such code.
>
> I think we're talking past each other.  This patch is about
> tv32-compat.h.  tv32-compat.h contains conversion functions between
> e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> time_t.  I still don't see any reason why these conversion functions

The generic patches added by this series convert between a 64-bit
time_t and a wordsize time_t. Alpha is different in that it converts
between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.

That is why Adhemerval is saying we would need some preprocessor magic
to ensure that for Alpha we always convert to/from a 32-bit time_t on
a 64-bit arch.

> need to be different on Alpha than they are on the 32-bit
> architectures.

My understanding is that Alpha is a 64-bit architecture (correct me if
I'm wrong, that's just from Wikipedia). The generic patches convert
time_t to the architectures wordsize before passing it to the kernel
(either 32-bit or 64-bit) while Alpha is different here.

>
> Alpha is unusual in that certain legacy code paths need to call these
> conversion functions, but *what the functions do* should be identical
> to what they do on 32-bit architectures.  There should be no need for
> preprocessor magic or anything.

Yes, they are the same as the 32-bit architectures, but not the 64-bit
architectures. Everything else in this patch is generic for both
32-bit and 64-bit (except for Alpha).

Alistair

>
> zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Zack Weinberg]

On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> >
> > On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> > > On 10/02/2020 22:10, Zack Weinberg wrote:
> > > >
> > > > This doesn't address any of my concerns.  It should not be necessary
> > > > to duplicate an *internal header* full of functions whose operation
> > > > is, or ought to be, completely generic, just because the exposed API
> > > > is different on Alpha.
> > >
> > > The 32-bit timeval struct are alpha specific in a sense that no other
> > > 64-bit architectures have 32 time_t.
> > >
> > > We could certainly make it generic and add even more internally
> > > pre-processor magic to fit alpha code in generic definitions, but I
> > > think it is really a worthless complication.  It is a legacy API,
> > > and it is highly unlikely that any other port will use such code.
> >
> > I think we're talking past each other.  This patch is about
> > tv32-compat.h.  tv32-compat.h contains conversion functions between
> > e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> > time_t.  I still don't see any reason why these conversion functions
>
> The generic patches added by this series convert between a 64-bit
> time_t and a wordsize time_t. Alpha is different in that it converts
> between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.

Yes, I understand that, but the definition of the 32-bit-time_t struct
timeval is

  struct __timeval32 {
     __time32_t tv_sec;
     __suseconds_t tv_usec;
  };

either way, isn't it?  So the conversion functions shouldn't care?

(If that's not what the definition of the 32-bit-time_t struct timeval
is, then I would like to know why, in *excruciating* detail.)

zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Adhemerval Zanella]

On 11/02/2020 13:23, Zack Weinberg wrote:
> On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
>>
>> On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
>>>
>>> On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
>>> <adhemerval.zanella@linaro.org> wrote:
>>>> On 10/02/2020 22:10, Zack Weinberg wrote:
>>>>>
>>>>> This doesn't address any of my concerns.  It should not be necessary
>>>>> to duplicate an *internal header* full of functions whose operation
>>>>> is, or ought to be, completely generic, just because the exposed API
>>>>> is different on Alpha.
>>>>
>>>> The 32-bit timeval struct are alpha specific in a sense that no other
>>>> 64-bit architectures have 32 time_t.
>>>>
>>>> We could certainly make it generic and add even more internally
>>>> pre-processor magic to fit alpha code in generic definitions, but I
>>>> think it is really a worthless complication.  It is a legacy API,
>>>> and it is highly unlikely that any other port will use such code.
>>>
>>> I think we're talking past each other.  This patch is about
>>> tv32-compat.h.  tv32-compat.h contains conversion functions between
>>> e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
>>> time_t.  I still don't see any reason why these conversion functions
>>
>> The generic patches added by this series convert between a 64-bit
>> time_t and a wordsize time_t. Alpha is different in that it converts
>> between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
> 
> Yes, I understand that, but the definition of the 32-bit-time_t struct
> timeval is
> 
>   struct __timeval32 {
>      __time32_t tv_sec;
>      __suseconds_t tv_usec;
>   };
> 
> either way, isn't it?  So the conversion functions shouldn't care?
> 
> (If that's not what the definition of the 32-bit-time_t struct timeval
> is, then I would like to know why, in *excruciating* detail.)

Yes, but why expose this specific types on architectures that do not
really require it?  Sure we could make this code generic, but again
this will be used solely by alpha.

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Zack Weinberg]

On Tue, Feb 11, 2020 at 12:14 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 11/02/2020 13:23, Zack Weinberg wrote:
> > On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
> >>
> >> On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> >>>
> >>> On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> >>> <adhemerval.zanella@linaro.org> wrote:
> >>>> On 10/02/2020 22:10, Zack Weinberg wrote:
> >>>>>
> >>>>> This doesn't address any of my concerns.  It should not be necessary
> >>>>> to duplicate an *internal header* full of functions whose operation
> >>>>> is, or ought to be, completely generic, just because the exposed API
> >>>>> is different on Alpha.
> >>>>
> >>>> The 32-bit timeval struct are alpha specific in a sense that no other
> >>>> 64-bit architectures have 32 time_t.
> >>>>
> >>>> We could certainly make it generic and add even more internally
> >>>> pre-processor magic to fit alpha code in generic definitions, but I
> >>>> think it is really a worthless complication.  It is a legacy API,
> >>>> and it is highly unlikely that any other port will use such code.
> >>>
> >>> I think we're talking past each other.  This patch is about
> >>> tv32-compat.h.  tv32-compat.h contains conversion functions between
> >>> e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> >>> time_t.  I still don't see any reason why these conversion functions
> >>
> >> The generic patches added by this series convert between a 64-bit
> >> time_t and a wordsize time_t. Alpha is different in that it converts
> >> between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
> >
> > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > timeval is
> >
> >   struct __timeval32 {
> >      __time32_t tv_sec;
> >      __suseconds_t tv_usec;
> >   };
> >
> > either way, isn't it?  So the conversion functions shouldn't care?
> >
> > (If that's not what the definition of the 32-bit-time_t struct timeval
> > is, then I would like to know why, in *excruciating* detail.)
>
> Yes, but why expose this specific types on architectures that do not
> really require it?  Sure we could make this code generic, but again
> this will be used solely by alpha.

Huh? Isn't it the exact same definition that would be used on the
32-bit architectures?  All the variation should be hiding in the
definition of __time32_t?

zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Alistair Francis]

On Tue, Feb 11, 2020 at 8:23 AM Zack Weinberg <zackw@panix.com> wrote:
>
> On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> > >
> > > On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> > > <adhemerval.zanella@linaro.org> wrote:
> > > > On 10/02/2020 22:10, Zack Weinberg wrote:
> > > > >
> > > > > This doesn't address any of my concerns.  It should not be necessary
> > > > > to duplicate an *internal header* full of functions whose operation
> > > > > is, or ought to be, completely generic, just because the exposed API
> > > > > is different on Alpha.
> > > >
> > > > The 32-bit timeval struct are alpha specific in a sense that no other
> > > > 64-bit architectures have 32 time_t.
> > > >
> > > > We could certainly make it generic and add even more internally
> > > > pre-processor magic to fit alpha code in generic definitions, but I
> > > > think it is really a worthless complication.  It is a legacy API,
> > > > and it is highly unlikely that any other port will use such code.
> > >
> > > I think we're talking past each other.  This patch is about
> > > tv32-compat.h.  tv32-compat.h contains conversion functions between
> > > e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> > > time_t.  I still don't see any reason why these conversion functions
> >
> > The generic patches added by this series convert between a 64-bit
> > time_t and a wordsize time_t. Alpha is different in that it converts
> > between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
>
> Yes, I understand that, but the definition of the 32-bit-time_t struct
> timeval is
>
>   struct __timeval32 {
>      __time32_t tv_sec;
>      __suseconds_t tv_usec;
>   };

This is what the Alpha specific calls use.

The generic one uses (the name is confusing, I thought I had changed it)

struct __timeval32
{
  long tv_sec;         /* Seconds.  */
  long tv_usec;        /* Microseconds.  */
};

This is because, as I mentioned earlier, that the kernel expects the
time to be the wordsize, regardless of the time_t size (except for
Alpha).

Using long types means that we can do the same thing for 32-bit and
64-bit architectures.

My v1 patchset had a specific 32-bit only version (which didn't apply
to 64-bit arches) but this didn't work with ARM, see the discussion
here: https://patchwork.ozlabs.org/patch/1232973/

Alistair

>
> either way, isn't it?  So the conversion functions shouldn't care?
>
> (If that's not what the definition of the 32-bit-time_t struct timeval
> is, then I would like to know why, in *excruciating* detail.)
>
> zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Alistair Francis]

On Tue, Feb 11, 2020 at 9:56 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Feb 11, 2020 at 8:23 AM Zack Weinberg <zackw@panix.com> wrote:
> >
> > On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> > > >
> > > > On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> > > > <adhemerval.zanella@linaro.org> wrote:
> > > > > On 10/02/2020 22:10, Zack Weinberg wrote:
> > > > > >
> > > > > > This doesn't address any of my concerns.  It should not be necessary
> > > > > > to duplicate an *internal header* full of functions whose operation
> > > > > > is, or ought to be, completely generic, just because the exposed API
> > > > > > is different on Alpha.
> > > > >
> > > > > The 32-bit timeval struct are alpha specific in a sense that no other
> > > > > 64-bit architectures have 32 time_t.
> > > > >
> > > > > We could certainly make it generic and add even more internally
> > > > > pre-processor magic to fit alpha code in generic definitions, but I
> > > > > think it is really a worthless complication.  It is a legacy API,
> > > > > and it is highly unlikely that any other port will use such code.
> > > >
> > > > I think we're talking past each other.  This patch is about
> > > > tv32-compat.h.  tv32-compat.h contains conversion functions between
> > > > e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> > > > time_t.  I still don't see any reason why these conversion functions
> > >
> > > The generic patches added by this series convert between a 64-bit
> > > time_t and a wordsize time_t. Alpha is different in that it converts
> > > between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
> >
> > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > timeval is
> >
> >   struct __timeval32 {
> >      __time32_t tv_sec;
> >      __suseconds_t tv_usec;
> >   };
>
> This is what the Alpha specific calls use.
>
> The generic one uses (the name is confusing, I thought I had changed it)
>
> struct __timeval32
> {
>   long tv_sec;         /* Seconds.  */
>   long tv_usec;        /* Microseconds.  */
> };

I have changed the name of this struct to __timeval_long, hopefully
that fixes some of the confusion. The name was left over from v1.

Alistair

>
> This is because, as I mentioned earlier, that the kernel expects the
> time to be the wordsize, regardless of the time_t size (except for
> Alpha).
>
> Using long types means that we can do the same thing for 32-bit and
> 64-bit architectures.
>
> My v1 patchset had a specific 32-bit only version (which didn't apply
> to 64-bit arches) but this didn't work with ARM, see the discussion
> here: https://patchwork.ozlabs.org/patch/1232973/
>
> Alistair
>
> >
> > either way, isn't it?  So the conversion functions shouldn't care?
> >
> > (If that's not what the definition of the 32-bit-time_t struct timeval
> > is, then I would like to know why, in *excruciating* detail.)
> >
> > zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Zack Weinberg]

On Tue, Feb 11, 2020 at 1:04 PM Alistair Francis <alistair23@gmail.com> wrote:
> > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > timeval is
> >
> >   struct __timeval32 {
> >      __time32_t tv_sec;
> >      __suseconds_t tv_usec;
> >   };
>
> This is what the Alpha specific calls use.
> The generic one uses (the name is confusing, I thought I had changed it)
>
> struct __timeval32
> {
>   long tv_sec;         /* Seconds.  */
>   long tv_usec;        /* Microseconds.  */
> };
>
> This is because, as I mentioned earlier, that the kernel expects the
> time to be the wordsize, regardless of the time_t size (except for
> Alpha).

I think that's profoundly wrong.  I think we should be using a
definition along the lines of

struct __timeval32 {
    __time32_t tv_sec;
    __suseconds32_t tv_usec;
};

on all architectures, with all necessary variation handled within the
definitions of __time32_t and __suseconds32_t.  That way it can all
live in bits/typesizes.h which has to be system-specific regardless.

Please consider this a sustained objection to the entire patchset, not
just this one patch, unless you can come back with a convincing reason
why the above is not possible or would actually make life more
complicated.

(Incidentally, since it is probably going to come up yet again if we
proceed down this path, I still believe that the specification of
`long tv_nsec` in struct timespec is a defect in C11 and POSIX and we
should ignore both standards and use a typedef name for it.)

zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Alistair Francis]

On Tue, Feb 11, 2020 at 10:38 AM Zack Weinberg <zackw@panix.com> wrote:
>
> On Tue, Feb 11, 2020 at 1:04 PM Alistair Francis <alistair23@gmail.com> wrote:
> > > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > > timeval is
> > >
> > >   struct __timeval32 {
> > >      __time32_t tv_sec;
> > >      __suseconds_t tv_usec;
> > >   };
> >
> > This is what the Alpha specific calls use.
> > The generic one uses (the name is confusing, I thought I had changed it)
> >
> > struct __timeval32
> > {
> >   long tv_sec;         /* Seconds.  */
> >   long tv_usec;        /* Microseconds.  */
> > };
> >
> > This is because, as I mentioned earlier, that the kernel expects the
> > time to be the wordsize, regardless of the time_t size (except for
> > Alpha).
>
> I think that's profoundly wrong.  I think we should be using a
> definition along the lines of
>
> struct __timeval32 {
>     __time32_t tv_sec;
>     __suseconds32_t tv_usec;
> };
>
> on all architectures, with all necessary variation handled within the
> definitions of __time32_t and __suseconds32_t.  That way it can all
> live in bits/typesizes.h which has to be system-specific regardless.

When you say all architectures, you mean just 32-bit architectures and
Alpha right?

This is what my v1 did, and the general consensus was against that as
it didn't work for all 32-bit architectures (see the discussion here:
https://patchwork.ozlabs.org/patch/1232973/).

>
> Please consider this a sustained objection to the entire patchset, not
> just this one patch, unless you can come back with a convincing reason
> why the above is not possible or would actually make life more
> complicated.

What are you proposing I do instead?

>
> (Incidentally, since it is probably going to come up yet again if we
> proceed down this path, I still believe that the specification of
> `long tv_nsec` in struct timespec is a defect in C11 and POSIX and we
> should ignore both standards and use a typedef name for it.)

I'll leave this for others to comment on, I don't really mind either
way but ignoring the standard seems like a strange choice.

Alistair

>
> zw

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Stepan Golosunov]

11.02.2020 ц≈ 09:56:59 -0800 Alistair Francis ц▌ц│ц░ц┴ц⌠ц│ц▄(ц│):
> On Tue, Feb 11, 2020 at 8:23 AM Zack Weinberg <zackw@panix.com> wrote:
> >
> > On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> > > >
> > > > On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> > > > <adhemerval.zanella@linaro.org> wrote:
> > > > > On 10/02/2020 22:10, Zack Weinberg wrote:
> > > > > >
> > > > > > This doesn't address any of my concerns.  It should not be necessary
> > > > > > to duplicate an *internal header* full of functions whose operation
> > > > > > is, or ought to be, completely generic, just because the exposed API
> > > > > > is different on Alpha.
> > > > >
> > > > > The 32-bit timeval struct are alpha specific in a sense that no other
> > > > > 64-bit architectures have 32 time_t.
> > > > >
> > > > > We could certainly make it generic and add even more internally
> > > > > pre-processor magic to fit alpha code in generic definitions, but I
> > > > > think it is really a worthless complication.  It is a legacy API,
> > > > > and it is highly unlikely that any other port will use such code.
> > > >
> > > > I think we're talking past each other.  This patch is about
> > > > tv32-compat.h.  tv32-compat.h contains conversion functions between
> > > > e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> > > > time_t.  I still don't see any reason why these conversion functions
> > >
> > > The generic patches added by this series convert between a 64-bit
> > > time_t and a wordsize time_t. Alpha is different in that it converts
> > > between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
> >
> > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > timeval is
> >
> >   struct __timeval32 {
> >      __time32_t tv_sec;
> >      __suseconds_t tv_usec;
> >   };
> 
> This is what the Alpha specific calls use.
> 
> The generic one uses (the name is confusing, I thought I had changed it)
> 
> struct __timeval32
> {
>   long tv_sec;         /* Seconds.  */
>   long tv_usec;        /* Microseconds.  */
> };

This probably won't work for x32 and sparc64.

I suspect that it will be much simpler to have __timeval32 with 32-bit
fields and then do something like this:

int
__getitimer64 (__itimer_which_t which, struct __itimerval64 *curr_value)
{
#if KERNEL_OLD_TIMEVAL_IS_TIMEVAL64
  return INLINE_SYSCALL_CALL (getitimer, which, &curr_value);
#else
  struct __itimerval32 curr_value_32;
  if (INLINE_SYSCALL_CALL (getitimer, which, &curr_value_32) == -1)
    return -1;

  /* Write all fields of 'curr_value' regardless of overflow.  */
  curr_value->it_interval
    = valid_timeval32_to_timeval64 (curr_value_32.it_interval);
  curr_value->it_value
    = valid_timeval32_to_timeval64 (curr_value_32.it_value);
  return 0;
#endif
}

where KERNEL_OLD_TIMEVAL_IS_TIMEVAL64 needs to be true on 64-bit
architectures and x32.

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Alistair Francis]

On Tue, Feb 11, 2020 at 12:15 PM Stepan Golosunov
<stepan@golosunov.pp.ru> wrote:
>
> 11.02.2020 в 09:56:59 -0800 Alistair Francis написал(а):
> > On Tue, Feb 11, 2020 at 8:23 AM Zack Weinberg <zackw@panix.com> wrote:
> > >
> > > On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
> > > >
> > > > On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> > > > >
> > > > > On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> > > > > <adhemerval.zanella@linaro.org> wrote:
> > > > > > On 10/02/2020 22:10, Zack Weinberg wrote:
> > > > > > >
> > > > > > > This doesn't address any of my concerns.  It should not be necessary
> > > > > > > to duplicate an *internal header* full of functions whose operation
> > > > > > > is, or ought to be, completely generic, just because the exposed API
> > > > > > > is different on Alpha.
> > > > > >
> > > > > > The 32-bit timeval struct are alpha specific in a sense that no other
> > > > > > 64-bit architectures have 32 time_t.
> > > > > >
> > > > > > We could certainly make it generic and add even more internally
> > > > > > pre-processor magic to fit alpha code in generic definitions, but I
> > > > > > think it is really a worthless complication.  It is a legacy API,
> > > > > > and it is highly unlikely that any other port will use such code.
> > > > >
> > > > > I think we're talking past each other.  This patch is about
> > > > > tv32-compat.h.  tv32-compat.h contains conversion functions between
> > > > > e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> > > > > time_t.  I still don't see any reason why these conversion functions
> > > >
> > > > The generic patches added by this series convert between a 64-bit
> > > > time_t and a wordsize time_t. Alpha is different in that it converts
> > > > between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
> > >
> > > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > > timeval is
> > >
> > >   struct __timeval32 {
> > >      __time32_t tv_sec;
> > >      __suseconds_t tv_usec;
> > >   };
> >
> > This is what the Alpha specific calls use.
> >
> > The generic one uses (the name is confusing, I thought I had changed it)
> >
> > struct __timeval32
> > {
> >   long tv_sec;         /* Seconds.  */
> >   long tv_usec;        /* Microseconds.  */
> > };
>
> This probably won't work for x32 and sparc64.
>
> I suspect that it will be much simpler to have __timeval32 with 32-bit
> fields and then do something like this:
>
> int
> __getitimer64 (__itimer_which_t which, struct __itimerval64 *curr_value)
> {
> #if KERNEL_OLD_TIMEVAL_IS_TIMEVAL64
>   return INLINE_SYSCALL_CALL (getitimer, which, &curr_value);
> #else
>   struct __itimerval32 curr_value_32;
>   if (INLINE_SYSCALL_CALL (getitimer, which, &curr_value_32) == -1)
>     return -1;
>
>   /* Write all fields of 'curr_value' regardless of overflow.  */
>   curr_value->it_interval
>     = valid_timeval32_to_timeval64 (curr_value_32.it_interval);
>   curr_value->it_value
>     = valid_timeval32_to_timeval64 (curr_value_32.it_value);
>   return 0;
> #endif
> }
>
> where KERNEL_OLD_TIMEVAL_IS_TIMEVAL64 needs to be true on 64-bit
> architectures and x32.

This works for me.

The files apply to all architectures (so we don't have the issues
where it doesn't take effect for ARM). It fixes the RV32 issues and it
means I can cleanup the Alpha code.

I have made these changes and am testing now.

Alistair

Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

On Mon, 10 Feb 2020 09:43:20 -0800
Alistair Francis <alistair.francis@wdc.com> wrote:

> These functions are alpha specifc, rename them to be clear.
> 
> Let's also rename the header file from tv32-compat.h to
> alpha-tv32-compat.h. This is to avoid conflicts with the one we will
> introduce later.

Reviewed-by: Lukasz Majewski <lukma@denx.de>

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 6/6] linux: Use long time_t for wait4/getrusage

[Alistair Francis]

The Linux kernel expects rusage to use a 32-bit time_t, even on archs
with a 64-bit time_t (like RV32). To address this let's convert
rusage to/from 32-bit and 64-bit to ensure the kernel always gets
a 32-bit time_t.

While we are converting these functions let's also convert them to be
the y2038 safe versions. This means there is a *64 function that is
called by a backwards compatible wrapper.
---
 include/sys/resource.h                | 10 +++++
 sysdeps/unix/sysv/linux/getrusage.c   | 53 +++++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/tv32-compat.h | 47 ++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/wait4.c       | 40 ++++++++++++++++++--
 4 files changed, 146 insertions(+), 4 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/getrusage.c

diff --git a/include/sys/resource.h b/include/sys/resource.h
index 9d604dfe3e..8e115b4df8 100644
--- a/include/sys/resource.h
+++ b/include/sys/resource.h
@@ -134,5 +134,15 @@ extern int __getrusage (enum __rusage_who __who, struct rusage *__usage)
 extern int __setrlimit (enum __rlimit_resource __resource,
 			const struct rlimit *__rlimits);
 libc_hidden_proto (__setrlimit);
+
+#if __TIMESIZE == 64
+# define __getrusage64 __getrusage
+# define __wait464 __wait4
+#else
+extern int __getrusage64 (enum __rusage_who who, struct __rusage64 *usage);
+libc_hidden_proto (__getrusage64)
+pid_t __wait464 (pid_t pid, int *stat_loc, int options, struct __rusage64 *usage);
+libc_hidden_proto (__wait464)
+#endif
 #endif
 #endif
diff --git a/sysdeps/unix/sysv/linux/getrusage.c b/sysdeps/unix/sysv/linux/getrusage.c
new file mode 100644
index 0000000000..f8bcd3f9d0
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/getrusage.c
@@ -0,0 +1,53 @@
+/* getrusage -- get the rusage struct.  Linux version.
+   Copyright (C) 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <sys/time.h>
+#include <sys/resource.h>
+#include <sysdep.h>
+#include <tv32-compat.h>
+
+int
+__getrusage64 (enum __rusage_who who, struct __rusage64 *usage)
+{
+  struct __rusage32 usage32;
+  if (INLINE_SYSCALL_CALL (getrusage, who, &usage32) == -1)
+    return -1;
+
+  rusage32_to_rusage64 (&usage32, usage);
+  return 0;
+}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__getrusage64)
+int
+__getrusage (enum __rusage_who who, struct rusage *usage)
+{
+  int ret ;
+  struct __rusage64 usage64;
+
+  ret = __getrusage64 (who, &usage64);
+
+  if (ret != 0)
+    return ret;
+
+  rusage64_to_rusage (&usage64, usage);
+
+  return ret;
+}
+#endif
+weak_alias (__getrusage, getrusage)
diff --git a/sysdeps/unix/sysv/linux/tv32-compat.h b/sysdeps/unix/sysv/linux/tv32-compat.h
index 4eb6f216ea..c2231f042f 100644
--- a/sysdeps/unix/sysv/linux/tv32-compat.h
+++ b/sysdeps/unix/sysv/linux/tv32-compat.h
@@ -24,6 +24,7 @@
 #include <bits/types.h>
 #include <bits/types/time_t.h>
 #include <bits/types/struct_timeval.h>
+#include <sys/resource.h>
 
 /* Structures containing 'struct timeval' with 32-bit time_t.  */
 struct __itimerval32
@@ -32,4 +33,50 @@ struct __itimerval32
   struct __timeval32 it_value;
 };
 
+struct __rusage32
+{
+  struct __timeval32 ru_utime;	/* user time used */
+  struct __timeval32 ru_stime;	/* system time used */
+  long ru_maxrss;		/* maximum resident set size */
+  long ru_ixrss;		/* integral shared memory size */
+  long ru_idrss;		/* integral unshared data size */
+  long ru_isrss;		/* integral unshared stack size */
+  long ru_minflt;		/* page reclaims */
+  long ru_majflt;		/* page faults */
+  long ru_nswap;		/* swaps */
+  long ru_inblock;		/* block input operations */
+  long ru_oublock;		/* block output operations */
+  long ru_msgsnd;		/* messages sent */
+  long ru_msgrcv;		/* messages received */
+  long ru_nsignals;		/* signals received */
+  long ru_nvcsw;		/* voluntary context switches */
+  long ru_nivcsw;		/* involuntary " */
+};
+
+static inline void
+rusage32_to_rusage64 (const struct __rusage32 *restrict r32,
+                    struct __rusage64 *restrict r64)
+{
+  /* Make sure the entire output structure is cleared, including
+     padding and reserved fields.  */
+  memset (r64, 0, sizeof *r64);
+
+  r64->ru_utime    = valid_timeval32_to_timeval64 (r32->ru_utime);
+  r64->ru_stime    = valid_timeval32_to_timeval64 (r32->ru_stime);
+  r64->ru_maxrss   = r32->ru_maxrss;
+  r64->ru_ixrss    = r32->ru_ixrss;
+  r64->ru_idrss    = r32->ru_idrss;
+  r64->ru_isrss    = r32->ru_isrss;
+  r64->ru_minflt   = r32->ru_minflt;
+  r64->ru_majflt   = r32->ru_majflt;
+  r64->ru_nswap    = r32->ru_nswap;
+  r64->ru_inblock  = r32->ru_inblock;
+  r64->ru_oublock  = r32->ru_oublock;
+  r64->ru_msgsnd   = r32->ru_msgsnd;
+  r64->ru_msgrcv   = r32->ru_msgrcv;
+  r64->ru_nsignals = r32->ru_nsignals;
+  r64->ru_nvcsw    = r32->ru_nvcsw;
+  r64->ru_nivcsw   = r32->ru_nivcsw;
+}
+
 #endif /* tv32-compat.h */
diff --git a/sysdeps/unix/sysv/linux/wait4.c b/sysdeps/unix/sysv/linux/wait4.c
index 3a8bed1169..73f1c24269 100644
--- a/sysdeps/unix/sysv/linux/wait4.c
+++ b/sysdeps/unix/sysv/linux/wait4.c
@@ -19,12 +19,22 @@
 #include <sys/wait.h>
 #include <sys/resource.h>
 #include <sysdep-cancel.h>
+#include <tv32-compat.h>
 
 pid_t
-__wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
+__wait464 (pid_t pid, int *stat_loc, int options, struct __rusage64 *usage)
 {
+  struct __rusage32 usage32;
 #ifdef __NR_wait4
-  return SYSCALL_CANCEL (wait4, pid, stat_loc, options, usage);
+  pid_t ret = SYSCALL_CANCEL (wait4, pid, stat_loc, options, &usage32);
+
+  if (ret != 0)
+    return ret;
+
+  if (usage != NULL)
+      rusage32_to_rusage64 (&usage32, usage);
+
+  return ret;
 #elif defined (__ASSUME_WAITID_PID0_P_PGID)
   idtype_t idtype = P_PID;
 
@@ -41,7 +51,7 @@ __wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
   options |= WEXITED;
 
   siginfo_t infop;
-  if (SYSCALL_CANCEL (waitid, idtype, pid, &infop, options, usage) < 0)
+  if (SYSCALL_CANCEL (waitid, idtype, pid, &infop, options, &usage32) < 0)
     return -1;
 
   if (stat_loc)
@@ -70,8 +80,11 @@ __wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
         }
     }
 
+  if (usage != NULL)
+      rusage32_to_rusage64 (&usage32, usage);
+
   return infop.si_pid;
-# else
+#else
 /* Linux waitid prior kernel 5.4 does not support waiting for the current
    process.  It is possible to emulate wait4 it by calling getpgid for
    PID 0, however, it would require an additional syscall and it is inherent
@@ -81,5 +94,24 @@ __wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
 # error "The kernel ABI does not provide a way to implement wait4"
 #endif
 }
+libc_hidden_def (__wait464)
+
+#if __TIMESIZE != 64
+pid_t
+__wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
+{
+  pid_t ret ;
+  struct __rusage64 usage64;
+
+  ret = __wait464 (pid, stat_loc, options, &usage64);
+
+  if (ret != 0)
+    return ret;
+
+  rusage64_to_rusage (&usage64, usage);
+
+  return ret;
+}
 libc_hidden_def (__wait4);
+#endif
 weak_alias (__wait4, wait4)
-- 
2.25.0

Re: [PATCH v2 6/6] linux: Use long time_t for wait4/getrusage

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 8861 bytes --]

Hi Alistair,

> The Linux kernel expects rusage to use a 32-bit time_t, even on archs
> with a 64-bit time_t (like RV32). To address this let's convert
> rusage to/from 32-bit and 64-bit to ensure the kernel always gets
> a 32-bit time_t.
> 
> While we are converting these functions let's also convert them to be
> the y2038 safe versions. This means there is a *64 function that is
> called by a backwards compatible wrapper.
> ---
>  include/sys/resource.h                | 10 +++++
>  sysdeps/unix/sysv/linux/getrusage.c   | 53
> +++++++++++++++++++++++++++ sysdeps/unix/sysv/linux/tv32-compat.h |
> 47 ++++++++++++++++++++++++ sysdeps/unix/sysv/linux/wait4.c       |
> 40 ++++++++++++++++++-- 4 files changed, 146 insertions(+), 4
> deletions(-) create mode 100644 sysdeps/unix/sysv/linux/getrusage.c
> 
> diff --git a/include/sys/resource.h b/include/sys/resource.h
> index 9d604dfe3e..8e115b4df8 100644
> --- a/include/sys/resource.h
> +++ b/include/sys/resource.h
> @@ -134,5 +134,15 @@ extern int __getrusage (enum __rusage_who __who,
> struct rusage *__usage) extern int __setrlimit (enum
> __rlimit_resource __resource, const struct rlimit *__rlimits);
>  libc_hidden_proto (__setrlimit);
> +
> +#if __TIMESIZE == 64
> +# define __getrusage64 __getrusage
> +# define __wait464 __wait4
> +#else
> +extern int __getrusage64 (enum __rusage_who who, struct __rusage64
> *usage); +libc_hidden_proto (__getrusage64)
> +pid_t __wait464 (pid_t pid, int *stat_loc, int options, struct
> __rusage64 *usage); +libc_hidden_proto (__wait464)
> +#endif
>  #endif
>  #endif
> diff --git a/sysdeps/unix/sysv/linux/getrusage.c
> b/sysdeps/unix/sysv/linux/getrusage.c new file mode 100644
> index 0000000000..f8bcd3f9d0
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/getrusage.c
> @@ -0,0 +1,53 @@
> +/* getrusage -- get the rusage struct.  Linux version.
> +   Copyright (C) 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/time.h>
> +#include <sys/resource.h>
> +#include <sysdep.h>
> +#include <tv32-compat.h>
> +
> +int
> +__getrusage64 (enum __rusage_who who, struct __rusage64 *usage)
> +{
> +  struct __rusage32 usage32;
> +  if (INLINE_SYSCALL_CALL (getrusage, who, &usage32) == -1)
> +    return -1;
> +
> +  rusage32_to_rusage64 (&usage32, usage);
> +  return 0;
> +}
> +
> +#if __TIMESIZE != 64
> +libc_hidden_def (__getrusage64)
> +int
> +__getrusage (enum __rusage_who who, struct rusage *usage)
> +{
> +  int ret ;
> +  struct __rusage64 usage64;
> +
> +  ret = __getrusage64 (who, &usage64);
> +
> +  if (ret != 0)
> +    return ret;
> +
> +  rusage64_to_rusage (&usage64, usage);
> +
> +  return ret;
> +}
> +#endif
> +weak_alias (__getrusage, getrusage)
> diff --git a/sysdeps/unix/sysv/linux/tv32-compat.h
> b/sysdeps/unix/sysv/linux/tv32-compat.h index 4eb6f216ea..c2231f042f
> 100644 --- a/sysdeps/unix/sysv/linux/tv32-compat.h
> +++ b/sysdeps/unix/sysv/linux/tv32-compat.h
> @@ -24,6 +24,7 @@
>  #include <bits/types.h>
>  #include <bits/types/time_t.h>
>  #include <bits/types/struct_timeval.h>
> +#include <sys/resource.h>
>  
>  /* Structures containing 'struct timeval' with 32-bit time_t.  */
>  struct __itimerval32
> @@ -32,4 +33,50 @@ struct __itimerval32
>    struct __timeval32 it_value;
>  };
>  
> +struct __rusage32
> +{
> +  struct __timeval32 ru_utime;	/* user time used */
> +  struct __timeval32 ru_stime;	/* system time used */
> +  long ru_maxrss;		/* maximum resident set size */
> +  long ru_ixrss;		/* integral shared memory size */
> +  long ru_idrss;		/* integral unshared data size */
> +  long ru_isrss;		/* integral unshared stack size */
> +  long ru_minflt;		/* page reclaims */
> +  long ru_majflt;		/* page faults */
> +  long ru_nswap;		/* swaps */
> +  long ru_inblock;		/* block input operations */
> +  long ru_oublock;		/* block output operations */
> +  long ru_msgsnd;		/* messages sent */
> +  long ru_msgrcv;		/* messages received */
> +  long ru_nsignals;		/* signals received */
> +  long ru_nvcsw;		/* voluntary context switches */
> +  long ru_nivcsw;		/* involuntary " */
> +};
> +
> +static inline void
> +rusage32_to_rusage64 (const struct __rusage32 *restrict r32,
> +                    struct __rusage64 *restrict r64)
> +{
> +  /* Make sure the entire output structure is cleared, including
> +     padding and reserved fields.  */
> +  memset (r64, 0, sizeof *r64);
> +
> +  r64->ru_utime    = valid_timeval32_to_timeval64 (r32->ru_utime);
> +  r64->ru_stime    = valid_timeval32_to_timeval64 (r32->ru_stime);
> +  r64->ru_maxrss   = r32->ru_maxrss;
> +  r64->ru_ixrss    = r32->ru_ixrss;
> +  r64->ru_idrss    = r32->ru_idrss;
> +  r64->ru_isrss    = r32->ru_isrss;
> +  r64->ru_minflt   = r32->ru_minflt;
> +  r64->ru_majflt   = r32->ru_majflt;
> +  r64->ru_nswap    = r32->ru_nswap;
> +  r64->ru_inblock  = r32->ru_inblock;
> +  r64->ru_oublock  = r32->ru_oublock;
> +  r64->ru_msgsnd   = r32->ru_msgsnd;
> +  r64->ru_msgrcv   = r32->ru_msgrcv;
> +  r64->ru_nsignals = r32->ru_nsignals;
> +  r64->ru_nvcsw    = r32->ru_nvcsw;
> +  r64->ru_nivcsw   = r32->ru_nivcsw;
> +}
> +
>  #endif /* tv32-compat.h */
> diff --git a/sysdeps/unix/sysv/linux/wait4.c
> b/sysdeps/unix/sysv/linux/wait4.c index 3a8bed1169..73f1c24269 100644
> --- a/sysdeps/unix/sysv/linux/wait4.c
> +++ b/sysdeps/unix/sysv/linux/wait4.c
> @@ -19,12 +19,22 @@
>  #include <sys/wait.h>
>  #include <sys/resource.h>
>  #include <sysdep-cancel.h>
> +#include <tv32-compat.h>
>  
>  pid_t
> -__wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
> +__wait464 (pid_t pid, int *stat_loc, int options, struct __rusage64

Just some consideration regarding naming convention - maybe it would be
more readable to have __wait4_time64 ? Frankly speaking - the __wait464
seems a bit confusing for me.

> *usage) {
> +  struct __rusage32 usage32;
>  #ifdef __NR_wait4
> -  return SYSCALL_CANCEL (wait4, pid, stat_loc, options, usage);
> +  pid_t ret = SYSCALL_CANCEL (wait4, pid, stat_loc, options,
> &usage32); +
> +  if (ret != 0)
> +    return ret;
> +
> +  if (usage != NULL)
> +      rusage32_to_rusage64 (&usage32, usage);
> +
> +  return ret;
>  #elif defined (__ASSUME_WAITID_PID0_P_PGID)
>    idtype_t idtype = P_PID;
>  
> @@ -41,7 +51,7 @@ __wait4 (pid_t pid, int *stat_loc, int options,
> struct rusage *usage) options |= WEXITED;
>  
>    siginfo_t infop;
> -  if (SYSCALL_CANCEL (waitid, idtype, pid, &infop, options, usage) <
> 0)
> +  if (SYSCALL_CANCEL (waitid, idtype, pid, &infop, options,
> &usage32) < 0) return -1;
>  
>    if (stat_loc)
> @@ -70,8 +80,11 @@ __wait4 (pid_t pid, int *stat_loc, int options,
> struct rusage *usage) }
>      }
>  
> +  if (usage != NULL)
> +      rusage32_to_rusage64 (&usage32, usage);
> +
>    return infop.si_pid;
> -# else
> +#else
>  /* Linux waitid prior kernel 5.4 does not support waiting for the
> current process.  It is possible to emulate wait4 it by calling
> getpgid for PID 0, however, it would require an additional syscall
> and it is inherent @@ -81,5 +94,24 @@ __wait4 (pid_t pid, int
> *stat_loc, int options, struct rusage *usage) # error "The kernel ABI
> does not provide a way to implement wait4" #endif
>  }
> +libc_hidden_def (__wait464)
> +
> +#if __TIMESIZE != 64
> +pid_t
> +__wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
> +{
> +  pid_t ret ;
> +  struct __rusage64 usage64;
> +
> +  ret = __wait464 (pid, stat_loc, options, &usage64);
> +
> +  if (ret != 0)
> +    return ret;
> +
> +  rusage64_to_rusage (&usage64, usage);
> +
> +  return ret;
> +}
>  libc_hidden_def (__wait4);
> +#endif
>  weak_alias (__wait4, wait4)




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 6/6] linux: Use long time_t for wait4/getrusage

[Alistair Francis]

On Tue, Feb 11, 2020 at 3:28 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Alistair,
>
> > The Linux kernel expects rusage to use a 32-bit time_t, even on archs
> > with a 64-bit time_t (like RV32). To address this let's convert
> > rusage to/from 32-bit and 64-bit to ensure the kernel always gets
> > a 32-bit time_t.
> >
> > While we are converting these functions let's also convert them to be
> > the y2038 safe versions. This means there is a *64 function that is
> > called by a backwards compatible wrapper.
> > ---
> >  include/sys/resource.h                | 10 +++++
> >  sysdeps/unix/sysv/linux/getrusage.c   | 53
> > +++++++++++++++++++++++++++ sysdeps/unix/sysv/linux/tv32-compat.h |
> > 47 ++++++++++++++++++++++++ sysdeps/unix/sysv/linux/wait4.c       |
> > 40 ++++++++++++++++++-- 4 files changed, 146 insertions(+), 4
> > deletions(-) create mode 100644 sysdeps/unix/sysv/linux/getrusage.c
> >
> > diff --git a/include/sys/resource.h b/include/sys/resource.h
> > index 9d604dfe3e..8e115b4df8 100644
> > --- a/include/sys/resource.h
> > +++ b/include/sys/resource.h
> > @@ -134,5 +134,15 @@ extern int __getrusage (enum __rusage_who __who,
> > struct rusage *__usage) extern int __setrlimit (enum
> > __rlimit_resource __resource, const struct rlimit *__rlimits);
> >  libc_hidden_proto (__setrlimit);
> > +
> > +#if __TIMESIZE == 64
> > +# define __getrusage64 __getrusage
> > +# define __wait464 __wait4
> > +#else
> > +extern int __getrusage64 (enum __rusage_who who, struct __rusage64
> > *usage); +libc_hidden_proto (__getrusage64)
> > +pid_t __wait464 (pid_t pid, int *stat_loc, int options, struct
> > __rusage64 *usage); +libc_hidden_proto (__wait464)
> > +#endif
> >  #endif
> >  #endif
> > diff --git a/sysdeps/unix/sysv/linux/getrusage.c
> > b/sysdeps/unix/sysv/linux/getrusage.c new file mode 100644
> > index 0000000000..f8bcd3f9d0
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/getrusage.c
> > @@ -0,0 +1,53 @@
> > +/* getrusage -- get the rusage struct.  Linux version.
> > +   Copyright (C) 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
> > +   <http://www.gnu.org/licenses/>.  */
> > +
> > +#include <sys/time.h>
> > +#include <sys/resource.h>
> > +#include <sysdep.h>
> > +#include <tv32-compat.h>
> > +
> > +int
> > +__getrusage64 (enum __rusage_who who, struct __rusage64 *usage)
> > +{
> > +  struct __rusage32 usage32;
> > +  if (INLINE_SYSCALL_CALL (getrusage, who, &usage32) == -1)
> > +    return -1;
> > +
> > +  rusage32_to_rusage64 (&usage32, usage);
> > +  return 0;
> > +}
> > +
> > +#if __TIMESIZE != 64
> > +libc_hidden_def (__getrusage64)
> > +int
> > +__getrusage (enum __rusage_who who, struct rusage *usage)
> > +{
> > +  int ret ;
> > +  struct __rusage64 usage64;
> > +
> > +  ret = __getrusage64 (who, &usage64);
> > +
> > +  if (ret != 0)
> > +    return ret;
> > +
> > +  rusage64_to_rusage (&usage64, usage);
> > +
> > +  return ret;
> > +}
> > +#endif
> > +weak_alias (__getrusage, getrusage)
> > diff --git a/sysdeps/unix/sysv/linux/tv32-compat.h
> > b/sysdeps/unix/sysv/linux/tv32-compat.h index 4eb6f216ea..c2231f042f
> > 100644 --- a/sysdeps/unix/sysv/linux/tv32-compat.h
> > +++ b/sysdeps/unix/sysv/linux/tv32-compat.h
> > @@ -24,6 +24,7 @@
> >  #include <bits/types.h>
> >  #include <bits/types/time_t.h>
> >  #include <bits/types/struct_timeval.h>
> > +#include <sys/resource.h>
> >
> >  /* Structures containing 'struct timeval' with 32-bit time_t.  */
> >  struct __itimerval32
> > @@ -32,4 +33,50 @@ struct __itimerval32
> >    struct __timeval32 it_value;
> >  };
> >
> > +struct __rusage32
> > +{
> > +  struct __timeval32 ru_utime;       /* user time used */
> > +  struct __timeval32 ru_stime;       /* system time used */
> > +  long ru_maxrss;            /* maximum resident set size */
> > +  long ru_ixrss;             /* integral shared memory size */
> > +  long ru_idrss;             /* integral unshared data size */
> > +  long ru_isrss;             /* integral unshared stack size */
> > +  long ru_minflt;            /* page reclaims */
> > +  long ru_majflt;            /* page faults */
> > +  long ru_nswap;             /* swaps */
> > +  long ru_inblock;           /* block input operations */
> > +  long ru_oublock;           /* block output operations */
> > +  long ru_msgsnd;            /* messages sent */
> > +  long ru_msgrcv;            /* messages received */
> > +  long ru_nsignals;          /* signals received */
> > +  long ru_nvcsw;             /* voluntary context switches */
> > +  long ru_nivcsw;            /* involuntary " */
> > +};
> > +
> > +static inline void
> > +rusage32_to_rusage64 (const struct __rusage32 *restrict r32,
> > +                    struct __rusage64 *restrict r64)
> > +{
> > +  /* Make sure the entire output structure is cleared, including
> > +     padding and reserved fields.  */
> > +  memset (r64, 0, sizeof *r64);
> > +
> > +  r64->ru_utime    = valid_timeval32_to_timeval64 (r32->ru_utime);
> > +  r64->ru_stime    = valid_timeval32_to_timeval64 (r32->ru_stime);
> > +  r64->ru_maxrss   = r32->ru_maxrss;
> > +  r64->ru_ixrss    = r32->ru_ixrss;
> > +  r64->ru_idrss    = r32->ru_idrss;
> > +  r64->ru_isrss    = r32->ru_isrss;
> > +  r64->ru_minflt   = r32->ru_minflt;
> > +  r64->ru_majflt   = r32->ru_majflt;
> > +  r64->ru_nswap    = r32->ru_nswap;
> > +  r64->ru_inblock  = r32->ru_inblock;
> > +  r64->ru_oublock  = r32->ru_oublock;
> > +  r64->ru_msgsnd   = r32->ru_msgsnd;
> > +  r64->ru_msgrcv   = r32->ru_msgrcv;
> > +  r64->ru_nsignals = r32->ru_nsignals;
> > +  r64->ru_nvcsw    = r32->ru_nvcsw;
> > +  r64->ru_nivcsw   = r32->ru_nivcsw;
> > +}
> > +
> >  #endif /* tv32-compat.h */
> > diff --git a/sysdeps/unix/sysv/linux/wait4.c
> > b/sysdeps/unix/sysv/linux/wait4.c index 3a8bed1169..73f1c24269 100644
> > --- a/sysdeps/unix/sysv/linux/wait4.c
> > +++ b/sysdeps/unix/sysv/linux/wait4.c
> > @@ -19,12 +19,22 @@
> >  #include <sys/wait.h>
> >  #include <sys/resource.h>
> >  #include <sysdep-cancel.h>
> > +#include <tv32-compat.h>
> >
> >  pid_t
> > -__wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
> > +__wait464 (pid_t pid, int *stat_loc, int options, struct __rusage64
>
> Just some consideration regarding naming convention - maybe it would be
> more readable to have __wait4_time64 ? Frankly speaking - the __wait464
> seems a bit confusing for me.

Yep, good idea. Fixed in v3.

Alistair

>
> > *usage) {
> > +  struct __rusage32 usage32;
> >  #ifdef __NR_wait4
> > -  return SYSCALL_CANCEL (wait4, pid, stat_loc, options, usage);
> > +  pid_t ret = SYSCALL_CANCEL (wait4, pid, stat_loc, options,
> > &usage32); +
> > +  if (ret != 0)
> > +    return ret;
> > +
> > +  if (usage != NULL)
> > +      rusage32_to_rusage64 (&usage32, usage);
> > +
> > +  return ret;
> >  #elif defined (__ASSUME_WAITID_PID0_P_PGID)
> >    idtype_t idtype = P_PID;
> >
> > @@ -41,7 +51,7 @@ __wait4 (pid_t pid, int *stat_loc, int options,
> > struct rusage *usage) options |= WEXITED;
> >
> >    siginfo_t infop;
> > -  if (SYSCALL_CANCEL (waitid, idtype, pid, &infop, options, usage) <
> > 0)
> > +  if (SYSCALL_CANCEL (waitid, idtype, pid, &infop, options,
> > &usage32) < 0) return -1;
> >
> >    if (stat_loc)
> > @@ -70,8 +80,11 @@ __wait4 (pid_t pid, int *stat_loc, int options,
> > struct rusage *usage) }
> >      }
> >
> > +  if (usage != NULL)
> > +      rusage32_to_rusage64 (&usage32, usage);
> > +
> >    return infop.si_pid;
> > -# else
> > +#else
> >  /* Linux waitid prior kernel 5.4 does not support waiting for the
> > current process.  It is possible to emulate wait4 it by calling
> > getpgid for PID 0, however, it would require an additional syscall
> > and it is inherent @@ -81,5 +94,24 @@ __wait4 (pid_t pid, int
> > *stat_loc, int options, struct rusage *usage) # error "The kernel ABI
> > does not provide a way to implement wait4" #endif
> >  }
> > +libc_hidden_def (__wait464)
> > +
> > +#if __TIMESIZE != 64
> > +pid_t
> > +__wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
> > +{
> > +  pid_t ret ;
> > +  struct __rusage64 usage64;
> > +
> > +  ret = __wait464 (pid, stat_loc, options, &usage64);
> > +
> > +  if (ret != 0)
> > +    return ret;
> > +
> > +  rusage64_to_rusage (&usage64, usage);
> > +
> > +  return ret;
> > +}
> >  libc_hidden_def (__wait4);
> > +#endif
> >  weak_alias (__wait4, wait4)
>
>
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[PATCH v2 5/6] resource: Add a __rusage64 struct

[Alistair Francis]

Add a __rusage64 struct which always uses a 64-bit time_t.

Reviewed-by: Lukasz Majewski <lukma@denx.de>
---
 include/sys/resource.h | 110 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/include/sys/resource.h b/include/sys/resource.h
index c55d4e63bd..9d604dfe3e 100644
--- a/include/sys/resource.h
+++ b/include/sys/resource.h
@@ -2,6 +2,116 @@
 #include <resource/sys/resource.h>
 
 #ifndef _ISOMAC
+# include <time.h>
+# include <string.h>
+
+/* Internal version of rusage with a 64-bit time_t. */
+#if __TIMESIZE == 64
+# define __rusage64 rusage
+#else
+struct __rusage64
+  {
+    struct __timeval64 ru_utime;
+    struct __timeval64 ru_stime;
+    __extension__ union
+      {
+	long int ru_maxrss;
+	__syscall_slong_t __ru_maxrss_word;
+      };
+    __extension__ union
+      {
+	long int ru_ixrss;
+	__syscall_slong_t __ru_ixrss_word;
+      };
+    __extension__ union
+      {
+	long int ru_idrss;
+	__syscall_slong_t __ru_idrss_word;
+      };
+    __extension__ union
+      {
+	long int ru_isrss;
+	 __syscall_slong_t __ru_isrss_word;
+      };
+    __extension__ union
+      {
+	long int ru_minflt;
+	__syscall_slong_t __ru_minflt_word;
+      };
+    __extension__ union
+      {
+	long int ru_majflt;
+	__syscall_slong_t __ru_majflt_word;
+      };
+    __extension__ union
+      {
+	long int ru_nswap;
+	__syscall_slong_t __ru_nswap_word;
+      };
+    __extension__ union
+      {
+	long int ru_inblock;
+	__syscall_slong_t __ru_inblock_word;
+      };
+    __extension__ union
+      {
+	long int ru_oublock;
+	__syscall_slong_t __ru_oublock_word;
+      };
+    __extension__ union
+      {
+	long int ru_msgsnd;
+	__syscall_slong_t __ru_msgsnd_word;
+      };
+    __extension__ union
+      {
+	long int ru_msgrcv;
+	__syscall_slong_t __ru_msgrcv_word;
+      };
+    __extension__ union
+      {
+	long int ru_nsignals;
+	__syscall_slong_t __ru_nsignals_word;
+      };
+    __extension__ union
+      {
+	long int ru_nvcsw;
+	__syscall_slong_t __ru_nvcsw_word;
+      };
+    __extension__ union
+      {
+	long int ru_nivcsw;
+	__syscall_slong_t __ru_nivcsw_word;
+      };
+  };
+#endif
+
+static inline void
+rusage64_to_rusage (const struct __rusage64 *restrict r64,
+                    struct rusage *restrict r)
+{
+  /* Make sure the entire output structure is cleared, including
+     padding and reserved fields.  */
+  memset (r, 0, sizeof *r);
+
+  r->ru_utime    = valid_timeval64_to_timeval (r64->ru_utime);
+  r->ru_stime    = valid_timeval64_to_timeval (r64->ru_stime);
+  r->ru_maxrss   = r64->ru_maxrss;
+  r->ru_ixrss    = r64->ru_ixrss;
+  r->ru_idrss    = r64->ru_idrss;
+  r->ru_isrss    = r64->ru_isrss;
+  r->ru_minflt   = r64->ru_minflt;
+  r->ru_majflt   = r64->ru_majflt;
+  r->ru_nswap    = r64->ru_nswap;
+  r->ru_inblock  = r64->ru_inblock;
+  r->ru_oublock  = r64->ru_oublock;
+  r->ru_msgsnd   = r64->ru_msgsnd;
+  r->ru_msgrcv   = r64->ru_msgrcv;
+  r->ru_nsignals = r64->ru_nsignals;
+  r->ru_nvcsw    = r64->ru_nvcsw;
+  r->ru_nivcsw   = r64->ru_nivcsw;
+}
+
 /* Prototypes repeated instead of using __typeof because
    sys/resource.h is included in C++ tests, and declaring functions
    with __typeof and __THROW doesn't work for C++.  */
-- 
2.25.0

Re: [PATCH v2 0/6] Always use long time_t for certain syscalls

[Alistair Francis]

On Mon, Feb 10, 2020 at 9:50 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> On y2038 safe 32-bit systems the Linux kernel expects itimerval
> and rusage to use a 32-bit time_t, even though the other time_t's
> are 64-bit.
>
> This series converts getitimer, setitimer, getrusage and wait4 to be
> both y2038 safe and always pass a long time_t. On 32 and 64-bit systems
> we will pass a long time to the kernel (no matter the time_t size). This is
> no change for 64-bit architectures or 32-bit architectures with a 32-bit time_t.
>
> This follows the standard y2038 conversion so that we don't break
> backwards compatibility but we expose a 64-bit version for y2038 safe
> architectrures (like RV32).

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 10 test failures.

>
> Alistair Francis (6):
>   sysv/linux: Rename alpha functions to be alpha specific
>   time: Add a timeval with a long tv_sec and tv_usec
>   time: Add a __itimerval64 struct
>   linux: Use long time_t __getitimer/__setitimer
>   resource: Add a __rusage64 struct
>   linux: Use long time_t for wait4/getrusage
>
>  include/sys/resource.h                        | 120 ++++++++++++++++++
>  include/time.h                                |  64 ++++++++++
>  .../{tv32-compat.h => alpha-tv32-compat.h}    |  16 +--
>  sysdeps/unix/sysv/linux/alpha/osf_adjtime.c   |  10 +-
>  sysdeps/unix/sysv/linux/alpha/osf_getitimer.c |   6 +-
>  sysdeps/unix/sysv/linux/alpha/osf_getrusage.c |   4 +-
>  .../unix/sysv/linux/alpha/osf_gettimeofday.c  |   4 +-
>  sysdeps/unix/sysv/linux/alpha/osf_setitimer.c |  10 +-
>  .../unix/sysv/linux/alpha/osf_settimeofday.c  |   4 +-
>  sysdeps/unix/sysv/linux/alpha/osf_utimes.c    |   6 +-
>  sysdeps/unix/sysv/linux/alpha/osf_wait4.c     |   4 +-
>  sysdeps/unix/sysv/linux/getitimer.c           |  54 ++++++++
>  sysdeps/unix/sysv/linux/getrusage.c           |  53 ++++++++
>  sysdeps/unix/sysv/linux/setitimer.c           |  89 +++++++++++++
>  sysdeps/unix/sysv/linux/tv32-compat.h         |  82 ++++++++++++
>  sysdeps/unix/sysv/linux/wait4.c               |  40 +++++-
>  16 files changed, 530 insertions(+), 36 deletions(-)
>  rename sysdeps/unix/sysv/linux/alpha/{tv32-compat.h => alpha-tv32-compat.h} (88%)
>  create mode 100644 sysdeps/unix/sysv/linux/getitimer.c
>  create mode 100644 sysdeps/unix/sysv/linux/getrusage.c
>  create mode 100644 sysdeps/unix/sysv/linux/setitimer.c
>  create mode 100644 sysdeps/unix/sysv/linux/tv32-compat.h
>
> --
> 2.25.0
>