[PATCH 4/6] linux: Use 32-bit time_t for itimerval

[PATCH 4/6] linux: Use 32-bit time_t for itimerval

[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.

This means that all 32-bit architectures with a 64-bit time_t will be
able to use this generic implementation.

This code is based on similar code in alpha, but adjusted to pass the
32-bit time_t to the kernel.

We can't directly call the __getitimer/__setitimer functions as they
expect a struct itimerval but we need to pass in a struct itimerval32.
---
 include/time.h                                | 12 +++
 .../linux/generic/wordsize-32/getitimer.c     | 54 +++++++++++++
 .../linux/generic/wordsize-32/setitimer.c     | 77 +++++++++++++++++++
 .../linux/generic/wordsize-32/tv32-compat.h   | 35 +++++++++
 4 files changed, 178 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h

diff --git a/include/time.h b/include/time.h
index 898ff0fb2d..e1d80b4190 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>
 
@@ -118,6 +119,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/generic/wordsize-32/getitimer.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c
new file mode 100644
index 0000000000..28a3e31126
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/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/generic/wordsize-32/setitimer.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
new file mode 100644
index 0000000000..fabc7f2c0c
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
@@ -0,0 +1,77 @@
+/* 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;
+  new_value_32.it_interval
+    = valid_timeval64_to_timeval32 (new_value->it_interval);
+  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/generic/wordsize-32/tv32-compat.h b/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
new file mode 100644
index 0000000000..4eb6f216ea
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/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

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

[Alistair Francis]

Add a __itimerval64 which always uses a 64-bit time_t.
---
 include/time.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/time.h b/include/time.h
index c2c05bb671..898ff0fb2d 100644
--- a/include/time.h
+++ b/include/time.h
@@ -107,6 +107,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 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 timeval_long struct to be used internally.
---
 include/time.h | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/include/time.h b/include/time.h
index d425c69ede..c2c05bb671 100644
--- a/include/time.h
+++ b/include/time.h
@@ -388,6 +388,49 @@ 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)
+{
+  if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647))
+    return (struct __timeval32) { 2147483647, 999999};
+  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

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

[Alistair Francis]

Add a __rusage64 struct which always uses a 64-bit time_t.
---
 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

[PATCH 0/6] Always use 32-bit 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 adds 32-bit overrides for the specific syscalls that converts
between the Linux kernel's expected 32-bit time_t to the user expected 64-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 applies ontop of: https://patchwork.ozlabs.org/patch/1230878/

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 32-bit time_t for itimerval
  resource: Add a __rusage64 struct
  linux: Use 32-bit time_t for rusage

 include/sys/resource.h                        | 116 ++++++++++++++++++
 include/time.h                                |  66 ++++++++++
 sysdeps/unix/sysv/linux/alpha/osf_adjtime.c   |   8 +-
 sysdeps/unix/sysv/linux/alpha/osf_getitimer.c |   4 +-
 sysdeps/unix/sysv/linux/alpha/osf_getrusage.c |   2 +-
 .../unix/sysv/linux/alpha/osf_gettimeofday.c  |   2 +-
 sysdeps/unix/sysv/linux/alpha/osf_setitimer.c |   8 +-
 .../unix/sysv/linux/alpha/osf_settimeofday.c  |   2 +-
 sysdeps/unix/sysv/linux/alpha/osf_utimes.c    |   4 +-
 sysdeps/unix/sysv/linux/alpha/osf_wait4.c     |   2 +-
 sysdeps/unix/sysv/linux/alpha/tv32-compat.h   |  14 +--
 .../linux/generic/wordsize-32/getitimer.c     |  54 ++++++++
 .../linux/generic/wordsize-32/getrusage.c     |  52 ++++++++
 .../linux/generic/wordsize-32/setitimer.c     |  77 ++++++++++++
 .../linux/generic/wordsize-32/tv32-compat.h   |  82 +++++++++++++
 .../sysv/linux/generic/wordsize-32/wait4.c    |  83 +++++++++++++
 16 files changed, 553 insertions(+), 23 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/wait4.c

-- 
2.25.0

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

[Alistair Francis]

These functions are alpha specifc, rename them to be clear.
---
 sysdeps/unix/sysv/linux/alpha/osf_adjtime.c      |  8 ++++----
 sysdeps/unix/sysv/linux/alpha/osf_getitimer.c    |  4 ++--
 sysdeps/unix/sysv/linux/alpha/osf_getrusage.c    |  2 +-
 sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c |  2 +-
 sysdeps/unix/sysv/linux/alpha/osf_setitimer.c    |  8 ++++----
 sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c |  2 +-
 sysdeps/unix/sysv/linux/alpha/osf_utimes.c       |  4 ++--
 sysdeps/unix/sysv/linux/alpha/osf_wait4.c        |  2 +-
 sysdeps/unix/sysv/linux/alpha/tv32-compat.h      | 14 +++++++-------
 9 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
index 9825a4734d..bd8fe5f92d 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
@@ -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..3c1bfdca38 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c
@@ -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..571bf3559a 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_getrusage.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_getrusage.c
@@ -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..9c800e1a0d 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
@@ -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..0e70a6ebda 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
@@ -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..092a6e6f7b 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c
@@ -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..006427c7fa 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
@@ -28,8 +28,8 @@ 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..c773be0d3a 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_wait4.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_wait4.c
@@ -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;
 }
 
diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
index 8e34ed1c1b..dbc020ddf8 100644
--- a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
+++ b/sysdeps/unix/sysv/linux/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;
-- 
2.25.0

[PATCH 6/6] linux: Use 32-bit time_t for rusage

[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.

This means that all 32-bit architectures with a 64-bit time_t will be
able to use this generic implementation.

This code is based on similar code in alpha, but adjusted to pass the
32-bit time_t to the kernel.

We can't directly call __wait4 as it expects a struct rusage but we have
to pass in and use a struct rusage32. The same appies to __getrusage.
---
 include/sys/resource.h                        |  6 ++
 .../linux/generic/wordsize-32/getrusage.c     | 52 ++++++++++++
 .../linux/generic/wordsize-32/tv32-compat.h   | 47 +++++++++++
 .../sysv/linux/generic/wordsize-32/wait4.c    | 83 +++++++++++++++++++
 4 files changed, 188 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c
 create mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/wait4.c

diff --git a/include/sys/resource.h b/include/sys/resource.h
index 9d604dfe3e..fcba56303a 100644
--- a/include/sys/resource.h
+++ b/include/sys/resource.h
@@ -134,5 +134,11 @@ 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
+#else
+extern int __getrusage64 (enum __rusage_who who, struct __rusage64 *usage);
+#endif
 #endif
 #endif
diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c
new file mode 100644
index 0000000000..a495cc6c8b
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c
@@ -0,0 +1,52 @@
+/* getrusage -- get the rusage struct.  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 <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
+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/generic/wordsize-32/tv32-compat.h b/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
index 4eb6f216ea..c2231f042f 100644
--- a/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/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/generic/wordsize-32/wait4.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/wait4.c
new file mode 100644
index 0000000000..ae42b2ecbb
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/wait4.c
@@ -0,0 +1,83 @@
+/* wait4 -- wait for process to change state.  Linux/RV32/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 <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)
+{
+  struct __rusage32 usage32;
+  idtype_t idtype = P_PID;
+
+  if (pid < -1)
+    {
+      idtype = P_PGID;
+      pid *= -1;
+    }
+  else if (pid == -1)
+    idtype = P_ALL;
+  else if (pid == 0)
+    idtype = P_PGID;
+
+  options |= WEXITED;
+
+  siginfo_t infop;
+  if (SYSCALL_CANCEL (waitid, idtype, pid, &infop, options, &usage32) < 0)
+    return -1;
+
+  if (stat_loc)
+    {
+      switch (infop.si_code)
+        {
+        case CLD_EXITED:
+          *stat_loc = W_EXITCODE (infop.si_status, 0);
+          break;
+        case CLD_DUMPED:
+          *stat_loc = WCOREFLAG | infop.si_status;
+    break;
+        case CLD_KILLED:
+          *stat_loc = infop.si_status;
+          break;
+        case CLD_TRAPPED:
+        case CLD_STOPPED:
+          *stat_loc = W_STOPCODE (infop.si_status);
+          break;
+        case CLD_CONTINUED:
+          *stat_loc = __W_CONTINUED;
+          break;
+  default:
+    *stat_loc = 0;
+    break;
+        }
+    }
+
+  if (usage != NULL)
+    {
+      struct __rusage64 usage64;
+      rusage32_to_rusage64 (&usage32, &usage64);
+      rusage64_to_rusage (&usage64, usage);
+    }
+
+  return infop.si_pid;
+}
+
+libc_hidden_def (__wait4);
+weak_alias (__wait4, wait4)
-- 
2.25.0

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

[Lukasz Majewski]

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

Hi Alistair,

> These functions are alpha specifc, rename them to be clear.
> ---
>  sysdeps/unix/sysv/linux/alpha/osf_adjtime.c      |  8 ++++----
>  sysdeps/unix/sysv/linux/alpha/osf_getitimer.c    |  4 ++--
>  sysdeps/unix/sysv/linux/alpha/osf_getrusage.c    |  2 +-
>  sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c |  2 +-
>  sysdeps/unix/sysv/linux/alpha/osf_setitimer.c    |  8 ++++----
>  sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c |  2 +-
>  sysdeps/unix/sysv/linux/alpha/osf_utimes.c       |  4 ++--
>  sysdeps/unix/sysv/linux/alpha/osf_wait4.c        |  2 +-
>  sysdeps/unix/sysv/linux/alpha/tv32-compat.h      | 14 +++++++-------
>  9 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c index
> 9825a4734d..bd8fe5f92d 100644 ---
> a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c +++
> b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c @@ -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);

In the aforementioned patch set:
https://patchwork.ozlabs.org/patch/1230880/

I was doing my best to avoid name clashes. IMHO adding "alpha_" prefix
would make this even better.

>    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..3c1bfdca38 100644 ---
> a/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c +++
> b/sysdeps/unix/sysv/linux/alpha/osf_getitimer.c @@ -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..571bf3559a 100644 ---
> a/sysdeps/unix/sysv/linux/alpha/osf_getrusage.c +++
> b/sysdeps/unix/sysv/linux/alpha/osf_getrusage.c @@ -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..9c800e1a0d 100644 ---
> a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c +++
> b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c @@ -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..0e70a6ebda 100644 ---
> a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c +++
> b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c @@ -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..092a6e6f7b 100644 ---
> a/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c +++
> b/sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c @@ -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..006427c7fa 100644 ---
> a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c +++
> b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c @@ -28,8 +28,8 @@
> 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..c773be0d3a 100644 ---
> a/sysdeps/unix/sysv/linux/alpha/osf_wait4.c +++
> b/sysdeps/unix/sysv/linux/alpha/osf_wait4.c @@ -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;
>  }
>  
> diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h index
> 8e34ed1c1b..dbc020ddf8 100644 ---
> a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h +++
> b/sysdeps/unix/sysv/linux/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;

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 --]

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

[Lukasz Majewski]

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

Hi Alistair,

> Add a __itimerval64 which always uses a 64-bit time_t.
> ---
>  include/time.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/include/time.h b/include/time.h
> index c2c05bb671..898ff0fb2d 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -107,6 +107,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

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 --]

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

[Lukasz Majewski]

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

Hi Alistair,

> Add a __rusage64 struct which always uses a 64-bit time_t.
> ---
>  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++.  */

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 --]

