[v4,1/2] y2038: linux: Provide __timerfd_gettime64 implementation

  This patch replaces auto generated wrapper (as described in
sysdeps/unix/sysv/linux/syscalls.list) for timerfd_gettime with one which
adds extra support for reading 64 bit time values on machines with
__TIMESIZE != 64.
There is no functional change for architectures already supporting 64 bit
time ABI.

This patch is conceptually identical to timer_gettime conversion already
done in sysdeps/unix/sysv/linux/timer_gettime.c.
Please refer to corresponding commit message for detailed description of
introduced functions and the testing procedure.

---
Changes for v4:
- Update date from 2019 to 2020

Changes for v3:
- Add missing libc_hidden_def()

Changes for v2:
- Remove "Contributed by" from the file header
- Remove early check for (fd < 0) in __timerfd_gettime64 as the fd
  correctness check is already done in Linux kernel
- Add single descriptive comment line to provide concise explanation
  of the code
---
 include/time.h                            |  3 +
 sysdeps/unix/sysv/linux/Makefile          |  3 +-
 sysdeps/unix/sysv/linux/syscalls.list     |  1 -
 sysdeps/unix/sysv/linux/timerfd_gettime.c | 68 +++++++++++++++++++++++
 4 files changed, 73 insertions(+), 2 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/timerfd_gettime.c
  

On 06/01/2020 09:17, Lukasz Majewski wrote:
> This patch replaces auto generated wrapper (as described in
> sysdeps/unix/sysv/linux/syscalls.list) for timerfd_gettime with one which
> adds extra support for reading 64 bit time values on machines with
> __TIMESIZE != 64.
> There is no functional change for architectures already supporting 64 bit
> time ABI.
> 
> This patch is conceptually identical to timer_gettime conversion already
> done in sysdeps/unix/sysv/linux/timer_gettime.c.
> Please refer to corresponding commit message for detailed description of
> introduced functions and the testing procedure.
> 
> ---
> Changes for v4:
> - Update date from 2019 to 2020
> 
> Changes for v3:
> - Add missing libc_hidden_def()
> 
> Changes for v2:
> - Remove "Contributed by" from the file header
> - Remove early check for (fd < 0) in __timerfd_gettime64 as the fd
>   correctness check is already done in Linux kernel
> - Add single descriptive comment line to provide concise explanation
>   of the code

LGTM when 2.32 opens, thanks.

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

> ---
>  include/time.h                            |  3 +
>  sysdeps/unix/sysv/linux/Makefile          |  3 +-
>  sysdeps/unix/sysv/linux/syscalls.list     |  1 -
>  sysdeps/unix/sysv/linux/timerfd_gettime.c | 68 +++++++++++++++++++++++
>  4 files changed, 73 insertions(+), 2 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/timerfd_gettime.c
> 
> diff --git a/include/time.h b/include/time.h
> index e5e8246eac..eb5082b4d7 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -181,9 +181,12 @@ libc_hidden_proto (__futimens64);
>  
>  #if __TIMESIZE == 64
>  # define __timer_gettime64 __timer_gettime
> +# define __timerfd_gettime64 __timerfd_gettime
>  #else
>  extern int __timer_gettime64 (timer_t timerid, struct __itimerspec64 *value);
> +extern int __timerfd_gettime64 (int fd, struct __itimerspec64 *value);
>  libc_hidden_proto (__timer_gettime64);
> +libc_hidden_proto (__timerfd_gettime64);
>  #endif
>  
>  #if __TIMESIZE == 64

Ok.

> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index f12b7b1a2d..74923740b9 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -60,7 +60,8 @@ sysdep_routines += adjtimex clone umount umount2 readahead \
>  		   setfsuid setfsgid epoll_pwait signalfd \
>  		   eventfd eventfd_read eventfd_write prlimit \
>  		   personality epoll_wait tee vmsplice splice \
> -		   open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get
> +		   open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get \
> +		   timerfd_gettime
>  
>  CFLAGS-gethostid.c = -fexceptions
>  CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables

Ok.

> diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
> index 5f1352ad43..adb9055ce2 100644
> --- a/sysdeps/unix/sysv/linux/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/syscalls.list
> @@ -94,7 +94,6 @@ mq_setattr	-	mq_getsetattr	i:ipp	mq_setattr
>  
>  timerfd_create	EXTRA	timerfd_create	i:ii	timerfd_create
>  timerfd_settime	EXTRA	timerfd_settime	i:iipp	timerfd_settime
> -timerfd_gettime	EXTRA	timerfd_gettime	i:ip	timerfd_gettime
>  
>  fanotify_init	EXTRA	fanotify_init	i:ii	fanotify_init
>  

Ok.

> diff --git a/sysdeps/unix/sysv/linux/timerfd_gettime.c b/sysdeps/unix/sysv/linux/timerfd_gettime.c
> new file mode 100644
> index 0000000000..7d09eeb11a
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/timerfd_gettime.c
> @@ -0,0 +1,68 @@
> +/* timerfd_gettime -- get the timer setting referred to by file descriptor.
> +   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; see the file COPYING.LIB.  If
> +   not, see <https://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <time.h>
> +#include <sysdep.h>
> +#include <kernel-features.h>
> +
> +int
> +__timerfd_gettime64 (int fd, struct __itimerspec64 *value)
> +{
> +#ifdef __ASSUME_TIME64_SYSCALLS
> +# ifndef __NR_timerfd_gettime64
> +#  define __NR_timerfd_gettime64 __NR_timerfd_gettime
> +# endif
> +  return INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value);
> +#else
> +# ifdef __NR_timerfd_gettime64
> +  int ret = INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value);
> +  if (ret == 0 || errno != ENOSYS)
> +    return ret;
> +# endif

Ok. 

As a side note, now that arch-syscall patch is upstream should we
assume that for !__ASSUME_TIME64_SYSCALLS the __NR_timerfd_gettime64
should be defined (meaning that Linux supports time64 for all 32-bit
architectures)?

> +  struct itimerspec its32;
> +  int retval = INLINE_SYSCALL_CALL (timerfd_gettime, fd, &its32);
> +  if (retval == 0)
> +    {
> +      value->it_interval = valid_timespec_to_timespec64 (its32.it_interval);
> +      value->it_value = valid_timespec_to_timespec64 (its32.it_value);
> +    }
> +
> +  return retval;
> +#endif
> +}


Ok.

> +
> +#if __TIMESIZE != 64
> +libc_hidden_def (__timerfd_gettime64)

Ok.

As a side note, we should fix it on clock_{get,set}time to add the missing
libc_hidden_def.

> +
> +int
> +__timerfd_gettime (int fd, struct itimerspec *value)
> +{
> +  struct __itimerspec64 its64;
> +  int retval = __timerfd_gettime64 (fd, &its64);
> +  if (retval == 0)
> +    {
> +      value->it_interval = valid_timespec64_to_timespec (its64.it_interval);
> +      value->it_value = valid_timespec64_to_timespec (its64.it_value);
> +    }
> +
> +  return retval;
> +}
> +#endif
> +strong_alias (__timerfd_gettime, timerfd_gettime)
> 

Ok.
  

Hi Adhemerval,

> On 06/01/2020 09:17, Lukasz Majewski wrote:
> > This patch replaces auto generated wrapper (as described in
> > sysdeps/unix/sysv/linux/syscalls.list) for timerfd_gettime with one
> > which adds extra support for reading 64 bit time values on machines
> > with __TIMESIZE != 64.
> > There is no functional change for architectures already supporting
> > 64 bit time ABI.
> > 
> > This patch is conceptually identical to timer_gettime conversion
> > already done in sysdeps/unix/sysv/linux/timer_gettime.c.
> > Please refer to corresponding commit message for detailed
> > description of introduced functions and the testing procedure.
> > 
> > ---
> > Changes for v4:
> > - Update date from 2019 to 2020
> > 
> > Changes for v3:
> > - Add missing libc_hidden_def()
> > 
> > Changes for v2:
> > - Remove "Contributed by" from the file header
> > - Remove early check for (fd < 0) in __timerfd_gettime64 as the fd
> >   correctness check is already done in Linux kernel
> > - Add single descriptive comment line to provide concise explanation
> >   of the code  
> 
> LGTM when 2.32 opens, thanks.

Ok. Now we do have the "freeze" (fixing period) for 2.31.

> 
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> 
> > ---
> >  include/time.h                            |  3 +
> >  sysdeps/unix/sysv/linux/Makefile          |  3 +-
> >  sysdeps/unix/sysv/linux/syscalls.list     |  1 -
> >  sysdeps/unix/sysv/linux/timerfd_gettime.c | 68
> > +++++++++++++++++++++++ 4 files changed, 73 insertions(+), 2
> > deletions(-) create mode 100644
> > sysdeps/unix/sysv/linux/timerfd_gettime.c
> > 
> > diff --git a/include/time.h b/include/time.h
> > index e5e8246eac..eb5082b4d7 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -181,9 +181,12 @@ libc_hidden_proto (__futimens64);
> >  
> >  #if __TIMESIZE == 64
> >  # define __timer_gettime64 __timer_gettime
> > +# define __timerfd_gettime64 __timerfd_gettime
> >  #else
> >  extern int __timer_gettime64 (timer_t timerid, struct
> > __itimerspec64 *value); +extern int __timerfd_gettime64 (int fd,
> > struct __itimerspec64 *value); libc_hidden_proto
> > (__timer_gettime64); +libc_hidden_proto (__timerfd_gettime64);
> >  #endif
> >  
> >  #if __TIMESIZE == 64  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/Makefile
> > b/sysdeps/unix/sysv/linux/Makefile index f12b7b1a2d..74923740b9
> > 100644 --- a/sysdeps/unix/sysv/linux/Makefile
> > +++ b/sysdeps/unix/sysv/linux/Makefile
> > @@ -60,7 +60,8 @@ sysdep_routines += adjtimex clone umount umount2
> > readahead \ setfsuid setfsgid epoll_pwait signalfd \
> >  		   eventfd eventfd_read eventfd_write prlimit \
> >  		   personality epoll_wait tee vmsplice splice \
> > -		   open_by_handle_at mlock2 pkey_mprotect pkey_set
> > pkey_get
> > +		   open_by_handle_at mlock2 pkey_mprotect pkey_set
> > pkey_get \
> > +		   timerfd_gettime
> >  
> >  CFLAGS-gethostid.c = -fexceptions
> >  CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/syscalls.list
> > b/sysdeps/unix/sysv/linux/syscalls.list index
> > 5f1352ad43..adb9055ce2 100644 ---
> > a/sysdeps/unix/sysv/linux/syscalls.list +++
> > b/sysdeps/unix/sysv/linux/syscalls.list @@ -94,7 +94,6 @@
> > mq_setattr	-	mq_getsetattr	i:ipp
> > mq_setattr timerfd_create	EXTRA	timerfd_create
> > i:ii	timerfd_create timerfd_settime	EXTRA
> > timerfd_settime	i:iipp	timerfd_settime
> > -timerfd_gettime	EXTRA	timerfd_gettime
> > i:ip	timerfd_gettime fanotify_init	EXTRA
> > fanotify_init	i:ii	fanotify_init 
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/timerfd_gettime.c
> > b/sysdeps/unix/sysv/linux/timerfd_gettime.c new file mode 100644
> > index 0000000000..7d09eeb11a
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/timerfd_gettime.c
> > @@ -0,0 +1,68 @@
> > +/* timerfd_gettime -- get the timer setting referred to by file
> > descriptor.
> > +   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; see the file COPYING.LIB.
> >  If
> > +   not, see <https://www.gnu.org/licenses/>.  */
> > +
> > +#include <errno.h>
> > +#include <stdlib.h>
> > +#include <time.h>
> > +#include <sysdep.h>
> > +#include <kernel-features.h>
> > +
> > +int
> > +__timerfd_gettime64 (int fd, struct __itimerspec64 *value)
> > +{
> > +#ifdef __ASSUME_TIME64_SYSCALLS
> > +# ifndef __NR_timerfd_gettime64
> > +#  define __NR_timerfd_gettime64 __NR_timerfd_gettime
> > +# endif
> > +  return INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value);
> > +#else
> > +# ifdef __NR_timerfd_gettime64
> > +  int ret = INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value);
> > +  if (ret == 0 || errno != ENOSYS)
> > +    return ret;
> > +# endif  
> 
> Ok. 
> 
> As a side note, now that arch-syscall patch is upstream should we
> assume that for !__ASSUME_TIME64_SYSCALLS the __NR_timerfd_gettime64
> should be defined (meaning that Linux supports time64 for all 32-bit
> architectures)?

