[PATCH 1/3] linux: Fix time64 support for futimesat

[PATCH 1/3] linux: Fix time64 support for futimesat

[Adhemerval Zanella via Libc-alpha]

The generic implementation does not support time64 and the default
one return overflow for invalid tv_sec with UTIME_NOW / UTIME_OMIT
(which is valid since tv_sec in such cases is ignored by the
kernel).

Checked on x86_64-linux-gnu and i686-linux-gnu (on 5.4 and on 4.15
kernel).
---
 sysdeps/unix/sysv/linux/generic/futimesat.c | 52 ---------------------
 sysdeps/unix/sysv/linux/utimensat.c         |  8 +++-
 2 files changed, 6 insertions(+), 54 deletions(-)
 delete mode 100644 sysdeps/unix/sysv/linux/generic/futimesat.c

diff --git a/sysdeps/unix/sysv/linux/generic/futimesat.c b/sysdeps/unix/sysv/linux/generic/futimesat.c
deleted file mode 100644
index 7be1fbc252..0000000000
--- a/sysdeps/unix/sysv/linux/generic/futimesat.c
+++ /dev/null
@@ -1,52 +0,0 @@
-/* futimesat -- Change access and modification times of file.  Linux version.
-   Copyright (C) 2005-2020 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library.  If not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#include <errno.h>
-#include <fcntl.h>
-#include <stddef.h>
-#include <stdio.h>
-#include <string.h>
-#include <utime.h>
-#include <sys/time.h>
-#include <sysdep.h>
-
-
-/* Change the access time of FILE relative to FD to TVP[0] and
-   the modification time of FILE to TVP[1].  */
-int
-futimesat (int fd, const char *file, const struct timeval tvp[2])
-{
-  struct timespec tsp[2];
-  int result;
-
-  if (tvp)
-    {
-      if (tvp[0].tv_usec >= 1000000 || tvp[0].tv_usec < 0
-          || tvp[1].tv_usec >= 1000000 || tvp[1].tv_usec < 0)
-        {
-          __set_errno (EINVAL);
-          return -1;
-        }
-      TIMEVAL_TO_TIMESPEC (&tvp[0], &tsp[0]);
-      TIMEVAL_TO_TIMESPEC (&tvp[1], &tsp[1]);
-    }
-
-  result = INLINE_SYSCALL (utimensat, 4, fd, file, tvp ? tsp : NULL, 0);
-  return result;
-}
diff --git a/sysdeps/unix/sysv/linux/utimensat.c b/sysdeps/unix/sysv/linux/utimensat.c
index ea23c2f051..72784d824a 100644
--- a/sysdeps/unix/sysv/linux/utimensat.c
+++ b/sysdeps/unix/sysv/linux/utimensat.c
@@ -36,9 +36,13 @@ __utimensat64_helper (int fd, const char *file,
   if (ret == 0 || errno != ENOSYS)
     return ret;
 
+  /* For UTIME_NOW and UTIME_OMIT the value of tv_sec field is ignored.  */
+# define NS_VALID(ns) \
+  ((((ns).tv_nsec == UTIME_NOW || (ns).tv_nsec == UTIME_OMIT) \
+   || in_time_t_range ((ns).tv_sec)))
+
   if (tsp64 != NULL
-      && (! in_time_t_range (tsp64[0].tv_sec)
-          || ! in_time_t_range (tsp64[1].tv_sec)))
+      && (!NS_VALID (tsp64[0]) || !NS_VALID (tsp64[1])))
     {
       __set_errno (EOVERFLOW);
       return -1;
-- 
2.25.1

[PATCH 2/3] Move ftime to a compatibility symbol

[Adhemerval Zanella via Libc-alpha]

It was made deprecated on 2.31, so it moves to compat symbol after
two releases.  It was also removed from exported symbol for riscv32
(since ABI will be supported on for 2.33).

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 include/sys/timeb.h                           |  1 -
 .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
 time/Makefile                                 |  5 +-
 time/ftime.c                                  | 19 +++++-
 time/sys/timeb.h                              | 44 --------------
 time/tst-ftime.c                              | 59 ++++++++++---------
 6 files changed, 50 insertions(+), 79 deletions(-)
 delete mode 100644 include/sys/timeb.h
 delete mode 100644 time/sys/timeb.h

diff --git a/include/sys/timeb.h b/include/sys/timeb.h
deleted file mode 100644
index 9f4509c35e..0000000000
--- a/include/sys/timeb.h
+++ /dev/null
@@ -1 +0,0 @@
-#include <time/sys/timeb.h>
diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
index 36aa34a5e7..e977715088 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
@@ -795,7 +795,6 @@ GLIBC_2.33 fsync F
 GLIBC_2.33 ftell F
 GLIBC_2.33 ftello F
 GLIBC_2.33 ftello64 F
-GLIBC_2.33 ftime F
 GLIBC_2.33 ftok F
 GLIBC_2.33 ftruncate F
 GLIBC_2.33 ftruncate64 F
diff --git a/time/Makefile b/time/Makefile
index a4fb13d6a3..ab8fb3303b 100644
--- a/time/Makefile
+++ b/time/Makefile
@@ -22,7 +22,7 @@ subdir	:= time
 
 include ../Makeconfig
 
-headers := time.h sys/time.h sys/timeb.h bits/time.h			\
+headers := time.h sys/time.h bits/time.h				\
 	   bits/types/clockid_t.h bits/types/clock_t.h			\
 	   bits/types/struct_itimerspec.h				\
 	   bits/types/struct_timespec.h bits/types/struct_timeval.h	\
@@ -45,9 +45,10 @@ aux :=	    era alt_digit lc-time-cleanup
 tests	:= test_time clocktest tst-posixtz tst-strptime tst_wcsftime \
 	   tst-getdate tst-mktime tst-mktime2 tst-ftime_l tst-strftime \
 	   tst-mktime3 tst-strptime2 bug-asctime bug-asctime_r bug-mktime1 \
-	   tst-strptime3 bug-getdate1 tst-strptime-whitespace tst-ftime \
+	   tst-strptime3 bug-getdate1 tst-strptime-whitespace \
 	   tst-tzname tst-y2039 bug-mktime4 tst-strftime2 tst-strftime3 \
 	   tst-clock tst-clock2 tst-clock_nanosleep tst-cpuclock1
+tests-internal := tst-ftime
 
 include ../Rules
 
diff --git a/time/ftime.c b/time/ftime.c
index 93f485bbf7..be3295ef76 100644
--- a/time/ftime.c
+++ b/time/ftime.c
@@ -16,11 +16,23 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/timeb.h>
+#include <shlib-compat.h>
+
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_33)
+
 #include <time.h>
 
