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

[Lukasz Majewski <lukma at denx dot de>]

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

Attachment: pgpSYUo1Tpkaq.pgp
Description: OpenPGP digital signature