Only Linux version >= 5.1 supports 64 bit time on archs with __WORDSIZE
= 32. I do guess (but I may be wrong here) that the arch-syscall is
supposed to reflect the exact syscalls provided by kernel headers used
for building (to help with validation of Y2038 patches).

> 
> > +  struct itimerspec its32;
> > +  int retval = INLINE_SYSCALL_CALL (timerfd_gettime, fd, &its32);
> > +  if (retval == 0)
> > +    {
> > +      value->it_interval = valid_timespec_to_timespec64
> > (its32.it_interval);
> > +      value->it_value = valid_timespec_to_timespec64
> > (its32.it_value);
> > +    }
> > +
> > +  return retval;
> > +#endif
> > +}  
> 
> 
> Ok.
> 
> > +
> > +#if __TIMESIZE != 64
> > +libc_hidden_def (__timerfd_gettime64)  
> 
> Ok.
> 
> As a side note, we should fix it on clock_{get,set}time to add the
> missing libc_hidden_def.

The clock_gettime already has libc_hidden_def. The difference is that we
use some compatibility code (after moving clock_gettime from librt to
libc) instead of strong_alias (as it mimics the behavior from auto
generated syscall wrapper).

> 
> > +
> > +int
> > +__timerfd_gettime (int fd, struct itimerspec *value)
> > +{
> > +  struct __itimerspec64 its64;
> > +  int retval = __timerfd_gettime64 (fd, &its64);
> > +  if (retval == 0)
> > +    {
> > +      value->it_interval = valid_timespec64_to_timespec
> > (its64.it_interval);
> > +      value->it_value = valid_timespec64_to_timespec
> > (its64.it_value);
> > +    }
> > +
> > +  return retval;
> > +}
> > +#endif
> > +strong_alias (__timerfd_gettime, timerfd_gettime)
> >   
> 
> Ok.