Re: [PATCH 6/6] linux: Use 32-bit time_t for rusage

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 10997 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.
> 
> This means that all 32-bit architectures with a 64-bit time_t will be
> able to use this generic implementation.
> 
> This code is based on similar code in alpha, but adjusted to pass the
> 32-bit time_t to the kernel.
> 
> We can't directly call __wait4 as it expects a struct rusage but we
> have to pass in and use a struct rusage32. The same appies to
> __getrusage. ---
>  include/sys/resource.h                        |  6 ++
>  .../linux/generic/wordsize-32/getrusage.c     | 52 ++++++++++++

I wanted to test this code with ARM Y2038 test setup (yocto with qemu).

Unfortunately, for ARM32 (armv7) the path:
sysdeps/unix/sysv/linux/generic/wordsize-32/ is not reached at all.

In the build log (with make --debug) I do see following paths:

config_sysdirs='sysdeps/unix/sysv/linux/arm/le
sysdeps/unix/sysv/linux/arm sysdeps/arm/nptl sysdeps/unix/sysv/linux
sysdeps/nptl sysdeps/pthread sysdeps/gnu sysdeps/unix/inet
sysdeps/unix/sysv sysdeps/unix/arm sysdeps/unix sysdeps/posix
sysdeps/arm/le/armv7/multiarch sysdeps/arm/armv7/multiarch
sysdeps/arm/le/armv7 sysdeps/arm/armv7 sysdeps/arm/armv6t2
sysdeps/arm/armv6 sysdeps/arm/le sysdeps/arm sysdeps/wordsize-32
sysdeps/ieee754/flt-32 sysdeps/ieee754/dbl-64 sysdeps/ieee754
sysdeps/generic'

The "wordsize-32" only appears in ./sysdeps/wordsize-32 (and only
sysdeps/generic is defined for searching files to build).

How shall I tune my setup to also make
sysdeps/unix/sysv/linux/generic/wordsize-32/ accessible for glibc build?

Could you share output from yours build setup:

grep config-sysdir config.make