+struct timeb
+  {
+    time_t time;		/* Seconds since epoch, as from `time'.  */
+    unsigned short int millitm;	/* Additional milliseconds.  */
+    short int timezone;		/* Minutes west of GMT.  */
+    short int dstflag;		/* Nonzero if Daylight Savings Time used.  */
+  };
+
 int
-ftime (struct timeb *timebuf)
+attribute_compat_text_section
+__ftime (struct timeb *timebuf)
 {
   struct timespec ts;
   __clock_gettime (CLOCK_REALTIME, &ts);
@@ -31,3 +43,6 @@ ftime (struct timeb *timebuf)
   timebuf->dstflag = 0;
   return 0;
 }
+
+compat_symbol (libc, __ftime, ftime, GLIBC_2_0);
+#endif
diff --git a/time/sys/timeb.h b/time/sys/timeb.h
deleted file mode 100644
index 641c333450..0000000000
--- a/time/sys/timeb.h
+++ /dev/null
@@ -1,44 +0,0 @@
-/* Copyright (C) 1994-2020 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#ifndef _SYS_TIMEB_H
-#define _SYS_TIMEB_H	1
-
-#include <features.h>
-
-#include <bits/types/time_t.h>
-
-__BEGIN_DECLS
-
-/* Structure returned by the `ftime' function.  */
-
-struct timeb
-  {
-    time_t time;		/* Seconds since epoch, as from `time'.  */
-    unsigned short int millitm;	/* Additional milliseconds.  */
-    short int timezone;		/* Minutes west of GMT.  */
-    short int dstflag;		/* Nonzero if Daylight Savings Time used.  */
-  };
-
-/* Fill in TIMEBUF with information about the current time.  */
-
-extern int ftime (struct timeb *__timebuf)
-  __nonnull ((1)) __attribute_deprecated__;
-
-__END_DECLS
-
-#endif	/* sys/timeb.h */
diff --git a/time/tst-ftime.c b/time/tst-ftime.c
index 08916c0c65..6978feb0f1 100644
--- a/time/tst-ftime.c
+++ b/time/tst-ftime.c
@@ -16,9 +16,23 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/timeb.h>
-#include <stdio.h>
-#include <libc-diag.h>
+
+#include <shlib-compat.h>
+#if TEST_COMPAT (libc, GLIBC_2_0, GLIBC_2_33)
+#include <time.h>
+#include <support/check.h>
+
+compat_symbol_reference (libc, ftime, ftime, GLIBC_2_0);
+
+struct timeb
+  {
+    time_t time;
+    unsigned short int millitm;
+    short int timezone;
+    short int dstflag;
+  };
+
+extern int ftime (struct timeb *__timebuf);
 
 static int
 do_test (void)
@@ -30,36 +44,23 @@ do_test (void)
     {
       prev = curr;
 
-      /* ftime was deprecated on 2.31.  */
-      DIAG_PUSH_NEEDS_COMMENT;
-      DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wdeprecated-declarations");
-
-      if (ftime (&curr))
-        {
-          printf ("ftime returned an error\n");
-          return 1;
-        }
-
-      DIAG_POP_NEEDS_COMMENT;
-
-      if (curr.time < prev.time)
-        {
-          printf ("ftime's time flowed backwards\n");
-          return 1;
-        }
-
-      if (curr.time == prev.time
-          && curr.millitm < prev.millitm)
-        {
-          printf ("ftime's millitm flowed backwards\n");
-          return 1;
-        }
+      /* ftime was deprecated on 2.31 and removed on 2.33.  */
+      TEST_COMPARE (ftime (&curr), 0);
+      TEST_VERIFY_EXIT (curr.time >= prev.time);
+      if (curr.time == prev.time)
+	TEST_VERIFY_EXIT (curr.millitm >= prev.millitm);
 
       if (curr.time > prev.time)
         sec ++;
     }
   return 0;
 }
+#else
+static int
+do_test (void)
+{
+  return EXIT_UNSUPPORTED;
+}
+#endif
 
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
-- 
2.25.1

[PATCH 3/3] linux: Add 64-bit time_t support for wait3

[Adhemerval Zanella via Libc-alpha]

It basically calls the 64-bit time_t wait4 internal symbol.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 include/sys/resource.h          |  4 +++
 sysdeps/unix/sysv/linux/wait3.c | 44 +++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/wait3.c

diff --git a/include/sys/resource.h b/include/sys/resource.h
index 64925f257c..2235b020fc 100644
--- a/include/sys/resource.h
+++ b/include/sys/resource.h
@@ -138,12 +138,16 @@ libc_hidden_proto (__setrlimit);
 #if __TIMESIZE == 64
 # define __getrusage64 __getrusage
 # define __wait4_time64 __wait4
+# define __wait3_time64 __wait3
 #else
 extern int __getrusage64 (enum __rusage_who who, struct __rusage64 *usage);
 libc_hidden_proto (__getrusage64)
 extern pid_t __wait4_time64 (pid_t pid, int *stat_loc, int options,
                              struct __rusage64 *usage);
 libc_hidden_proto (__wait4_time64)
+extern pid_t __wait3_time64 (int *stat_loc, int options,
+                             struct __rusage64 *usage);
+libc_hidden_proto (__wait3_time64)
 #endif
 #endif
 #endif