Thanks for the review.


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
  

On 07/01/2020 06:27, Lukasz Majewski wrote:

>> As a side note, now that arch-syscall patch is upstream should we
>> assume that for !__ASSUME_TIME64_SYSCALLS the __NR_timerfd_gettime64
>> should be defined (meaning that Linux supports time64 for all 32-bit
>> architectures)?
> 
> Only Linux version >= 5.1 supports 64 bit time on archs with __WORDSIZE
> = 32. I do guess (but I may be wrong here) that the arch-syscall is
> supposed to reflect the exact syscalls provided by kernel headers used
> for building (to help with validation of Y2038 patches).

The arch-syscall is now autogenerated from the latest kernel release
defined in build-many-glibcs.py. So the question is whether Linux
support and enforces time64 support on all and future 32-bit 
architectures or if there is still some missing ones (as it has
happen on some syscall additions, where some architecture lag
behind some releases).


> 
>>
>>> +  struct itimerspec its32;
>>> +  int retval = INLINE_SYSCALL_CALL (timerfd_gettime, fd, &its32);
>>> +  if (retval == 0)
>>> +    {
>>> +      value->it_interval = valid_timespec_to_timespec64
>>> (its32.it_interval);
>>> +      value->it_value = valid_timespec_to_timespec64
>>> (its32.it_value);
>>> +    }
>>> +
>>> +  return retval;
>>> +#endif
>>> +}  
>>
>>
>> Ok.
>>
>>> +
>>> +#if __TIMESIZE != 64
>>> +libc_hidden_def (__timerfd_gettime64)  
>>
>> Ok.
>>
>> As a side note, we should fix it on clock_{get,set}time to add the
>> missing libc_hidden_def.
> 
> The clock_gettime already has libc_hidden_def. The difference is that we
> use some compatibility code (after moving clock_gettime from librt to
> libc) instead of strong_alias (as it mimics the behavior from auto
> generated syscall wrapper).