>  .../linux/generic/wordsize-32/tv32-compat.h   | 47 +++++++++++
>  .../sysv/linux/generic/wordsize-32/wait4.c    | 83
> +++++++++++++++++++ 4 files changed, 188 insertions(+)
>  create mode 100644
> sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c create mode
> 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/wait4.c
> 
> diff --git a/include/sys/resource.h b/include/sys/resource.h
> index 9d604dfe3e..fcba56303a 100644
> --- a/include/sys/resource.h
> +++ b/include/sys/resource.h
> @@ -134,5 +134,11 @@ 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
> +#else
> +extern int __getrusage64 (enum __rusage_who who, struct __rusage64

Please add libc_hidden_proto (__getrusage64)

> *usage); +#endif
>  #endif
>  #endif
> diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c
> b/sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c new file
> mode 100644 index 0000000000..a495cc6c8b
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c
> @@ -0,0 +1,52 @@
> +/* getrusage -- get the rusage struct.  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 <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

Please add 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)

Ok.

> diff --git
> a/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> b/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h index
> 4eb6f216ea..c2231f042f 100644 ---
> a/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h +++
> b/sysdeps/unix/sysv/linux/generic/wordsize-32/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/generic/wordsize-32/wait4.c
> b/sysdeps/unix/sysv/linux/generic/wordsize-32/wait4.c new file mode
> 100644 index 0000000000..ae42b2ecbb
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/wait4.c
> @@ -0,0 +1,83 @@
> +/* wait4 -- wait for process to change state.  Linux/RV32/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 <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)

If I may have one "small" remark - it seems like wait4 uses here struct
rusage.

Would it be a big problem if you provide 64 bit conversion for this
function as well?

I mean the conversion code for it:

pid_t
__wait4_time64 (pid_t pid, int *stat_loc, int options, struct __rusage64
*usage)
{

}

#if __TIMESIZE != 64

pid_t
__wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
{

//conversions

ret = __wait4_time64 ()

//conversions

}

#endif

> +{
> +  struct __rusage32 usage32;
> +  idtype_t idtype = P_PID;
> +
> +  if (pid < -1)
> +    {
> +      idtype = P_PGID;
> +      pid *= -1;
> +    }
> +  else if (pid == -1)
> +    idtype = P_ALL;
> +  else if (pid == 0)
> +    idtype = P_PGID;
> +
> +  options |= WEXITED;
> +
> +  siginfo_t infop;
> +  if (SYSCALL_CANCEL (waitid, idtype, pid, &infop, options,
> &usage32) < 0)
> +    return -1;
> +
> +  if (stat_loc)
> +    {
> +      switch (infop.si_code)
> +        {
> +        case CLD_EXITED:
> +          *stat_loc = W_EXITCODE (infop.si_status, 0);
> +          break;
> +        case CLD_DUMPED:
> +          *stat_loc = WCOREFLAG | infop.si_status;
> +    break;
> +        case CLD_KILLED:
> +          *stat_loc = infop.si_status;
> +          break;
> +        case CLD_TRAPPED:
> +        case CLD_STOPPED:
> +          *stat_loc = W_STOPCODE (infop.si_status);
> +          break;
> +        case CLD_CONTINUED:
> +          *stat_loc = __W_CONTINUED;
> +          break;
> +  default:
> +    *stat_loc = 0;
> +    break;
> +        }
> +    }
> +
> +  if (usage != NULL)
> +    {
> +      struct __rusage64 usage64;
> +      rusage32_to_rusage64 (&usage32, &usage64);
> +      rusage64_to_rusage (&usage64, usage);
> +    }
> +
> +  return infop.si_pid;
> +}
> +
> +libc_hidden_def (__wait4);
> +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 2/6] time: Add a timeval with a long tv_sec and tv_usec

[Lukasz Majewski]

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

Hi Alistair,

> 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 timeval_long struct to be used internally.
			   ^^^^^^^^^^ - I think that this shall be
			   updated.

> ---
>  include/time.h | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/include/time.h b/include/time.h
> index d425c69ede..c2c05bb671 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -388,6 +388,49 @@ 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.  */

As __timeval32 will be used in e.g __setitimer64 (which in turn will be
aliased to setitimer on archs with __WORDSIZE==64 && __TIMESIZE==64)
long seems to be the best option as it will be 64 bits for those
machines.

For ones with __WORDSIZE==32 it will be 32 bits instead. Am I correct?

> +  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)
> +{
> +  if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647))
> +    return (struct __timeval32) { 2147483647, 999999};

What is the purpose of this code?

The valid_* prefix shall indicate that the timeval64 will fit the
timeval32 and there is no need for any explicit check.

I would expect usage of this function as presented here:
https://patchwork.ozlabs.org/patch/1230884/

if (! in_time_t_range (tv64.tv_sec))
  {
    __set_errno (EOVERFLOW);
    return -1;
  }

  if (tv)
    *tv = valid_timeval64_to_timeval (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 --]

Re: [PATCH 6/6] linux: Use 32-bit time_t for rusage

[Andreas Schwab]

On Feb 04 2020, Lukasz Majewski wrote:

> Unfortunately, for ARM32 (armv7) the path:
> sysdeps/unix/sysv/linux/generic/wordsize-32/ is not reached at all.

unix/linux/generic/wordsize-32 cannot be used for armv7, since it
doesn't use asm-generic syscalls.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH 4/6] linux: Use 32-bit time_t for itimerval

[Lukasz Majewski]

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

Hi Alistair,

Maybe it would be good to rewrite the subject line to also reflect the
introduction of __[gs]etitimer64 ?

> 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.
> 
> This means that all 32-bit architectures with a 64-bit time_t will be
> able to use this generic implementation.
> 
> This code is based on similar code in alpha, but adjusted to pass the
> 32-bit time_t to the kernel.
> 
> We can't directly call the __getitimer/__setitimer functions as they
> expect a struct itimerval but we need to pass in a struct itimerval32.

Please update this patch series to also include information about
build/run testing.

For example:

Build-tests:
./src/scripts/build-many-glibcs.py glibcs


> ---
>  include/time.h                                | 12 +++
>  .../linux/generic/wordsize-32/getitimer.c     | 54 +++++++++++++
>  .../linux/generic/wordsize-32/setitimer.c     | 77
> +++++++++++++++++++ .../linux/generic/wordsize-32/tv32-compat.h   |
> 35 +++++++++ 4 files changed, 178 insertions(+)
>  create mode 100644
> sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c create mode

The same issue as pointed out in patch 6/6. The path
sysdeps/unix/sysv/linux/generic/wordsize-32 is NOT searched by ARMv7
(ARM32) for files to be built.

I've removed getitimer, getrusage and setitimer from
./sysdeps/unix/syscalls.list, so they are NOT auto generated wrappers
anymore, but this doesn't help.

It seems like for RV32 some extra paths are added...

I've dug to the @sysnames@ variable set in configure in the source of glibc.
I'm wondering how RV32 modifies it (and how it differs from ARM32)?


> 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c create
> mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> 
> diff --git a/include/time.h b/include/time.h
> index 898ff0fb2d..e1d80b4190 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>
>  
> @@ -118,6 +119,17 @@ struct __itimerval64
>  };
>  #endif
>  
> +#if __TIMESIZE == 64
> +# define __getitimer64 __getitimer
> +# define __setitimer64 __setitimer
> +#else
> +extern int __getitimer64 (enum __itimer_which __which,
> +                          struct __itimerval64 *__value);

Please add here:
libc_hidden_proto (__getitimer64)

> +extern int __setitimer64 (enum __itimer_which __which,
> +                          const struct __itimerval64 *__restrict
> __new,
> +                          struct __itimerval64 *__restrict __old);

Please add here:
libc_hidden_proto (__setitimer64)

> +#endif
> +
>  #if __TIMESIZE == 64
>  # define __ctime64 ctime
>  #else
> diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c
> b/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c new file
> mode 100644 index 0000000000..28a3e31126
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/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

Please add here:
libc_hidden_def (__gettimer64)

Rationale:

Before we introduce the Y2038 support (with -D_TIME_BITS=64) the
__gettimer64 shall be hidden (to avoid resolving it via PLT).

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

Ok. As we expect 32 bit values - there is no need for checking (and 64
<-> 32 conversions will be OK).

> +#endif
> +weak_alias (__getitimer, getitimer)
> diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
> b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c new file
> mode 100644 index 0000000000..fabc7f2c0c
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
> @@ -0,0 +1,77 @@
> +/* 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;
> +  new_value_32.it_interval
> +    = valid_timeval64_to_timeval32 (new_value->it_interval);
> +  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); +

The above code is correct of course.

However, I've used another approach for timerfd_settime (IMHO a bit
more concise):
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/timerfd_settime.c;h=164b4e860acb6a365a3890f30e2eac364a518c22;hb=eae22432723b877354291aca4dbbfde5891dad59#l49

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

Please add here:
libc_hidden_def (__settimer64)

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

I think that here we shall also consider the NULL pointer for old_value
(as stated in http://man7.org/linux/man-pages/man2/setitimer.2.html)

ret = __setitimer64 (which, &new64, old_value ? &old64 : NULL);

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

Maybe it would be better to change this code to:

if (ret == 0 && old_value)
  {
    old_value->it_interval
      = valid_timeval64_to_timeval (old64.it_interval);
    old_value->it_value
      = valid_timeval64_to_timeval (old64.it_value);
  }

return ret

Please find the timerfd_settime as a reference:
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/timerfd_settime.c;h=164b4e860acb6a365a3890f30e2eac364a518c22;hb=eae22432723b877354291aca4dbbfde5891dad59#l77

> +  return ret;
> +}
> +#endif
> +weak_alias (__setitimer, setitimer)
> diff --git
> a/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> b/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h new file
> mode 100644 index 0000000000..4eb6f216ea --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/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;
> +};

I do guess that here (with the introduction of tv32-compat.h) we do
follow what is done for alpha port?

> +
> +#endif /* tv32-compat.h */