diff --git a/sysdeps/unix/sysv/linux/wait3.c b/sysdeps/unix/sysv/linux/wait3.c
new file mode 100644
index 0000000000..c05776f7ab
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/wait3.c
@@ -0,0 +1,44 @@
+/* Wait for process to change state, BSD style.  Linux version.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <sys/wait.h>
+#include <sys/resource.h>
+#include <sys/types.h>
+
+pid_t
+__wait3_time64 (int *stat_loc, int options, struct __rusage64 *usage)
+{
+  return __wait4_time64 (WAIT_ANY, stat_loc, options, usage);
+}
+#if __TIMESIZE != 64
+libc_hidden_def (__wait3_time64)
+
+pid_t
+__wait3 (int *stat_loc, int options, struct rusage *usage)
+{
+  struct __rusage64 usage64;
+  pid_t ret = __wait3_time64 (stat_loc, options,
+			      usage != NULL ? &usage64 : NULL);
+  if (ret > 0 && usage != NULL)
+     rusage64_to_rusage (&usage64, usage);
+
+  return ret;
+}
+#endif
+
+weak_alias (__wait3, wait3)
-- 
2.25.1

Re: [PATCH 3/3] linux: Add 64-bit time_t support for wait3

[Alistair Francis via Libc-alpha]

On Thu, Oct 15, 2020 at 6:06 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> It basically calls the 64-bit time_t wait4 internal symbol.
>
> Checked on x86_64-linux-gnu and i686-linux-gnu.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/sys/resource.h          |  4 +++
>  sysdeps/unix/sysv/linux/wait3.c | 44 +++++++++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
>  create mode 100644 sysdeps/unix/sysv/linux/wait3.c
>
> diff --git a/include/sys/resource.h b/include/sys/resource.h
> index 64925f257c..2235b020fc 100644
> --- a/include/sys/resource.h
> +++ b/include/sys/resource.h
> @@ -138,12 +138,16 @@ libc_hidden_proto (__setrlimit);
>  #if __TIMESIZE == 64
>  # define __getrusage64 __getrusage
>  # define __wait4_time64 __wait4
> +# define __wait3_time64 __wait3
>  #else
>  extern int __getrusage64 (enum __rusage_who who, struct __rusage64 *usage);
>  libc_hidden_proto (__getrusage64)
>  extern pid_t __wait4_time64 (pid_t pid, int *stat_loc, int options,
>                               struct __rusage64 *usage);
>  libc_hidden_proto (__wait4_time64)
> +extern pid_t __wait3_time64 (int *stat_loc, int options,
> +                             struct __rusage64 *usage);
> +libc_hidden_proto (__wait3_time64)
>  #endif
>  #endif
>  #endif
> diff --git a/sysdeps/unix/sysv/linux/wait3.c b/sysdeps/unix/sysv/linux/wait3.c
> new file mode 100644
> index 0000000000..c05776f7ab
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/wait3.c
> @@ -0,0 +1,44 @@
> +/* Wait for process to change state, BSD style.  Linux version.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <sys/wait.h>
> +#include <sys/resource.h>
> +#include <sys/types.h>
> +
> +pid_t
> +__wait3_time64 (int *stat_loc, int options, struct __rusage64 *usage)
> +{
> +  return __wait4_time64 (WAIT_ANY, stat_loc, options, usage);
> +}
> +#if __TIMESIZE != 64
> +libc_hidden_def (__wait3_time64)
> +
> +pid_t
> +__wait3 (int *stat_loc, int options, struct rusage *usage)
> +{
> +  struct __rusage64 usage64;
> +  pid_t ret = __wait3_time64 (stat_loc, options,
> +                             usage != NULL ? &usage64 : NULL);
> +  if (ret > 0 && usage != NULL)
> +     rusage64_to_rusage (&usage64, usage);
> +
> +  return ret;
> +}
> +#endif
> +
> +weak_alias (__wait3, wait3)
> --
> 2.25.1
>

Re: [PATCH 1/3] linux: Fix time64 support for futimesat

[Lukasz Majewski]

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

Hi Adhemerval,

> The generic implementation does not support time64 and the default
> one return overflow for invalid tv_sec with UTIME_NOW / UTIME_OMIT
> (which is valid since tv_sec in such cases is ignored by the
> kernel).
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu (on 5.4 and on 4.15
> kernel).
> ---
>  sysdeps/unix/sysv/linux/generic/futimesat.c | 52
> --------------------- sysdeps/unix/sysv/linux/utimensat.c         |
> 8 +++- 2 files changed, 6 insertions(+), 54 deletions(-)
>  delete mode 100644 sysdeps/unix/sysv/linux/generic/futimesat.c
> 
> diff --git a/sysdeps/unix/sysv/linux/generic/futimesat.c
> b/sysdeps/unix/sysv/linux/generic/futimesat.c deleted file mode 100644
> index 7be1fbc252..0000000000
> --- a/sysdeps/unix/sysv/linux/generic/futimesat.c
> +++ /dev/null
> @@ -1,52 +0,0 @@
> -/* futimesat -- Change access and modification times of file.  Linux
> version.
> -   Copyright (C) 2005-2020 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -   Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be
> useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library.  If not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#include <errno.h>
> -#include <fcntl.h>
> -#include <stddef.h>
> -#include <stdio.h>
> -#include <string.h>
> -#include <utime.h>
> -#include <sys/time.h>
> -#include <sysdep.h>
> -
> -
> -/* Change the access time of FILE relative to FD to TVP[0] and
> -   the modification time of FILE to TVP[1].  */
> -int
> -futimesat (int fd, const char *file, const struct timeval tvp[2])
> -{
> -  struct timespec tsp[2];
> -  int result;
> -
> -  if (tvp)
> -    {
> -      if (tvp[0].tv_usec >= 1000000 || tvp[0].tv_usec < 0
> -          || tvp[1].tv_usec >= 1000000 || tvp[1].tv_usec < 0)
> -        {
> -          __set_errno (EINVAL);
> -          return -1;
> -        }
> -      TIMEVAL_TO_TIMESPEC (&tvp[0], &tsp[0]);
> -      TIMEVAL_TO_TIMESPEC (&tvp[1], &tsp[1]);
> -    }
> -
> -  result = INLINE_SYSCALL (utimensat, 4, fd, file, tvp ? tsp : NULL,
> 0);
> -  return result;
> -}

Good that we have removed the duplicated code, so "generic" can reuse
Y2038 ready one from sysdeps/unix/sysv/linux/utimensat.c

> diff --git a/sysdeps/unix/sysv/linux/utimensat.c
> b/sysdeps/unix/sysv/linux/utimensat.c index ea23c2f051..72784d824a
> 100644 --- a/sysdeps/unix/sysv/linux/utimensat.c
> +++ b/sysdeps/unix/sysv/linux/utimensat.c
> @@ -36,9 +36,13 @@ __utimensat64_helper (int fd, const char *file,
>    if (ret == 0 || errno != ENOSYS)
>      return ret;
>  
> +  /* For UTIME_NOW and UTIME_OMIT the value of tv_sec field is
> ignored.  */ +# define NS_VALID(ns) \
> +  ((((ns).tv_nsec == UTIME_NOW || (ns).tv_nsec == UTIME_OMIT) \
> +   || in_time_t_range ((ns).tv_sec)))

IMHO, this is a bit misleading. The macro NS_VALID() seems to be
supposed to check the nano seconds value,but it also check if tv_sec is
in time_t range.

Maybe we can change its name (and passed argument) to e.g.
#define TS_VALID(ts) ...

Despite this minor comment:

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

> +
>    if (tsp64 != NULL
> -      && (! in_time_t_range (tsp64[0].tv_sec)
> -          || ! in_time_t_range (tsp64[1].tv_sec)))
> +      && (!NS_VALID (tsp64[0]) || !NS_VALID (tsp64[1])))
>      {
>        __set_errno (EOVERFLOW);
>        return -1;




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/3] Move ftime to a compatibility symbol

[Lukasz Majewski]

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

Hi Adhemerval,