I meant for the new time64 symbols.  Currently it is not an issue because
the internal time64 symbol is not exported and static linker uses the
internal __GI_ name for the symbol.  For instance, objdump -t on
clock_gettime.os on a 32-bit architecture (powerpc in this case) shows:

 00000144 g     F .text  00000088 __clock_gettime
 00000144 g     F .text  00000088 __clock_gettime_2
 00000000 g     F .text  00000144 .hidden __GI___clock_gettime64
 00000144 g     F .text  00000088 .hidden __GI___clock_gettime
 00000144 g     F .text  00000088 clock_gettime@@GLIBC_2.17
 00000144 g     F .text  00000088 clock_gettime@GLIBC_2.2

Where with a libc_hidden_def (__clock_gettime64) it shows:

 00000144 g     F .text  00000088 __clock_gettime
 00000144 g     F .text  00000088 __clock_gettime_2
 00000000 g     F .text  00000144 .hidden __GI___clock_gettime64
*00000000 g     F .text  00000144 __clock_gettime64
 00000144 g     F .text  00000088 .hidden __GI___clock_gettime
 00000144 g     F .text  00000088 clock_gettime@@GLIBC_2.17
 00000144 g     F .text  00000088 clock_gettime@GLIBC_2.2

The requirement of libc_hidden_def will de defined in the end if glibc
exports or not __clock_gettime64 on some header redirection or if 
clock_gettime64 would be suffice (with a {weak,strong}_alias).

However I do think we should fix it to avoid such confusion why there 
is a hidden_proto and not a hidden_def.
  

* Lukasz Majewski:

> Only Linux version >= 5.1 supports 64 bit time on archs with __WORDSIZE
> = 32. I do guess (but I may be wrong here) that the arch-syscall is
> supposed to reflect the exact syscalls provided by kernel headers used
> for building (to help with validation of Y2038 patches).

The tables are supposed to be complete in the sense that they include
all system calls which

(a) have a system call number assigned in some recent kernel version,
    and

(b) are actually used during the glibc build in some way.

The intent is that you can build glibc against older kernel headers (say
Linux 4.18) and still get the exact same system call profile as you
would get when building against Linux 5.3 (or Linux 4.18 with random
system call backports).

As Adhemerval pointed out, (a) can be problematic in the sense that
system call numbers could be added late on some architectures.  This
only matters if their existence actually affects the glibc build, as per
(b).  But if it does, we would have to update the tables.

Thanks,
Florian
  

Hi Adhemerval,

> On 07/01/2020 06:27, Lukasz Majewski wrote:
> 
> >> As a side note, now that arch-syscall patch is upstream should we
> >> assume that for !__ASSUME_TIME64_SYSCALLS the
> >> __NR_timerfd_gettime64 should be defined (meaning that Linux
> >> supports time64 for all 32-bit architectures)?  
> > 
> > Only Linux version >= 5.1 supports 64 bit time on archs with
> > __WORDSIZE = 32. I do guess (but I may be wrong here) that the
> > arch-syscall is supposed to reflect the exact syscalls provided by
> > kernel headers used for building (to help with validation of Y2038
> > patches).  
> 
> The arch-syscall is now autogenerated from the latest kernel release
> defined in build-many-glibcs.py. So the question is whether Linux
> support and enforces time64 support on all and future 32-bit 
> architectures or if there is still some missing ones (as it has
> happen on some syscall additions, where some architecture lag
> behind some releases).