Best regards,

Lukasz Majewski

--

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

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

Re: [PATCH 4/6] linux: Use 32-bit time_t for itimerval

[Andreas Schwab]

On Feb 04 2020, Lukasz Majewski wrote:

> I've dug to the @sysnames@ variable set in configure in the source of glibc.
> I'm wondering how RV32 modifies it (and how it differs from ARM32)?

It is explicitly added through Implies (git grep -e wordsize-32 -- '*/Implies*')

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH 6/6] linux: Use 32-bit time_t for rusage

[Lukasz Majewski]

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

Hi Andreas,

> On Feb 04 2020, Lukasz Majewski wrote:
> 
> > Unfortunately, for ARM32 (armv7) the path:
> > sysdeps/unix/sysv/linux/generic/wordsize-32/ is not reached at all.
> >  
> 
> unix/linux/generic/wordsize-32 cannot be used for armv7, since it
> doesn't use asm-generic syscalls.

Could you elaborate on this?

I though that for ARM32 glibc wrappers for syscalls were generated from
sysdeps/unix/syscalls.list. And those syscalls were matching ones from
uapi headers generated from
https://elixir.bootlin.com/linux/v5.3-rc5/source/arch/arm/tools/syscall.tbl#L420

However for ARM linux (5.1):
kernel-build-artifacts/include/generated/uapi/linux
 only has version.h [*]

and all relevant headers are available in:
kernel-build-artifacts/arch/arm/include/generated/uapi/asm/  {unistd*.h}


So I guess that you refer to the almost empty [*] directory ?
(And this means the lack of asm-generic syscalls in this case)?

> 
> Andreas.
> 




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 4/6] linux: Use 32-bit time_t for itimerval

[Lukasz Majewski]

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

Hi Andreas, Alistair,

> On Feb 04 2020, Lukasz Majewski wrote:
> 
> > I've dug to the @sysnames@ variable set in configure in the source
> > of glibc. I'm wondering how RV32 modifies it (and how it differs
> > from ARM32)?  
> 
> It is explicitly added through Implies (git grep -e wordsize-32 --
> '*/Implies*')

Thanks for pointing this out.

It seems like no riscv (rv32) sets this particular patch.

Only it is set for csky and nios2:
sysdeps/unix/sysv/linux/csky/Implies:unix/sysv/linux/generic/wordsize-32
sysdeps/unix/sysv/linux/nios2/Implies:unix/sysv/linux/generic/wordsize-32

However, it seems like I could:

1. Remove setitimer, getitimer, getrusage from
sysdeps/unix/syscalls.list

2. Extend Imply file for ARM32 (sysdeps/unix/sysv/linux/arm/Implies) by
adding
unix/sysv/linux/generic/wordsize-32

Unfortunately, this brings some issues with overflow header

../sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h:42:12: error:
'struct stat' has no member named '__st_ino_pad'; did you mean
'st_ino'? |    if (buf->__st_ino_pad == 0 && buf->__st_size_pad == 0 |
           ^~~~~~~~~~~~ |             st_ino
                                                                                                                                                                                                         

Maybe it would be better to not introduce setitimer, getitimer and
getrusage in:
sysdeps/unix/sysv/linux/generic/wordsize-32/

but instead in:
sysdeps/unix/sysv/linux/


So, it would be widely reusable as for example the __getitimer64 is
aliased anyway to getitimer for __TIMESIZE == 64 and __WORDSIZE==64 ?

(The same approach was taken with __timerfd_settime64 conversion:
https://sourceware.org/git/?p=glibc.git;a=commit;h=eae22432723b877354291aca4dbbfde5891dad59


> 
> Andreas.
> 




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 6/6] linux: Use 32-bit time_t for rusage

[Andreas Schwab]

On Feb 04 2020, Lukasz Majewski wrote:

> I though that for ARM32 glibc wrappers for syscalls were generated from
> sysdeps/unix/syscalls.list.

But linux/generic/wordsize-32/fstatfs.c would override that, for
example.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH 6/6] linux: Use 32-bit time_t for rusage

[Lukasz Majewski]

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

Hi Andreas,

> On Feb 04 2020, Lukasz Majewski wrote:
> 
> > I though that for ARM32 glibc wrappers for syscalls were generated
> > from sysdeps/unix/syscalls.list.  
> 
> But linux/generic/wordsize-32/fstatfs.c would override that, for
> example.

And this is IMHO the issue. Giving ARM32 full access to code located in
linux/generic/wordsize-32 is fragile as it will override wrappers
generated automatically.

In fact for ARM32 one would need either to locate the [sg]etitimer.c and
getrusage.c in e.g. sysdeps/unix/sysv/linux or allow compiling only
mentioned above files in linux/generic/wordsize-32.

The latter option would be possible with hack as:

introduce in sysdeps/unix/sysv/linux/arm/getitimer.c
#include <sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c>


When following this idiom for setitimer and getrusage, the arm32 port
seems to compile with those files.

Is it the recommended way?

> 
> Andreas.
> 




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 6/6] linux: Use 32-bit time_t for rusage

[Lukasz Majewski]

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

Dear community,

> Hi Andreas,
> 
> > On Feb 04 2020, Lukasz Majewski wrote:
> >   
> > > I though that for ARM32 glibc wrappers for syscalls were generated
> > > from sysdeps/unix/syscalls.list.    
> > 
> > But linux/generic/wordsize-32/fstatfs.c would override that, for
> > example.  
> 
> And this is IMHO the issue. Giving ARM32 full access to code located
> in linux/generic/wordsize-32 is fragile as it will override wrappers
> generated automatically.
> 
> In fact for ARM32 one would need either to locate the [sg]etitimer.c
> and getrusage.c in e.g. sysdeps/unix/sysv/linux or allow compiling
> only mentioned above files in linux/generic/wordsize-32.
> 
> The latter option would be possible with hack as:
> 
> introduce in sysdeps/unix/sysv/linux/arm/getitimer.c
> #include <sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c>
> 
> 
> When following this idiom for setitimer and getrusage, the arm32 port
> seems to compile with those files.
> 
> Is it the recommended way?

Any thoughts ?


Just a tester's note:
---------------------

I've run tested this approach with the QEMU + Yocto Y2038 setup [*] and
it seems like this approach (recreation of files with only 
#include <linux/generic/wordsize-32/*.c> in
sysdeps/unix/sysv/linux) works seamlessly when/without Y2038
redirection code applied.


(The only problem is with __setitimer() not handling "old_value" as
NULL properly, but I've pointed it out in the other answer for this
patch).

Moreover, as the path sysdeps/unix/sysv/linux/arm is searched earlier
than sysdeps/unix, there is no need to remove [gs]etitimer and
getrusage from sysdeps/unix/syscalls.list.
(But the same approach would be needed if for example ppc32 or mips32
will need the Y2038 safe conversion for this function).




Links:

[*] - https://github.com/lmajewski/meta-y2038

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