> It was made deprecated on 2.31, so it moves to compat symbol after
> two releases.  It was also removed from exported symbol for riscv32
> (since ABI will be supported on for 2.33).
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  include/sys/timeb.h                           |  1 -
>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
>  time/Makefile                                 |  5 +-
>  time/ftime.c                                  | 19 +++++-
>  time/sys/timeb.h                              | 44 --------------
>  time/tst-ftime.c                              | 59
> ++++++++++--------- 6 files changed, 50 insertions(+), 79 deletions(-)
>  delete mode 100644 include/sys/timeb.h
>  delete mode 100644 time/sys/timeb.h
> 
> diff --git a/include/sys/timeb.h b/include/sys/timeb.h
> deleted file mode 100644
> index 9f4509c35e..0000000000
> --- a/include/sys/timeb.h
> +++ /dev/null
> @@ -1 +0,0 @@
> -#include <time/sys/timeb.h>
> diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
> b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist index
> 36aa34a5e7..e977715088 100644 ---
> a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist +++
> b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist @@ -795,7 +795,6 @@
> GLIBC_2.33 fsync F GLIBC_2.33 ftell F
>  GLIBC_2.33 ftello F
>  GLIBC_2.33 ftello64 F
> -GLIBC_2.33 ftime F
>  GLIBC_2.33 ftok F
>  GLIBC_2.33 ftruncate F
>  GLIBC_2.33 ftruncate64 F
> diff --git a/time/Makefile b/time/Makefile
> index a4fb13d6a3..ab8fb3303b 100644
> --- a/time/Makefile
> +++ b/time/Makefile
> @@ -22,7 +22,7 @@ subdir	:= time
>  
>  include ../Makeconfig
>  
> -headers := time.h sys/time.h sys/timeb.h bits/time.h
> 	\ +headers := time.h sys/time.h bits/time.h
> 		\ bits/types/clockid_t.h bits/types/clock_t.h
> 		\ bits/types/struct_itimerspec.h
> 		\ bits/types/struct_timespec.h
> bits/types/struct_timeval.h	\ @@ -45,9 +45,10 @@ aux :=
>     era alt_digit lc-time-cleanup tests	:= test_time clocktest
> tst-posixtz tst-strptime tst_wcsftime \ tst-getdate tst-mktime
> tst-mktime2 tst-ftime_l tst-strftime \ tst-mktime3 tst-strptime2
> bug-asctime bug-asctime_r bug-mktime1 \
> -	   tst-strptime3 bug-getdate1 tst-strptime-whitespace
> tst-ftime \
> +	   tst-strptime3 bug-getdate1 tst-strptime-whitespace \
>  	   tst-tzname tst-y2039 bug-mktime4 tst-strftime2
> tst-strftime3 \ tst-clock tst-clock2 tst-clock_nanosleep tst-cpuclock1
> +tests-internal := tst-ftime
>  
>  include ../Rules
>  
> diff --git a/time/ftime.c b/time/ftime.c
> index 93f485bbf7..be3295ef76 100644
> --- a/time/ftime.c
> +++ b/time/ftime.c
> @@ -16,11 +16,23 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#include <sys/timeb.h>
> +#include <shlib-compat.h>
> +
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_33)
> +
>  #include <time.h>
>  
> +struct timeb
> +  {
> +    time_t time;		/* Seconds since epoch, as from
> `time'.  */
> +    unsigned short int millitm;	/* Additional milliseconds.
> */
> +    short int timezone;		/* Minutes west of GMT.  */
> +    short int dstflag;		/* Nonzero if Daylight Savings
> Time used.  */
> +  };
> +
>  int
> -ftime (struct timeb *timebuf)
> +attribute_compat_text_section
> +__ftime (struct timeb *timebuf)
>  {
>    struct timespec ts;
>    __clock_gettime (CLOCK_REALTIME, &ts);
> @@ -31,3 +43,6 @@ ftime (struct timeb *timebuf)
>    timebuf->dstflag = 0;
>    return 0;
>  }
> +
> +compat_symbol (libc, __ftime, ftime, GLIBC_2_0);
> +#endif
> diff --git a/time/sys/timeb.h b/time/sys/timeb.h
> deleted file mode 100644
> index 641c333450..0000000000
> --- a/time/sys/timeb.h
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -/* Copyright (C) 1994-2020 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be
> useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#ifndef _SYS_TIMEB_H
> -#define _SYS_TIMEB_H	1
> -
> -#include <features.h>
> -
> -#include <bits/types/time_t.h>
> -
> -__BEGIN_DECLS
> -
> -/* Structure returned by the `ftime' function.  */
> -
> -struct timeb
> -  {
> -    time_t time;		/* Seconds since epoch, as from
> `time'.  */
> -    unsigned short int millitm;	/* Additional milliseconds.
> */
> -    short int timezone;		/* Minutes west of GMT.  */
> -    short int dstflag;		/* Nonzero if Daylight Savings
> Time used.  */
> -  };
> -
> -/* Fill in TIMEBUF with information about the current time.  */
> -
> -extern int ftime (struct timeb *__timebuf)
> -  __nonnull ((1)) __attribute_deprecated__;
> -
> -__END_DECLS
> -
> -#endif	/* sys/timeb.h */

One less exported header to maintain. Good :-)

> diff --git a/time/tst-ftime.c b/time/tst-ftime.c
> index 08916c0c65..6978feb0f1 100644
> --- a/time/tst-ftime.c
> +++ b/time/tst-ftime.c
> @@ -16,9 +16,23 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#include <sys/timeb.h>
> -#include <stdio.h>
> -#include <libc-diag.h>
> +
> +#include <shlib-compat.h>
> +#if TEST_COMPAT (libc, GLIBC_2_0, GLIBC_2_33)
> +#include <time.h>
> +#include <support/check.h>
> +
> +compat_symbol_reference (libc, ftime, ftime, GLIBC_2_0);
> +
> +struct timeb
> +  {
> +    time_t time;
> +    unsigned short int millitm;
> +    short int timezone;
> +    short int dstflag;
> +  };
> +
> +extern int ftime (struct timeb *__timebuf);
>  
>  static int
>  do_test (void)
> @@ -30,36 +44,23 @@ do_test (void)
>      {
>        prev = curr;
>  
> -      /* ftime was deprecated on 2.31.  */
> -      DIAG_PUSH_NEEDS_COMMENT;
> -      DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wdeprecated-declarations");
> -
> -      if (ftime (&curr))
> -        {
> -          printf ("ftime returned an error\n");
> -          return 1;
> -        }
> -
> -      DIAG_POP_NEEDS_COMMENT;
> -
> -      if (curr.time < prev.time)
> -        {
> -          printf ("ftime's time flowed backwards\n");
> -          return 1;
> -        }
> -
> -      if (curr.time == prev.time
> -          && curr.millitm < prev.millitm)
> -        {
> -          printf ("ftime's millitm flowed backwards\n");
> -          return 1;
> -        }
> +      /* ftime was deprecated on 2.31 and removed on 2.33.  */
> +      TEST_COMPARE (ftime (&curr), 0);
> +      TEST_VERIFY_EXIT (curr.time >= prev.time);
> +      if (curr.time == prev.time)
> +	TEST_VERIFY_EXIT (curr.millitm >= prev.millitm);
>  
>        if (curr.time > prev.time)
>          sec ++;
>      }
>    return 0;
>  }
> +#else
> +static int
> +do_test (void)
> +{
> +  return EXIT_UNSUPPORTED;
> +}
> +#endif
>  
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.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 3/3] linux: Add 64-bit time_t support for wait3