This question would be best answered by Arnd (CC'ed) IMHO. From what I
know all 32 bit architectures gained syscalls covered by
__ASSUME_TIME64_SYSCALLS from Linux 5.1+.

The arch-syscall seems to me like a mean to test for example the time
related syscalls which use different versions (32bit time vs 64 bit) on
different archs. Notable example - clock_gettime(). Am I right?

> 
> 
> >   
> >>  
> >>> +  struct itimerspec its32;
> >>> +  int retval = INLINE_SYSCALL_CALL (timerfd_gettime, fd, &its32);
> >>> +  if (retval == 0)
> >>> +    {
> >>> +      value->it_interval = valid_timespec_to_timespec64
> >>> (its32.it_interval);
> >>> +      value->it_value = valid_timespec_to_timespec64
> >>> (its32.it_value);
> >>> +    }
> >>> +
> >>> +  return retval;
> >>> +#endif
> >>> +}    
> >>
> >>
> >> Ok.
> >>  
> >>> +
> >>> +#if __TIMESIZE != 64
> >>> +libc_hidden_def (__timerfd_gettime64)    
> >>
> >> Ok.
> >>
> >> As a side note, we should fix it on clock_{get,set}time to add the
> >> missing libc_hidden_def.  
> > 
> > The clock_gettime already has libc_hidden_def. The difference is
> > that we use some compatibility code (after moving clock_gettime
> > from librt to libc) instead of strong_alias (as it mimics the
> > behavior from auto generated syscall wrapper).  
> 
> I meant for the new time64 symbols.  Currently it is not an issue
> because the internal time64 symbol is not exported and static linker
> uses the internal __GI_ name for the symbol.  For instance, objdump
> -t on clock_gettime.os on a 32-bit architecture (powerpc in this
> case) shows:
> 
>  00000144 g     F .text  00000088 __clock_gettime
>  00000144 g     F .text  00000088 __clock_gettime_2
>  00000000 g     F .text  00000144 .hidden __GI___clock_gettime64
>  00000144 g     F .text  00000088 .hidden __GI___clock_gettime
>  00000144 g     F .text  00000088 clock_gettime@@GLIBC_2.17
>  00000144 g     F .text  00000088 clock_gettime@GLIBC_2.2
> 
> Where with a libc_hidden_def (__clock_gettime64) it shows:
> 
>  00000144 g     F .text  00000088 __clock_gettime
>  00000144 g     F .text  00000088 __clock_gettime_2
>  00000000 g     F .text  00000144 .hidden __GI___clock_gettime64
> *00000000 g     F .text  00000144 __clock_gettime64
>  00000144 g     F .text  00000088 .hidden __GI___clock_gettime
>  00000144 g     F .text  00000088 clock_gettime@@GLIBC_2.17
>  00000144 g     F .text  00000088 clock_gettime@GLIBC_2.2
> 
> The requirement of libc_hidden_def will de defined in the end if glibc
> exports or not __clock_gettime64 on some header redirection or if 

The __clock_gettime64 is going to be exported (as clock_gettime
redirection) on 32 bit archs which are going to be Y2038 safe (with 64
bit time_t).

> clock_gettime64 would be suffice (with a {weak,strong}_alias).
> 

The internal in-glibc usage (calling) of clock_gettime() shall be
replaced by either __clock_gettime64 or clock_gettime64. I would prefer
the former as it reflects that it is internal function (with __ prefix).

> However I do think we should fix it to avoid such confusion why there 
> is a hidden_proto and not a hidden_def.

+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
  