Best regards,

Lukasz Majewski

--

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

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

Re: [PATCH 6/6] linux: Use 32-bit time_t for rusage

[Andreas Schwab]

Why can't the implementation be put in sysdeps/unix/sysv/linux?

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH 6/6] linux: Use 32-bit time_t for rusage

[Lukasz Majewski]

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

Hi Andreas,

> Why can't the implementation be put in sysdeps/unix/sysv/linux?
> 

I would opt for putting the implementation there.

However, Alistair shall say if such move would work for e.g. RV32 (or
maybe there are some issues).

> Andreas.
> 




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 2/6] time: Add a timeval with a long tv_sec and tv_usec

[Alistair Francis]

On Tue, Feb 4, 2020 at 6:24 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Alistair,
>
> > 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 timeval_long struct to be used internally.
>                            ^^^^^^^^^^ - I think that this shall be
>                            updated.

I'm not clear what you mean here, what should be changed?

>
> > ---
> >  include/time.h | 43 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git a/include/time.h b/include/time.h
> > index d425c69ede..c2c05bb671 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -388,6 +388,49 @@ 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.  */
>
> As __timeval32 will be used in e.g __setitimer64 (which in turn will be
> aliased to setitimer on archs with __WORDSIZE==64 && __TIMESIZE==64)
> long seems to be the best option as it will be 64 bits for those
> machines.

Yep, that's the plan :)

>
> For ones with __WORDSIZE==32 it will be 32 bits instead. Am I correct?

Yes.

>
> > +  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)
> > +{
> > +  if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647))
> > +    return (struct __timeval32) { 2147483647, 999999};
>
> What is the purpose of this code?
>
> The valid_* prefix shall indicate that the timeval64 will fit the
> timeval32 and there is no need for any explicit check.

Ah ok. I'll remove the check from here.

>
> I would expect usage of this function as presented here:
> https://patchwork.ozlabs.org/patch/1230884/
>
> if (! in_time_t_range (tv64.tv_sec))
>   {
>     __set_errno (EOVERFLOW);
>     return -1;
>   }
>
>   if (tv)
>     *tv = valid_timeval64_to_timeval (tv64);

and I have added the check to here.

Alistair

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

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

[Lukasz Majewski]

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

Hi Alistair,

> On Tue, Feb 4, 2020 at 6:24 AM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > Hi Alistair,
> >  
> > > 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 timeval_long struct to be used
> > > internally.  
> >                            ^^^^^^^^^^ - I think that this shall be
> >                            updated.  
> 
> I'm not clear what you mean here, what should be changed?

timeval_long -> timeval long ?

> 
> >  
> > > ---
> > >  include/time.h | 43 +++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 43 insertions(+)
> > >
> > > diff --git a/include/time.h b/include/time.h
> > > index d425c69ede..c2c05bb671 100644
> > > --- a/include/time.h
> > > +++ b/include/time.h
> > > @@ -388,6 +388,49 @@ 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.  */  
> >
> > As __timeval32 will be used in e.g __setitimer64 (which in turn
> > will be aliased to setitimer on archs with __WORDSIZE==64 &&
> > __TIMESIZE==64) long seems to be the best option as it will be 64
> > bits for those machines.  
> 
> Yep, that's the plan :)
> 
> >
> > For ones with __WORDSIZE==32 it will be 32 bits instead. Am I
> > correct?  
> 
> Yes.
> 
> >  
> > > +  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)
> > > +{
> > > +  if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647))
> > > +    return (struct __timeval32) { 2147483647, 999999};  
> >
> > What is the purpose of this code?
> >
> > The valid_* prefix shall indicate that the timeval64 will fit the
> > timeval32 and there is no need for any explicit check.  
> 
> Ah ok. I'll remove the check from here.
> 
> >
> > I would expect usage of this function as presented here:
> > https://patchwork.ozlabs.org/patch/1230884/
> >
> > if (! in_time_t_range (tv64.tv_sec))
> >   {
> >     __set_errno (EOVERFLOW);
> >     return -1;
> >   }
> >
> >   if (tv)
> >     *tv = valid_timeval64_to_timeval (tv64);  
> 
> and I have added the check to here.

Thanks :-)

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




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 2/6] time: Add a timeval with a long tv_sec and tv_usec

[Alistair Francis]

On Thu, Feb 6, 2020 at 1:56 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Alistair,
>
> > On Tue, Feb 4, 2020 at 6:24 AM Lukasz Majewski <lukma@denx.de> wrote:
> > >
> > > Hi Alistair,
> > >
> > > > 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 timeval_long struct to be used
> > > > internally.
> > >                            ^^^^^^^^^^ - I think that this shall be
> > >                            updated.
> >
> > I'm not clear what you mean here, what should be changed?
>
> timeval_long -> timeval long ?

Ah! You are right. It should be __timeval32

Alistair

>
> >
> > >
> > > > ---
> > > >  include/time.h | 43 +++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 43 insertions(+)
> > > >
> > > > diff --git a/include/time.h b/include/time.h
> > > > index d425c69ede..c2c05bb671 100644
> > > > --- a/include/time.h
> > > > +++ b/include/time.h
> > > > @@ -388,6 +388,49 @@ 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.  */
> > >
> > > As __timeval32 will be used in e.g __setitimer64 (which in turn
> > > will be aliased to setitimer on archs with __WORDSIZE==64 &&
> > > __TIMESIZE==64) long seems to be the best option as it will be 64
> > > bits for those machines.
> >
> > Yep, that's the plan :)
> >
> > >
> > > For ones with __WORDSIZE==32 it will be 32 bits instead. Am I
> > > correct?
> >
> > Yes.
> >
> > >
> > > > +  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)
> > > > +{
> > > > +  if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647))
> > > > +    return (struct __timeval32) { 2147483647, 999999};
> > >
> > > What is the purpose of this code?
> > >
> > > The valid_* prefix shall indicate that the timeval64 will fit the
> > > timeval32 and there is no need for any explicit check.
> >
> > Ah ok. I'll remove the check from here.
> >
> > >
> > > I would expect usage of this function as presented here:
> > > https://patchwork.ozlabs.org/patch/1230884/
> > >
> > > if (! in_time_t_range (tv64.tv_sec))
> > >   {
> > >     __set_errno (EOVERFLOW);
> > >     return -1;
> > >   }
> > >
> > >   if (tv)
> > >     *tv = valid_timeval64_to_timeval (tv64);
> >
> > and I have added the check to here.
>
> Thanks :-)
>
> >
> > Alistair
> >
> > >
> > >
> > > > +  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
>
>
>
>
> 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

Re: [PATCH 6/6] linux: Use 32-bit time_t for rusage

[Joseph Myers]

On Tue, 4 Feb 2020, Lukasz Majewski wrote:

> > unix/linux/generic/wordsize-32 cannot be used for armv7, since it
> > doesn't use asm-generic syscalls.
> 
> Could you elaborate on this?

There was a long discussion on the linux-kernel mailing list in 2008, much 
of which was under the subject "microblaze syscall list".

The conclusion of that discussion was that, whereas architectures for 
which support was added to the Linux kernel up to that point each had 
their own syscall ABI (their own syscall numbers, their own set of 
associated structure layouts and constants - those layouts and constants 
often following those used by some older proprietary Unix on the same 
architecture, in the cases of the oldest Linux kernel ports), newer Linux 
kernel ports would share a common syscall ABI as far as possible, meaning 
the same syscall numbers, constants and structure layouts.  This common 
ABI is known as the generic or asm-generic ABI.