[Lukasz Majewski]

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

Hi Adhemerval,

> It basically calls the 64-bit time_t wait4 internal symbol.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  include/sys/resource.h          |  4 +++
>  sysdeps/unix/sysv/linux/wait3.c | 44
> +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+)
>  create mode 100644 sysdeps/unix/sysv/linux/wait3.c
> 
> diff --git a/include/sys/resource.h b/include/sys/resource.h
> index 64925f257c..2235b020fc 100644
> --- a/include/sys/resource.h
> +++ b/include/sys/resource.h
> @@ -138,12 +138,16 @@ libc_hidden_proto (__setrlimit);
>  #if __TIMESIZE == 64
>  # define __getrusage64 __getrusage
>  # define __wait4_time64 __wait4
> +# define __wait3_time64 __wait3
>  #else
>  extern int __getrusage64 (enum __rusage_who who, struct __rusage64
> *usage); libc_hidden_proto (__getrusage64)
>  extern pid_t __wait4_time64 (pid_t pid, int *stat_loc, int options,
>                               struct __rusage64 *usage);
>  libc_hidden_proto (__wait4_time64)
> +extern pid_t __wait3_time64 (int *stat_loc, int options,
> +                             struct __rusage64 *usage);
> +libc_hidden_proto (__wait3_time64)
>  #endif
>  #endif
>  #endif
> diff --git a/sysdeps/unix/sysv/linux/wait3.c
> b/sysdeps/unix/sysv/linux/wait3.c new file mode 100644
> index 0000000000..c05776f7ab
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/wait3.c
> @@ -0,0 +1,44 @@
> +/* Wait for process to change state, BSD style.  Linux version.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be
> useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <sys/wait.h>
> +#include <sys/resource.h>
> +#include <sys/types.h>
> +
> +pid_t
> +__wait3_time64 (int *stat_loc, int options, struct __rusage64 *usage)
> +{
> +  return __wait4_time64 (WAIT_ANY, stat_loc, options, usage);
> +}
> +#if __TIMESIZE != 64
> +libc_hidden_def (__wait3_time64)
> +
> +pid_t
> +__wait3 (int *stat_loc, int options, struct rusage *usage)
> +{
> +  struct __rusage64 usage64;
> +  pid_t ret = __wait3_time64 (stat_loc, options,
> +			      usage != NULL ? &usage64 : NULL);
> +  if (ret > 0 && usage != NULL)
> +     rusage64_to_rusage (&usage64, usage);
> +
> +  return ret;
> +}
> +#endif
> +
> +weak_alias (__wait3, wait3)

Thanks for converting this.

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

Side comment:
-------------

Now, I've just realized that struct __rusage64 (defined in non-exported
header: include/sys/resource.h) also needs to be exported for proper
Y2038 support (the same case as with struct __stat64_t64).


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 1/3] linux: Fix time64 support for futimesat

[Adhemerval Zanella via Libc-alpha]

[-- Attachment #1.1: Type: text/plain, Size: 1054 bytes --]



On 16/10/2020 05:41, Lukasz Majewski wrote:
> Hi Adhemerval,

>> diff --git a/sysdeps/unix/sysv/linux/utimensat.c
>> b/sysdeps/unix/sysv/linux/utimensat.c index ea23c2f051..72784d824a
>> 100644 --- a/sysdeps/unix/sysv/linux/utimensat.c
>> +++ b/sysdeps/unix/sysv/linux/utimensat.c
>> @@ -36,9 +36,13 @@ __utimensat64_helper (int fd, const char *file,
>>    if (ret == 0 || errno != ENOSYS)
>>      return ret;
>>  
>> +  /* For UTIME_NOW and UTIME_OMIT the value of tv_sec field is
>> ignored.  */ +# define NS_VALID(ns) \
>> +  ((((ns).tv_nsec == UTIME_NOW || (ns).tv_nsec == UTIME_OMIT) \
>> +   || in_time_t_range ((ns).tv_sec)))
> 
> IMHO, this is a bit misleading. The macro NS_VALID() seems to be
> supposed to check the nano seconds value,but it also check if tv_sec is
> in time_t range.
> 
> Maybe we can change its name (and passed argument) to e.g.
> #define TS_VALID(ts) ...

Fair enough, I changed to to TS_VALID.

> 
> Despite this minor comment:
> 
> Reviewed-by: Lukasz Majewski <lukma@denx.de>

Ack.


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

Re: [PATCH 3/3] linux: Add 64-bit time_t support for wait3

[Adhemerval Zanella via Libc-alpha]

[-- Attachment #1.1: Type: text/plain, Size: 517 bytes --]



On 16/10/2020 06:14, Lukasz Majewski wrote:
> Thanks for converting this.
> 
> Reviewed-by: Lukasz Majewski <lukma@denx.de>
> 
> Side comment:
> -------------
> 
> Now, I've just realized that struct __rusage64 (defined in non-exported
> header: include/sys/resource.h) also needs to be exported for proper
> Y2038 support (the same case as with struct __stat64_t64).

It should to enable getrusage, wait3, and wait4.  Although not really
related, I wonder if we should deprecate vtimes as well. 


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

[COMMITTED] Add NEWS entry for ftime compatibility move

[Adhemerval Zanella via Libc-alpha]

---
 NEWS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/NEWS b/NEWS
index e84c39aeb1..6eb577a669 100644
--- a/NEWS
+++ b/NEWS
@@ -37,6 +37,11 @@ Deprecated and removed features, and other changes affecting compatibility:
   implementations from HWCAP subdirectories are no longer loaded.
   Instead, the default implementation is used.
 
+* The deprecated <sys/timeb.h> header and the ftime function have been
+  removed.  To support old binaries, the ftime function continue to exist
+  as a compatibility symbol (on those architectures which had it).  All
+  programs should use gettimeofday or clock_gettime instead.
+
 Changes to build and runtime requirements:
 
 * On Linux, the system administrator needs to configure /dev/pts with
-- 
2.25.1

Re: [PATCH 3/3] linux: Add 64-bit time_t support for wait3

[Lukasz Majewski]

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

Hi Adhemerval,