On Tue, Jan 7, 2020 at 3:25 PM Lukasz Majewski <lukma@denx.de> wrote:
> > On 07/01/2020 06:27, Lukasz Majewski wrote:
> >
> > >> As a side note, now that arch-syscall patch is upstream should we
> > >> assume that for !__ASSUME_TIME64_SYSCALLS the
> > >> __NR_timerfd_gettime64 should be defined (meaning that Linux
> > >> supports time64 for all 32-bit architectures)?
> > >
> > > Only Linux version >= 5.1 supports 64 bit time on archs with
> > > __WORDSIZE = 32. I do guess (but I may be wrong here) that the
> > > arch-syscall is supposed to reflect the exact syscalls provided by
> > > kernel headers used for building (to help with validation of Y2038
> > > patches).
> >
> > The arch-syscall is now autogenerated from the latest kernel release
> > defined in build-many-glibcs.py. So the question is whether Linux
> > support and enforces time64 support on all and future 32-bit
> > architectures or if there is still some missing ones (as it has
> > happen on some syscall additions, where some architecture lag
> > behind some releases).
>
> This question would be best answered by Arnd (CC'ed) IMHO. From what I
> know all 32 bit architectures gained syscalls covered by
> __ASSUME_TIME64_SYSCALLS from Linux 5.1+.

Yes, we intentionally converted all architectures at the same time to
have a reliable baseline, i.e. once a future glibc requires linux-5.1 as
the minimum kernel all the backwards-compatibility support for
old kernels can be dropped.

New 32-bit architectures (if any) will only support the time64 syscalls
and not time time32 ones.

For some ioctl interfaces, you also need to use the latest kernel
headers, e.g. sound/asound.h from kernels before 5.6 has some
bugs with time64. For the ioctl implementation I hope to wrap
up the final bits in linux-5.6 as well, earlier kernels may return
-EINVAL on some of the ioctls that pass a time_t.

        Arnd
  

On 07/01/2020 11:25, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 07/01/2020 06:27, Lukasz Majewski wrote:
>>
>>>> As a side note, now that arch-syscall patch is upstream should we
>>>> assume that for !__ASSUME_TIME64_SYSCALLS the
>>>> __NR_timerfd_gettime64 should be defined (meaning that Linux
>>>> supports time64 for all 32-bit architectures)?  
>>>
>>> Only Linux version >= 5.1 supports 64 bit time on archs with
>>> __WORDSIZE = 32. I do guess (but I may be wrong here) that the
>>> arch-syscall is supposed to reflect the exact syscalls provided by
>>> kernel headers used for building (to help with validation of Y2038
>>> patches).  
>>
>> The arch-syscall is now autogenerated from the latest kernel release
>> defined in build-many-glibcs.py. So the question is whether Linux
>> support and enforces time64 support on all and future 32-bit 
>> architectures or if there is still some missing ones (as it has
>> happen on some syscall additions, where some architecture lag
>> behind some releases).
> 
> This question would be best answered by Arnd (CC'ed) IMHO. From what I
> know all 32 bit architectures gained syscalls covered by
> __ASSUME_TIME64_SYSCALLS from Linux 5.1+.
> 
> The arch-syscall seems to me like a mean to test for example the time
> related syscalls which use different versions (32bit time vs 64 bit) on
> different archs. Notable example - clock_gettime(). Am I right?

The arch-syscall is a way to decouple the build from the kernel header
used on build, which might simplify the logic to use some kernel 
features.

On the clock_gettime, for instance, as Arnd has indicated we can
assume that __NR_clock_gettime64 will be always presented for
!__ASSUME_TIME64_SYSCALLS.

It would be interesting if kernel also could enforce that new
generic syscalls would be wire-up, or at least the syscall number
reserved; once a new generic syscall is introduced.  It would
simplify the __ASSUME_* macro, not requiring the arch-specific
overrides on some architectures.

> 
> The __clock_gettime64 is going to be exported (as clock_gettime
> redirection) on 32 bit archs which are going to be Y2038 safe (with 64
> bit time_t).
> 
>> clock_gettime64 would be suffice (with a {weak,strong}_alias).
>>
> 
> The internal in-glibc usage (calling) of clock_gettime() shall be
> replaced by either __clock_gettime64 or clock_gettime64. I would prefer
> the former as it reflects that it is internal function (with __ prefix).

It required to be the former because we also need to take in consideration
linking namespace pollution. 

> 
>> However I do think we should fix it to avoid such confusion why there 
>> is a hidden_proto and not a hidden_def.
> 
> +1.

Ack, I will send a patch.
  

On Tue, Jan 7, 2020 at 9:16 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 07/01/2020 11:25, Lukasz Majewski wrote:
> >> On 07/01/2020 06:27, Lukasz Majewski wrote:
> >>
> >>>> As a side note, now that arch-syscall patch is upstream should we
> >>>> assume that for !__ASSUME_TIME64_SYSCALLS the
> >>>> __NR_timerfd_gettime64 should be defined (meaning that Linux
> >>>> supports time64 for all 32-bit architectures)?
> >>>
> >>> Only Linux version >= 5.1 supports 64 bit time on archs with
> >>> __WORDSIZE = 32. I do guess (but I may be wrong here) that the
> >>> arch-syscall is supposed to reflect the exact syscalls provided by
> >>> kernel headers used for building (to help with validation of Y2038
> >>> patches).
> >>
> >> The arch-syscall is now autogenerated from the latest kernel release
> >> defined in build-many-glibcs.py. So the question is whether Linux
> >> support and enforces time64 support on all and future 32-bit
> >> architectures or if there is still some missing ones (as it has
> >> happen on some syscall additions, where some architecture lag
> >> behind some releases).
> >
> > This question would be best answered by Arnd (CC'ed) IMHO. From what I
> > know all 32 bit architectures gained syscalls covered by
> > __ASSUME_TIME64_SYSCALLS from Linux 5.1+.
> >
> > The arch-syscall seems to me like a mean to test for example the time
> > related syscalls which use different versions (32bit time vs 64 bit) on
> > different archs. Notable example - clock_gettime(). Am I right?
>
> The arch-syscall is a way to decouple the build from the kernel header
> used on build, which might simplify the logic to use some kernel
> features.
>
> On the clock_gettime, for instance, as Arnd has indicated we can
> assume that __NR_clock_gettime64 will be always presented for
> !__ASSUME_TIME64_SYSCALLS.
>
> It would be interesting if kernel also could enforce that new
> generic syscalls would be wire-up, or at least the syscall number
> reserved; once a new generic syscall is introduced.  It would
> simplify the __ASSUME_* macro, not requiring the arch-specific
> overrides on some architectures.

We currently try to enforce it through review since the introduction
of the automatically generated asm/unistd.h header files, but his
has already failed one time for the recent clone3 system call that
accidentally was missing on one architecture.

I have some plans to enforce it using tooling, but it should
at least be a safe assumption that all new system calls have
the same numbers across all architectures and are added at
the same time.

I also have some vague plans to automatically generate not
only the asm/unistd.h header but also some trivial wrappers
around syscall() that provide an inline function with the correct
arguments, to allow calling any syscall without relying on libc
to provide a wrapper for it.

          Arnd
  

Hi Adhemerval,

> On 07/01/2020 11:25, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 07/01/2020 06:27, Lukasz Majewski wrote:
> >>  
> >>>> As a side note, now that arch-syscall patch is upstream should we
> >>>> assume that for !__ASSUME_TIME64_SYSCALLS the
> >>>> __NR_timerfd_gettime64 should be defined (meaning that Linux
> >>>> supports time64 for all 32-bit architectures)?    
> >>>
> >>> Only Linux version >= 5.1 supports 64 bit time on archs with
> >>> __WORDSIZE = 32. I do guess (but I may be wrong here) that the
> >>> arch-syscall is supposed to reflect the exact syscalls provided by
> >>> kernel headers used for building (to help with validation of Y2038
> >>> patches).    
> >>
> >> The arch-syscall is now autogenerated from the latest kernel
> >> release defined in build-many-glibcs.py. So the question is
> >> whether Linux support and enforces time64 support on all and
> >> future 32-bit architectures or if there is still some missing ones
> >> (as it has happen on some syscall additions, where some
> >> architecture lag behind some releases).  
> > 
> > This question would be best answered by Arnd (CC'ed) IMHO. From
> > what I know all 32 bit architectures gained syscalls covered by
> > __ASSUME_TIME64_SYSCALLS from Linux 5.1+.
> > 
> > The arch-syscall seems to me like a mean to test for example the
> > time related syscalls which use different versions (32bit time vs
> > 64 bit) on different archs. Notable example - clock_gettime(). Am I
> > right?  
> 
> The arch-syscall is a way to decouple the build from the kernel header
> used on build, 

So then we will build against the newest kernel (like 5.4 now). As it
was noted in the other thread - this would simplify the
build-many-glibcs.py

> which might simplify the logic to use some kernel 
> features.

I must admit that I do not see such simplification... Could you give an
example?

> 
> On the clock_gettime, for instance, as Arnd has indicated we can
> assume that __NR_clock_gettime64 will be always presented for
> !__ASSUME_TIME64_SYSCALLS.
> 
> It would be interesting if kernel also could enforce that new
> generic syscalls would be wire-up, or at least the syscall number
> reserved; once a new generic syscall is introduced.  It would
> simplify the __ASSUME_* macro, not requiring the arch-specific
> overrides on some architectures.
> 
> > 
> > The __clock_gettime64 is going to be exported (as clock_gettime
> > redirection) on 32 bit archs which are going to be Y2038 safe (with
> > 64 bit time_t).
> >   
> >> clock_gettime64 would be suffice (with a {weak,strong}_alias).
> >>  
> > 
> > The internal in-glibc usage (calling) of clock_gettime() shall be
> > replaced by either __clock_gettime64 or clock_gettime64. I would
> > prefer the former as it reflects that it is internal function (with
> > __ prefix).  
> 
> It required to be the former because we also need to take in
> consideration linking namespace pollution. 
> 
> >   
> >> However I do think we should fix it to avoid such confusion why
> >> there is a hidden_proto and not a hidden_def.  
> > 
> > +1.  
> 
> Ack, I will send a patch.

Thanks.


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