In glibc, sysdeps/unix/sysv/linux/generic is for architectures using the 
asm-generic syscall ABI, and *only* for such architectures (which, in the 
end, did not include microblaze).  This should not be confused with other 
uses of the word "generic" in the glibc context, in particular 
sysdeps/generic/.

As the Linux kernel arm port was added before 2008, it does not use the 
generic ABI and so cannot use sysdeps/unix/sysv/linux/generic.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 6/6] linux: Use 32-bit time_t for rusage

[Lukasz Majewski]

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

Hi Joseph,

> On Tue, 4 Feb 2020, Lukasz Majewski wrote:
> 
> > > unix/linux/generic/wordsize-32 cannot be used for armv7, since it
> > > doesn't use asm-generic syscalls.  
> > 
> > Could you elaborate on this?  
> 
> There was a long discussion on the linux-kernel mailing list in 2008,
> much of which was under the subject "microblaze syscall list".
> 
> The conclusion of that discussion was that, whereas architectures for 
> which support was added to the Linux kernel up to that point each had 
> their own syscall ABI (their own syscall numbers, their own set of 
> associated structure layouts and constants - those layouts and
> constants often following those used by some older proprietary Unix
> on the same architecture, in the cases of the oldest Linux kernel
> ports), newer Linux kernel ports would share a common syscall ABI as
> far as possible, meaning the same syscall numbers, constants and
> structure layouts.  This common ABI is known as the generic or
> asm-generic ABI.
> 
> In glibc, sysdeps/unix/sysv/linux/generic is for architectures using
> the asm-generic syscall ABI, and *only* for such architectures
> (which, in the end, did not include microblaze).  This should not be
> confused with other uses of the word "generic" in the glibc context,
> in particular sysdeps/generic/.
> 
> As the Linux kernel arm port was added before 2008, it does not use
> the generic ABI and so cannot use sysdeps/unix/sysv/linux/generic.
> 

Thank you for the explanation.

Considering the above - it would be best to place
setitimer/getitimer/getrusage code into sysdeps/unix/sysv/linux , so
all Linux glibc ports could use them.


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 6/6] linux: Use 32-bit time_t for rusage

[Alistair Francis]

On Tue, Feb 4, 2020 at 6:23 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.
> >
> > This means that all 32-bit architectures with a 64-bit time_t will be
> > able to use this generic implementation.
> >
> > This code is based on similar code in alpha, but adjusted to pass the
> > 32-bit time_t to the kernel.
> >
> > We can't directly call __wait4 as it expects a struct rusage but we
> > have to pass in and use a struct rusage32. The same appies to
> > __getrusage. ---
> >  include/sys/resource.h                        |  6 ++
> >  .../linux/generic/wordsize-32/getrusage.c     | 52 ++++++++++++
>
> I wanted to test this code with ARM Y2038 test setup (yocto with qemu).

Thanks for doing that!

>
> Unfortunately, for ARM32 (armv7) the path:
> sysdeps/unix/sysv/linux/generic/wordsize-32/ is not reached at all.
>
> In the build log (with make --debug) I do see following paths:
>
> config_sysdirs='sysdeps/unix/sysv/linux/arm/le
> sysdeps/unix/sysv/linux/arm sysdeps/arm/nptl sysdeps/unix/sysv/linux
> sysdeps/nptl sysdeps/pthread sysdeps/gnu sysdeps/unix/inet
> sysdeps/unix/sysv sysdeps/unix/arm sysdeps/unix sysdeps/posix
> sysdeps/arm/le/armv7/multiarch sysdeps/arm/armv7/multiarch
> sysdeps/arm/le/armv7 sysdeps/arm/armv7 sysdeps/arm/armv6t2
> sysdeps/arm/armv6 sysdeps/arm/le sysdeps/arm sysdeps/wordsize-32
> sysdeps/ieee754/flt-32 sysdeps/ieee754/dbl-64 sysdeps/ieee754
> sysdeps/generic'
>
> The "wordsize-32" only appears in ./sysdeps/wordsize-32 (and only
> sysdeps/generic is defined for searching files to build).
>
> How shall I tune my setup to also make
> sysdeps/unix/sysv/linux/generic/wordsize-32/ accessible for glibc build?
>
> Could you share output from yours build setup:
>
> grep config-sysdir config.make

I'll comment on this in a later reply as there has been some
discussion in this thread.