> On 16/10/2020 06:14, Lukasz Majewski wrote:
> > Thanks for converting this.
> > 
> > Reviewed-by: Lukasz Majewski <lukma@denx.de>
> > 
> > Side comment:
> > -------------
> > 
> > Now, I've just realized that struct __rusage64 (defined in
> > non-exported header: include/sys/resource.h) also needs to be
> > exported for proper Y2038 support (the same case as with struct
> > __stat64_t64).  
> 
> It should to enable getrusage, wait3, and wait4. 

Yes, indeed.

> Although not really
> related, I wonder if we should deprecate vtimes as well. 
> 

As I've found following text in the vtimes/getrusage man page [1]:

"Ancient systems provided a vtimes() function with a similar purpose
to getrusage().  For backward compatibility, glibc also provides
vtimes().  All new applications should be written using getrusage()."

I think that we should. The Y2038 is a good occasion to do it (and
cleanup the glibc's code base) :-)


Links:
[1] - https://man7.org/linux/man-pages/man3/vtimes.3.html


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/3] Move ftime to a compatibility symbol

[Szabolcs Nagy via Libc-alpha]

The 10/15/2020 10:06, Adhemerval Zanella via Libc-alpha wrote:
> It was made deprecated on 2.31, so it moves to compat symbol after
> two releases.  It was also removed from exported symbol for riscv32
> (since ABI will be supported on for 2.33).
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  include/sys/timeb.h                           |  1 -
>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
>  time/Makefile                                 |  5 +-
>  time/ftime.c                                  | 19 +++++-
>  time/sys/timeb.h                              | 44 --------------
>  time/tst-ftime.c                              | 59 ++++++++++---------
>  6 files changed, 50 insertions(+), 79 deletions(-)
>  delete mode 100644 include/sys/timeb.h
>  delete mode 100644 time/sys/timeb.h

removing sys/timeb.h and ftime breaks several
spec2006 and spec2017 benchmarks.

which means this header is used in practice,
sometimes without configure checks.

is there a reason it cannot be supported?

Re: [PATCH 2/3] Move ftime to a compatibility symbol

[Andreas Schwab]

On Okt 19 2020, Szabolcs Nagy via Libc-alpha wrote:

> is there a reason it cannot be supported?

The interface has been removed from POSIX 12+ years ago.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH 2/3] Move ftime to a compatibility symbol

[Florian Weimer via Libc-alpha]

* Szabolcs Nagy via Libc-alpha:

> The 10/15/2020 10:06, Adhemerval Zanella via Libc-alpha wrote:
>> It was made deprecated on 2.31, so it moves to compat symbol after
>> two releases.  It was also removed from exported symbol for riscv32
>> (since ABI will be supported on for 2.33).
>> 
>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>> ---
>>  include/sys/timeb.h                           |  1 -
>>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
>>  time/Makefile                                 |  5 +-
>>  time/ftime.c                                  | 19 +++++-
>>  time/sys/timeb.h                              | 44 --------------
>>  time/tst-ftime.c                              | 59 ++++++++++---------
>>  6 files changed, 50 insertions(+), 79 deletions(-)
>>  delete mode 100644 include/sys/timeb.h
>>  delete mode 100644 time/sys/timeb.h
>
> removing sys/timeb.h and ftime breaks several
> spec2006 and spec2017 benchmarks.
>
> which means this header is used in practice,
> sometimes without configure checks.
>
> is there a reason it cannot be supported?

I think we should minimize the number of time-related symbols, in case
we ever want to provide a dual ABI (with multiple time_t sizes).

SPEC isn't going to change, so I don't think it makes sense to wait for
it getting fixed.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

RE: [PATCH 2/3] Move ftime to a compatibility symbol

[Tamar Christina via Libc-alpha]

> -----Original Message-----
> From: Libc-alpha <libc-alpha-bounces@sourceware.org> On Behalf Of Florian
> Weimer via Libc-alpha
> Sent: Monday, October 19, 2020 9:31 AM
> To: Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org>
> Subject: Re: [PATCH 2/3] Move ftime to a compatibility symbol
> 
> * Szabolcs Nagy via Libc-alpha:
> 
> > The 10/15/2020 10:06, Adhemerval Zanella via Libc-alpha wrote:
> >> It was made deprecated on 2.31, so it moves to compat symbol after
> >> two releases.  It was also removed from exported symbol for riscv32
> >> (since ABI will be supported on for 2.33).
> >>
> >> Checked on x86_64-linux-gnu and i686-linux-gnu.
> >> ---
> >>  include/sys/timeb.h                           |  1 -
> >>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
> >>  time/Makefile                                 |  5 +-
> >>  time/ftime.c                                  | 19 +++++-
> >>  time/sys/timeb.h                              | 44 --------------
> >>  time/tst-ftime.c                              | 59 ++++++++++---------
> >>  6 files changed, 50 insertions(+), 79 deletions(-)  delete mode
> >> 100644 include/sys/timeb.h  delete mode 100644 time/sys/timeb.h
> >
> > removing sys/timeb.h and ftime breaks several
> > spec2006 and spec2017 benchmarks.
> >
> > which means this header is used in practice, sometimes without
> > configure checks.
> >
> > is there a reason it cannot be supported?
> 
> I think we should minimize the number of time-related symbols, in case we
> ever want to provide a dual ABI (with multiple time_t sizes).
> 
> SPEC isn't going to change, so I don't think it makes sense to wait for it getting
> fixed.

That's a pretty hard line to draw.. Yes released version of spec are unlikely to change but
doesn't mean future versions won't. Has anyone ever pointed this out to the committee?

Regardless the issue now exists that we can't use glibc for benchmark anymore and I don't
really see a workaround.  Yes we can use older glibc versions but then what's the point,
we'll never see any of the improvements we make anymore.

Is there a viable workaround for this? Or do I have to now carry my own extra headers.

Regards,
Tamar

> 
> Thanks,
> Florian
> --
> Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
> Commercial register: Amtsgericht Muenchen, HRB 153243, Managing
> Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH 2/3] Move ftime to a compatibility symbol

[Florian Weimer via Libc-alpha]

* Tamar Christina:

