Re: [PATCH 11/12] linux: Consolidate time implementation

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 28/06/2019 19:45, Tulio Magno Quites Machado Filho wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> 
>> The time syscall has currently 3 possible implementations:
>>
>>    1. Wire-up __NR_time with a vDSO implementation (currently only x86 and
>>       powerpc).
>>    2. Wire-up __NR_time (hppa, i686, m68k, microblaze, mips, powerpc,
>>       powerpc64, s390, sh4, sparcv9, x86_64).
>>    3. Using internal gettimeofday (aarch64, alpha, arm, mips64, mips64-n32,
>>       nios2, s390x, sparc64).
>>
>> This patch consolidates all implementation on Linux generic
>> sysdeps/unix/sysv/linux/time.c.  To simplify the code, some changes are
>> made:
>>
>>    * The wire-up with vDSO implementation route external calls directly
>>      to vDSO through IFUNC.  To enable it the architecture need to
>>      explicit define USE_TIME_VSYSCALL_IFUNC.
>>
>>    * Also, powerpc and x86 tries to route internal time usages to
>>      IFUNC mechanism, which is problematic since powerpc32 and i686 does
>>      not really support it.  Instead, all internal calls are routed to
>>      a default internal symbol which in turn calls INTERNAL_VSYSCALL.
>>
>>    * Static linking also uses the fallback mechanism which calls
>>      INTERNAL_VSYSCALL, so vDSO is used for this case as well.
>>
>> The generic implementation issues a syscall as default, calls the
>> vDSO if the architecture defines HAVE_TIME_VSYSCALL, and route the external
>> calls to iFUNC if the architecture also defines USE_TIME_VSYSCALL_IFUNC.
>>
>> Checked on x86_64-linux-gnu, i686-linux-gnu, powerpc-linux-gnu,
>> powerpc64-linux-gnu, powerpc64le-linux-gnu, and aarch64-linux-gnu.
> 
> This patch is causing segmentation faults in powerpc64, e.g.
> $ ./testrun.sh ./io/test-utime
> Segmentation fault
> 

Indeed when I had to refactor the 'powerpc: Refactor vsyscall internal macros'
I forgot to update both the gettimeofday and time one, which required some
changes as well.  I will send an updated version.

>> diff --git a/sysdeps/unix/sysv/linux/time.c b/sysdeps/unix/sysv/linux/time.c
>> index 1978f6d817..93e036fe16 100644
>> --- a/sysdeps/unix/sysv/linux/time.c
>> +++ b/sysdeps/unix/sysv/linux/time.c
>> ...
>> +#if HAVE_IFUNC && defined USE_TIME_VSYSCALL_IFUNC
>> +/* Route externals calls direct to vDSO and static and internal calls to
>> +   fallback implementation (which also might call the vDSO).  */
>> +# ifdef SHARED
>> +#  define INIT_ARCH()
>> +libc_ifunc_redirected (__redirect_time, time,
>> +		       get_vdso_symbol (HAVE_TIME_VSYSCALL)
>> +		       ?: time_syscall);
> 
> Missing VDSO_IFUNC_RET ?
>