>
>
> >  .../linux/generic/wordsize-32/tv32-compat.h   | 47 +++++++++++
> >  .../sysv/linux/generic/wordsize-32/wait4.c    | 83
> > +++++++++++++++++++ 4 files changed, 188 insertions(+)
> >  create mode 100644
> > sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c create mode
> > 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/wait4.c
> >
> > diff --git a/include/sys/resource.h b/include/sys/resource.h
> > index 9d604dfe3e..fcba56303a 100644
> > --- a/include/sys/resource.h
> > +++ b/include/sys/resource.h
> > @@ -134,5 +134,11 @@ 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
> > +#else
> > +extern int __getrusage64 (enum __rusage_who who, struct __rusage64
>
> Please add libc_hidden_proto (__getrusage64)

Done

>
> > *usage); +#endif
> >  #endif
> >  #endif
> > diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c
> > b/sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c new file
> > mode 100644 index 0000000000..a495cc6c8b
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/getrusage.c
> > @@ -0,0 +1,52 @@
> > +/* getrusage -- get the rusage struct.  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 <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
>
> Please add libc_hidden_def (__getrusage64)

Done

>
> > +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)
>
> Ok.
>
> > diff --git
> > a/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> > b/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h index
> > 4eb6f216ea..c2231f042f 100644 ---
> > a/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h +++
> > b/sysdeps/unix/sysv/linux/generic/wordsize-32/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/generic/wordsize-32/wait4.c
> > b/sysdeps/unix/sysv/linux/generic/wordsize-32/wait4.c new file mode
> > 100644 index 0000000000..ae42b2ecbb
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/wait4.c
> > @@ -0,0 +1,83 @@
> > +/* wait4 -- wait for process to change state.  Linux/RV32/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 <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)
>
> If I may have one "small" remark - it seems like wait4 uses here struct
> rusage.
>
> Would it be a big problem if you provide 64 bit conversion for this
> function as well?
>
> I mean the conversion code for it:
>
> pid_t
> __wait4_time64 (pid_t pid, int *stat_loc, int options, struct __rusage64
> *usage)
> {
>
> }
>
> #if __TIMESIZE != 64
>
> pid_t
> __wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
> {
>
> //conversions
>
> ret = __wait4_time64 ()
>
> //conversions
>
> }
>
> #endif

Yep, I have added this.

Alistair

>
> > +{
> > +  struct __rusage32 usage32;
> > +  idtype_t idtype = P_PID;
> > +
> > +  if (pid < -1)
> > +    {
> > +      idtype = P_PGID;
> > +      pid *= -1;
> > +    }
> > +  else if (pid == -1)
> > +    idtype = P_ALL;
> > +  else if (pid == 0)
> > +    idtype = P_PGID;
> > +
> > +  options |= WEXITED;
> > +
> > +  siginfo_t infop;
> > +  if (SYSCALL_CANCEL (waitid, idtype, pid, &infop, options,
> > &usage32) < 0)
> > +    return -1;
> > +
> > +  if (stat_loc)
> > +    {
> > +      switch (infop.si_code)
> > +        {
> > +        case CLD_EXITED:
> > +          *stat_loc = W_EXITCODE (infop.si_status, 0);
> > +          break;
> > +        case CLD_DUMPED:
> > +          *stat_loc = WCOREFLAG | infop.si_status;
> > +    break;
> > +        case CLD_KILLED:
> > +          *stat_loc = infop.si_status;
> > +          break;
> > +        case CLD_TRAPPED:
> > +        case CLD_STOPPED:
> > +          *stat_loc = W_STOPCODE (infop.si_status);
> > +          break;
> > +        case CLD_CONTINUED:
> > +          *stat_loc = __W_CONTINUED;
> > +          break;
> > +  default:
> > +    *stat_loc = 0;
> > +    break;
> > +        }
> > +    }
> > +
> > +  if (usage != NULL)
> > +    {
> > +      struct __rusage64 usage64;
> > +      rusage32_to_rusage64 (&usage32, &usage64);
> > +      rusage64_to_rusage (&usage64, usage);
> > +    }
> > +
> > +  return infop.si_pid;
> > +}
> > +
> > +libc_hidden_def (__wait4);
> > +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

Re: [PATCH 6/6] linux: Use 32-bit time_t for rusage

[Alistair Francis]

On Fri, Feb 7, 2020 at 12:51 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Joseph,
>
> > On Tue, 4 Feb 2020, Lukasz Majewski wrote:
> >
> > > > unix/linux/generic/wordsize-32 cannot be used for armv7, since it
> > > > doesn't use asm-generic syscalls.
> > >
> > > Could you elaborate on this?
> >
> > There was a long discussion on the linux-kernel mailing list in 2008,
> > much of which was under the subject "microblaze syscall list".
> >
> > The conclusion of that discussion was that, whereas architectures for
> > which support was added to the Linux kernel up to that point each had
> > their own syscall ABI (their own syscall numbers, their own set of
> > associated structure layouts and constants - those layouts and
> > constants often following those used by some older proprietary Unix
> > on the same architecture, in the cases of the oldest Linux kernel
> > ports), newer Linux kernel ports would share a common syscall ABI as
> > far as possible, meaning the same syscall numbers, constants and
> > structure layouts.  This common ABI is known as the generic or
> > asm-generic ABI.
> >
> > In glibc, sysdeps/unix/sysv/linux/generic is for architectures using
> > the asm-generic syscall ABI, and *only* for such architectures
> > (which, in the end, did not include microblaze).  This should not be
> > confused with other uses of the word "generic" in the glibc context,
> > in particular sysdeps/generic/.
> >
> > As the Linux kernel arm port was added before 2008, it does not use
> > the generic ABI and so cannot use sysdeps/unix/sysv/linux/generic.
> >
>
> Thank you for the explanation.
>
> Considering the above - it would be best to place
> setitimer/getitimer/getrusage code into sysdeps/unix/sysv/linux , so
> all Linux glibc ports could use them.

I just sent a v2 that does exactly that.

Alistair

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

Re: [PATCH 4/6] linux: Use 32-bit time_t for itimerval

[Alistair Francis]

On Tue, Feb 4, 2020 at 6:49 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Alistair,
>
> Maybe it would be good to rewrite the subject line to also reflect the
> introduction of __[gs]etitimer64 ?
>
> > 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.
> >
> > This means that all 32-bit architectures with a 64-bit time_t will be
> > able to use this generic implementation.
> >
> > This code is based on similar code in alpha, but adjusted to pass the
> > 32-bit time_t to the kernel.
> >
> > We can't directly call the __getitimer/__setitimer functions as they
> > expect a struct itimerval but we need to pass in a struct itimerval32.
>
> Please update this patch series to also include information about
> build/run testing.
>
> For example:
>
> Build-tests:
> ./src/scripts/build-many-glibcs.py glibcs

Sorry Lukasz, I somehow lost your response in my v2. I have updated my
v3 with your comments here.

>
>
> > ---
> >  include/time.h                                | 12 +++
> >  .../linux/generic/wordsize-32/getitimer.c     | 54 +++++++++++++
> >  .../linux/generic/wordsize-32/setitimer.c     | 77
> > +++++++++++++++++++ .../linux/generic/wordsize-32/tv32-compat.h   |
> > 35 +++++++++ 4 files changed, 178 insertions(+)
> >  create mode 100644
> > sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c create mode
>
> The same issue as pointed out in patch 6/6. The path
> sysdeps/unix/sysv/linux/generic/wordsize-32 is NOT searched by ARMv7
> (ARM32) for files to be built.
>
> I've removed getitimer, getrusage and setitimer from
> ./sysdeps/unix/syscalls.list, so they are NOT auto generated wrappers
> anymore, but this doesn't help.
>
> It seems like for RV32 some extra paths are added...
>
> I've dug to the @sysnames@ variable set in configure in the source of glibc.
> I'm wondering how RV32 modifies it (and how it differs from ARM32)?

This has been addressed in v2.

>
>
> > 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c create
> > mode 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> >
> > diff --git a/include/time.h b/include/time.h
> > index 898ff0fb2d..e1d80b4190 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>
> >
> > @@ -118,6 +119,17 @@ struct __itimerval64
> >  };
> >  #endif
> >
> > +#if __TIMESIZE == 64
> > +# define __getitimer64 __getitimer
> > +# define __setitimer64 __setitimer
> > +#else
> > +extern int __getitimer64 (enum __itimer_which __which,
> > +                          struct __itimerval64 *__value);
>
> Please add here:
> libc_hidden_proto (__getitimer64)

Fixed in v3

>
> > +extern int __setitimer64 (enum __itimer_which __which,
> > +                          const struct __itimerval64 *__restrict
> > __new,
> > +                          struct __itimerval64 *__restrict __old);
>
> Please add here:
> libc_hidden_proto (__setitimer64)

Fixed in v3

>
> > +#endif
> > +
> >  #if __TIMESIZE == 64
> >  # define __ctime64 ctime
> >  #else
> > diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c
> > b/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c new file
> > mode 100644 index 0000000000..28a3e31126
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/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
>
> Please add here:
> libc_hidden_def (__gettimer64)

Fixed in v3

>
> Rationale:
>
> Before we introduce the Y2038 support (with -D_TIME_BITS=64) the
> __gettimer64 shall be hidden (to avoid resolving it via PLT).
>
> > +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);
> > +}
>
> Ok. As we expect 32 bit values - there is no need for checking (and 64
> <-> 32 conversions will be OK).
>
> > +#endif
> > +weak_alias (__getitimer, getitimer)
> > diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
> > b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c new file
> > mode 100644 index 0000000000..fabc7f2c0c
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
> > @@ -0,0 +1,77 @@
> > +/* 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;
> > +  new_value_32.it_interval
> > +    = valid_timeval64_to_timeval32 (new_value->it_interval);
> > +  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); +
>
> The above code is correct of course.
>
> However, I've used another approach for timerfd_settime (IMHO a bit
> more concise):
> https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/timerfd_settime.c;h=164b4e860acb6a365a3890f30e2eac364a518c22;hb=eae22432723b877354291aca4dbbfde5891dad59#l49
>
> > +  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
>
> Please add here:
> libc_hidden_def (__settimer64)

Fixed in v3

>
> > +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);
>
> I think that here we shall also consider the NULL pointer for old_value
> (as stated in http://man7.org/linux/man-pages/man2/setitimer.2.html)
>
> ret = __setitimer64 (which, &new64, old_value ? &old64 : NULL);
>
> > +
> > +  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);
> > +
>
> Maybe it would be better to change this code to:
>
> if (ret == 0 && old_value)
>   {
>     old_value->it_interval
>       = valid_timeval64_to_timeval (old64.it_interval);
>     old_value->it_value
>       = valid_timeval64_to_timeval (old64.it_value);
>   }
>
> return ret

Fixed in v3

>
> Please find the timerfd_settime as a reference:
> https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/timerfd_settime.c;h=164b4e860acb6a365a3890f30e2eac364a518c22;hb=eae22432723b877354291aca4dbbfde5891dad59#l77
>
> > +  return ret;
> > +}
> > +#endif
> > +weak_alias (__setitimer, setitimer)
> > diff --git
> > a/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> > b/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h new file
> > mode 100644 index 0000000000..4eb6f216ea --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/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;
> > +};
>
> I do guess that here (with the introduction of tv32-compat.h) we do
> follow what is done for alpha port?

Alpha always uses a 32-bit value, while in the generic case we use a
long (so the wordsize).

Alistair

>
> > +
> > +#endif /* tv32-compat.h */
>
>
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Re: [PATCH 4/6] linux: Use 32-bit time_t for itimerval