>> -----Original Message-----
>> From: Libc-alpha <libc-alpha-bounces@sourceware.org> On Behalf Of Florian
>> Weimer via Libc-alpha
>> Sent: Monday, October 19, 2020 9:31 AM
>> To: Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org>
>> Subject: Re: [PATCH 2/3] Move ftime to a compatibility symbol
>> 
>> * Szabolcs Nagy via Libc-alpha:
>> 
>> > The 10/15/2020 10:06, Adhemerval Zanella via Libc-alpha wrote:
>> >> It was made deprecated on 2.31, so it moves to compat symbol after
>> >> two releases.  It was also removed from exported symbol for riscv32
>> >> (since ABI will be supported on for 2.33).
>> >>
>> >> Checked on x86_64-linux-gnu and i686-linux-gnu.
>> >> ---
>> >>  include/sys/timeb.h                           |  1 -
>> >>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
>> >>  time/Makefile                                 |  5 +-
>> >>  time/ftime.c                                  | 19 +++++-
>> >>  time/sys/timeb.h                              | 44 --------------
>> >>  time/tst-ftime.c                              | 59 ++++++++++---------
>> >>  6 files changed, 50 insertions(+), 79 deletions(-)  delete mode
>> >> 100644 include/sys/timeb.h  delete mode 100644 time/sys/timeb.h
>> >
>> > removing sys/timeb.h and ftime breaks several
>> > spec2006 and spec2017 benchmarks.
>> >
>> > which means this header is used in practice, sometimes without
>> > configure checks.
>> >
>> > is there a reason it cannot be supported?
>> 
>> I think we should minimize the number of time-related symbols, in case we
>> ever want to provide a dual ABI (with multiple time_t sizes).
>> 
>> SPEC isn't going to change, so I don't think it makes sense to wait
>> for it getting fixed.
>
> That's a pretty hard line to draw.. Yes released version of spec are
> unlikely to change but doesn't mean future versions won't. Has anyone
> ever pointed this out to the committee?

Has SPEC ever fixed any standards violations?  I don't have access to
the SPEC web site, so I don't know if they have released patches to fix
conformance bugs in benchmarks.  (I recall several discussions about
aliasing violations in older benchmarks on the GCC lists.)

> Regardless the issue now exists that we can't use glibc for benchmark
> anymore and I don't really see a workaround.  Yes we can use older
> glibc versions but then what's the point, we'll never see any of the
> improvements we make anymore.

How does SPEC run on platforms that do not have ftime?  Even on
UNIX-like systems, using ftime presently requires linking with some
compatibility library.  Is there really no knob to use gettimeofday or
clock_gettime instead of ftime?

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

RE: [PATCH 2/3] Move ftime to a compatibility symbol

[Tamar Christina via Libc-alpha]

> -----Original Message-----
> From: Florian Weimer <fweimer@redhat.com>
> Sent: Monday, October 19, 2020 10:51 AM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: libc-alpha@sourceware.org
> Subject: Re: [PATCH 2/3] Move ftime to a compatibility symbol
> 
> * Tamar Christina:
> 
> >> -----Original Message-----
> >> From: Libc-alpha <libc-alpha-bounces@sourceware.org> On Behalf Of
> >> Florian Weimer via Libc-alpha
> >> Sent: Monday, October 19, 2020 9:31 AM
> >> To: Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org>
> >> Subject: Re: [PATCH 2/3] Move ftime to a compatibility symbol
> >>
> >> * Szabolcs Nagy via Libc-alpha:
> >>
> >> > The 10/15/2020 10:06, Adhemerval Zanella via Libc-alpha wrote:
> >> >> It was made deprecated on 2.31, so it moves to compat symbol after
> >> >> two releases.  It was also removed from exported symbol for
> >> >> riscv32 (since ABI will be supported on for 2.33).
> >> >>
> >> >> Checked on x86_64-linux-gnu and i686-linux-gnu.
> >> >> ---
> >> >>  include/sys/timeb.h                           |  1 -
> >> >>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
> >> >>  time/Makefile                                 |  5 +-
> >> >>  time/ftime.c                                  | 19 +++++-
> >> >>  time/sys/timeb.h                              | 44 --------------
> >> >>  time/tst-ftime.c                              | 59 ++++++++++---------
> >> >>  6 files changed, 50 insertions(+), 79 deletions(-)  delete mode
> >> >> 100644 include/sys/timeb.h  delete mode 100644 time/sys/timeb.h
> >> >
> >> > removing sys/timeb.h and ftime breaks several
> >> > spec2006 and spec2017 benchmarks.
> >> >
> >> > which means this header is used in practice, sometimes without
> >> > configure checks.
> >> >
> >> > is there a reason it cannot be supported?
> >>
> >> I think we should minimize the number of time-related symbols, in
> >> case we ever want to provide a dual ABI (with multiple time_t sizes).
> >>
> >> SPEC isn't going to change, so I don't think it makes sense to wait
> >> for it getting fixed.
> >
> > That's a pretty hard line to draw.. Yes released version of spec are
> > unlikely to change but doesn't mean future versions won't. Has anyone
> > ever pointed this out to the committee?
> 
> Has SPEC ever fixed any standards violations?  I don't have access to the SPEC
> web site, so I don't know if they have released patches to fix conformance
> bugs in benchmarks.  (I recall several discussions about aliasing violations in
> older benchmarks on the GCC lists.)

They do, and have in the past, but while I can reasonably get one for SPEC2017,
SPEC2006 is no longer maintained as far as I am aware.

> 
> > Regardless the issue now exists that we can't use glibc for benchmark
> > anymore and I don't really see a workaround.  Yes we can use older
> > glibc versions but then what's the point, we'll never see any of the
> > improvements we make anymore.
> 
> How does SPEC run on platforms that do not have ftime?  Even on UNIX-like
> systems, using ftime presently requires linking with some compatibility
> library.  Is there really no knob to use gettimeofday or clock_gettime instead
> of ftime?

If the header was still there but empty I could do something at link time, but
since it doesn't exist at all I have to provide my own local headers.  Also if you
have a list of interfaces glibc plans to remove we can also feed this on to them
to avoid in the next SPEC release.  https://www.spec.org/cpuv8/

Most of the SPEC2017 benchmarks do the right thing with a include guard,
but xalancbmk doesn't, which was presumable taken directly from the
2006 version which also fails (along with h264 in 2006).

I will contact the committee but it seems highly unlikely that they would
Issue a fix for 2006.

Regards,
Tamar

> 
> Thanks,
> Florian
> --
> Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
> Commercial register: Amtsgericht Muenchen, HRB 153243, Managing
> Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH 2/3] Move ftime to a compatibility symbol

[Florian Weimer via Libc-alpha]

* Tamar Christina:

>> Has SPEC ever fixed any standards violations?  I don't have access to the SPEC
>> web site, so I don't know if they have released patches to fix conformance
>> bugs in benchmarks.  (I recall several discussions about aliasing violations in
>> older benchmarks on the GCC lists.)
>
> They do, and have in the past, but while I can reasonably get one for
> SPEC2017, SPEC2006 is no longer maintained as far as I am aware.

Here they refused it for SPEC2017:

