Re: [Patch] Use VDSO interface for gettimeofday on aarch64

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 16/05/2018 09:12, Szabolcs Nagy wrote:
> On 16/05/18 13:03, Szabolcs Nagy wrote:
>> On 16/05/18 12:51, Adhemerval Zanella wrote:
>>> On 16/05/2018 07:44, Szabolcs Nagy wrote:
>>>> The other thing that would be nice to document is that
>>>> why this change is safe for gettimeofday but not clock_gettime.
>>>> (former does not have to set errno other than EFAULT but that
>>>> case never works with vdso anyway, so the gettimeofday vdso
>>>> function is a complete implementation, while clock_gettime
>>>> has to deal with errno after the vdso call)
>>>
>>> As I put previously keep in mind that different that x86 and powerpc
>>> implementations, where the vDSO symbol does not fail; the arm64 vDSO
>>> implements a syscall fallback in case of underlying hardware requires
>>> an out-of-line counter access (arch_timer_enable_workaround).
>>>
>>> Using a ifunc accessors to call vDSO directly will result in a slight
>>> different semantic since generic implementation (kernel/time/time.c)
>>> might return EFAULT in some cases (which won't be handled by ifunc
>>> implementation).  This should not be an issue since POSIX [1] defines
>>> no error code should reserved for the symbol, but it might trigger
>>> some test in LTP.
>>>
>>
>> sorry i missed this comment,
>>
>> if the vdso can fail with -EFAULT then this optimization is not valid
>> (gettimeofday must either return 0 or -1).
>>
>> if userspace can test whatever the kernel is testing there, that
>> could be added to the ifunc resolver and only use vdso if we know
>> it does not fail (i don't know if this is possible).
>>
> 
> hm in posix EFAULT is not a requirement, it's probably
> undefined behaviour if gettimeofday is called with
> wrong pointers. (this is borderline since linux man
> pages document the EFAULT, but obviously that cannot
> work for vdso anyway)
> 
> then the optimization is ok, but we definitely need
> to document why we think directly going to the vdso
> is safe.

I think the optimization is fine as well, we had previous discussion about some
syscall semantics returning EFAULT [1] (which also lead to LTP test fixes [2]),
and the generic idea was since is UB to rely on EFAULT return code (for instance
a call to 'gettimeofday (0, 0)') it is fine to glibc also change semantic if it
finds suitable.

[1] https://sourceware.org/ml/libc-alpha/2017-10/msg00830.html
[2] https://github.com/linux-test-project/ltp/commit/259db6fed55f88ab32a0875e66803eee44d298be