[Lukasz Majewski]

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

On Mon, 10 Feb 2020 09:56:40 -0800
Alistair Francis <alistair23@gmail.com> wrote:

> On Tue, Feb 4, 2020 at 6:49 AM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > Hi Alistair,
> >
> > Maybe it would be good to rewrite the subject line to also reflect
> > the introduction of __[gs]etitimer64 ?
> >  
> > > 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.
> > >
> > > This means that all 32-bit architectures with a 64-bit time_t
> > > will be able to use this generic implementation.
> > >
> > > This code is based on similar code in alpha, but adjusted to pass
> > > the 32-bit time_t to the kernel.
> > >
> > > We can't directly call the __getitimer/__setitimer functions as
> > > they expect a struct itimerval but we need to pass in a struct
> > > itimerval32.  
> >
> > Please update this patch series to also include information about
> > build/run testing.
> >
> > For example:
> >
> > Build-tests:
> > ./src/scripts/build-many-glibcs.py glibcs  
> 
> Sorry Lukasz, I somehow lost your response in my v2. I have updated my
> v3 with your comments here.

Ok, I'm looking forward for v3 then :-).

Thanks for your patches.

> 
> >
> >  
> > > ---
> > >  include/time.h                                | 12 +++
> > >  .../linux/generic/wordsize-32/getitimer.c     | 54 +++++++++++++
> > >  .../linux/generic/wordsize-32/setitimer.c     | 77
> > > +++++++++++++++++++ .../linux/generic/wordsize-32/tv32-compat.h
> > > | 35 +++++++++ 4 files changed, 178 insertions(+)
> > >  create mode 100644
> > > sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c create
> > > mode  
> >
> > The same issue as pointed out in patch 6/6. The path
> > sysdeps/unix/sysv/linux/generic/wordsize-32 is NOT searched by ARMv7
> > (ARM32) for files to be built.
> >
> > I've removed getitimer, getrusage and setitimer from
> > ./sysdeps/unix/syscalls.list, so they are NOT auto generated
> > wrappers anymore, but this doesn't help.
> >
> > It seems like for RV32 some extra paths are added...
> >
> > I've dug to the @sysnames@ variable set in configure in the source
> > of glibc. I'm wondering how RV32 modifies it (and how it differs
> > from ARM32)?  
> 
> This has been addressed in v2.
> 
> >
> >  
> > > 100644 sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
> > > create mode 100644
> > > sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> > >
> > > diff --git a/include/time.h b/include/time.h
> > > index 898ff0fb2d..e1d80b4190 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>
> > >
> > > @@ -118,6 +119,17 @@ struct __itimerval64
> > >  };
> > >  #endif
> > >
> > > +#if __TIMESIZE == 64
> > > +# define __getitimer64 __getitimer
> > > +# define __setitimer64 __setitimer
> > > +#else
> > > +extern int __getitimer64 (enum __itimer_which __which,
> > > +                          struct __itimerval64 *__value);  
> >
> > Please add here:
> > libc_hidden_proto (__getitimer64)  
> 
> Fixed in v3
> 
> >  
> > > +extern int __setitimer64 (enum __itimer_which __which,
> > > +                          const struct __itimerval64 *__restrict
> > > __new,
> > > +                          struct __itimerval64 *__restrict
> > > __old);  
> >
> > Please add here:
> > libc_hidden_proto (__setitimer64)  
> 
> Fixed in v3
> 
> >  
> > > +#endif
> > > +
> > >  #if __TIMESIZE == 64
> > >  # define __ctime64 ctime
> > >  #else
> > > diff --git
> > > a/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c
> > > b/sysdeps/unix/sysv/linux/generic/wordsize-32/getitimer.c new
> > > file mode 100644 index 0000000000..28a3e31126 --- /dev/null
> > > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/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  
> >
> > Please add here:
> > libc_hidden_def (__gettimer64)  
> 
> Fixed in v3
> 
> >
> > Rationale:
> >
> > Before we introduce the Y2038 support (with -D_TIME_BITS=64) the
> > __gettimer64 shall be hidden (to avoid resolving it via PLT).
> >  
> > > +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);
> > > +}  
> >
> > Ok. As we expect 32 bit values - there is no need for checking (and
> > 64 <-> 32 conversions will be OK).
> >  
> > > +#endif
> > > +weak_alias (__getitimer, getitimer)
> > > diff --git
> > > a/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
> > > b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c new
> > > file mode 100644 index 0000000000..fabc7f2c0c --- /dev/null
> > > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/setitimer.c
> > > @@ -0,0 +1,77 @@
> > > +/* 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;
> > > +  new_value_32.it_interval
> > > +    = valid_timeval64_to_timeval32 (new_value->it_interval);
> > > +  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); +  
> >
> > The above code is correct of course.
> >
> > However, I've used another approach for timerfd_settime (IMHO a bit
> > more concise):
> > https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/timerfd_settime.c;h=164b4e860acb6a365a3890f30e2eac364a518c22;hb=eae22432723b877354291aca4dbbfde5891dad59#l49
> >  
> > > +  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  
> >
> > Please add here:
> > libc_hidden_def (__settimer64)  
> 
> Fixed in v3
> 
> >  
> > > +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);  
> >
> > I think that here we shall also consider the NULL pointer for
> > old_value (as stated in
> > http://man7.org/linux/man-pages/man2/setitimer.2.html)
> >
> > ret = __setitimer64 (which, &new64, old_value ? &old64 : NULL);
> >  
> > > +
> > > +  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);
> > > +  
> >
> > Maybe it would be better to change this code to:
> >
> > if (ret == 0 && old_value)
> >   {
> >     old_value->it_interval
> >       = valid_timeval64_to_timeval (old64.it_interval);
> >     old_value->it_value
> >       = valid_timeval64_to_timeval (old64.it_value);
> >   }
> >
> > return ret  
> 
> Fixed in v3
> 
> >
> > Please find the timerfd_settime as a reference:
> > https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/timerfd_settime.c;h=164b4e860acb6a365a3890f30e2eac364a518c22;hb=eae22432723b877354291aca4dbbfde5891dad59#l77
> >  
> > > +  return ret;
> > > +}
> > > +#endif
> > > +weak_alias (__setitimer, setitimer)
> > > diff --git
> > > a/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h
> > > b/sysdeps/unix/sysv/linux/generic/wordsize-32/tv32-compat.h new
> > > file mode 100644 index 0000000000..4eb6f216ea --- /dev/null
> > > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/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;
> > > +};  
> >
> > I do guess that here (with the introduction of tv32-compat.h) we do
> > follow what is done for alpha port?  
> 
> Alpha always uses a 32-bit value, while in the generic case we use a
> long (so the wordsize).
> 
> Alistair
> 
> >  
> > > +
> > > +#endif /* tv32-compat.h */  
> >
> >
> >
> >
> > Best regards,
> >
> > Lukasz Majewski
> >
> > --
> >
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma@denx.de  




Best regards,

Lukasz Majewski

--

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

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