| q1. Will SPEC fix spec_qsort.c?
| 
| a1. No. There are two reasons:
| 
|     That's the way it is in real life
|     The module spec_qsort.c is based directly on the BSD version, as updated Thu Mar 5 17:17:11 2015 UTC in revision 279666: https://svnweb.freebsd.org/base/head/lib/libc/stdlib/qsort.c?revision=279666.
|     No moving targets
|     SPEC CPU®2017 has already been released. Once source code has been released for a benchmark, SPEC strongly prefers not to change the source code unless there are compelling portability reasons.
|
| Note, therefore, that the patch attached to GCC bug 83201 is not approved by SPEC and would not be allowed in a reportable run.

<https://www.spec.org/cpu2017/Docs/benchmarks/505.mcf_r.html>

> If the header was still there but empty I could do something at link
> time, but since it doesn't exist at all I have to provide my own local
> headers.  Also if you have a list of interfaces glibc plans to remove
> we can also feed this on to them to avoid in the next SPEC release.
> https://www.spec.org/cpuv8/

There is no such list.  There isn't even consensus among glibc
developers that it's necessary to provide a deprecation notice before
removal.  Building with an appropriate version of _POSIX_SOURCE can help.

A good indicator is whether a function has been removed by POSIX.
Sometimes the manual pages also mark a function as obsolete.  Usually,
that's sufficient reason to stay away from it.  For ftime, this happened
some time before 2004.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH 2/3] Move ftime to a compatibility symbol

[Florian Weimer via Libc-alpha]

* Adhemerval Zanella via Libc-alpha:

> It was made deprecated on 2.31, so it moves to compat symbol after
> two releases.  It was also removed from exported symbol for riscv32
> (since ABI will be supported on for 2.33).

Why doesn't this break the conformance tests for
-D_POSIX_C_SOURCE=200112L?

Have we ever turned anything into a compatibility symbol that is
declared in a standard header?

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH 2/3] Move ftime to a compatibility symbol

[Andreas Schwab]

On Okt 19 2020, Florian Weimer via Libc-alpha wrote:

> * Adhemerval Zanella via Libc-alpha:
>
>> It was made deprecated on 2.31, so it moves to compat symbol after
>> two releases.  It was also removed from exported symbol for riscv32
>> (since ABI will be supported on for 2.33).
>
> Why doesn't this break the conformance tests for
> -D_POSIX_C_SOURCE=200112L?
>
> Have we ever turned anything into a compatibility symbol that is
> declared in a standard header?

I think it should be handled the same way as gets.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH 2/3] Move ftime to a compatibility symbol

[Florian Weimer via Libc-alpha]

* Andreas Schwab:

> On Okt 19 2020, Florian Weimer via Libc-alpha wrote:
>
>> * Adhemerval Zanella via Libc-alpha:
>>
>>> It was made deprecated on 2.31, so it moves to compat symbol after
>>> two releases.  It was also removed from exported symbol for riscv32
>>> (since ABI will be supported on for 2.33).
>>
>> Why doesn't this break the conformance tests for
>> -D_POSIX_C_SOURCE=200112L?
>>
>> Have we ever turned anything into a compatibility symbol that is
>> declared in a standard header?
>
> I think it should be handled the same way as gets.

Yes, that would make sense.  No compatibility symbol then.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH 2/3] Move ftime to a compatibility symbol

[Adhemerval Zanella via Libc-alpha]

On 19/10/2020 08:58, Florian Weimer via Libc-alpha wrote:
> * Andreas Schwab:
> 
>> On Okt 19 2020, Florian Weimer via Libc-alpha wrote:
>>
>>> * Adhemerval Zanella via Libc-alpha:
>>>
>>>> It was made deprecated on 2.31, so it moves to compat symbol after
>>>> two releases.  It was also removed from exported symbol for riscv32
>>>> (since ABI will be supported on for 2.33).
>>>
>>> Why doesn't this break the conformance tests for
>>> -D_POSIX_C_SOURCE=200112L?
>>>
>>> Have we ever turned anything into a compatibility symbol that is
>>> declared in a standard header?
>>
>> I think it should be handled the same way as gets.
> 
> Yes, that would make sense.  No compatibility symbol then.

We can move this to be handled as gets and only provide the symbol
for older _POSIX_C_SOURCE source, but it will still require all the
boilerplate to provide a y2038 safe interface. I don't have a strong
preference here, but even for -D_POSIX_C_SOURCE=200112L the interface
was marked as LEGACY.

Re: [PATCH 2/3] Move ftime to a compatibility symbol

[Adhemerval Zanella via Libc-alpha]

On 19/10/2020 09:30, Adhemerval Zanella wrote:
> 
> 
> On 19/10/2020 08:58, Florian Weimer via Libc-alpha wrote:
>> * Andreas Schwab:
>>
>>> On Okt 19 2020, Florian Weimer via Libc-alpha wrote:
>>>
>>>> * Adhemerval Zanella via Libc-alpha:
>>>>
>>>>> It was made deprecated on 2.31, so it moves to compat symbol after
>>>>> two releases.  It was also removed from exported symbol for riscv32
>>>>> (since ABI will be supported on for 2.33).
>>>>
>>>> Why doesn't this break the conformance tests for
>>>> -D_POSIX_C_SOURCE=200112L?
>>>>
>>>> Have we ever turned anything into a compatibility symbol that is
>>>> declared in a standard header?
>>>
>>> I think it should be handled the same way as gets.
>>
>> Yes, that would make sense.  No compatibility symbol then.
> 
> We can move this to be handled as gets and only provide the symbol
> for older _POSIX_C_SOURCE source, but it will still require all the
> boilerplate to provide a y2038 safe interface. I don't have a strong
> preference here, but even for -D_POSIX_C_SOURCE=200112L the interface
> was marked as LEGACY.
> 

It seems the least disruptive way, I will send a fix.

Re: [PATCH 2/3] Move ftime to a compatibility symbol

[Joseph Myers]

On Mon, 19 Oct 2020, Florian Weimer via Libc-alpha wrote:

> * Adhemerval Zanella via Libc-alpha:
> 
> > It was made deprecated on 2.31, so it moves to compat symbol after
> > two releases.  It was also removed from exported symbol for riscv32
> > (since ABI will be supported on for 2.33).
> 
> Why doesn't this break the conformance tests for
> -D_POSIX_C_SOURCE=200112L?

It does break them - if you use a compiler that doesn't find a previously 
installed copy of the header (for example, rerun the compilers stage of 
build-many-glibcs.py, not just the glibcs stage).

If a required feature from an old standard is deliberately unsupported, we 
may XFAIL the tests in conform/Makefile (as done for varargs.h, for 
example).

> Have we ever turned anything into a compatibility symbol that is
> declared in a standard header?

The regexp.h obsoletion is an example.  (The header was in UNIX98, but 
marked LEGACY there, and removed in the 2001 edition of POSIX.)

-- 
Joseph S. Myers
joseph@codesourcery.com