[PATCH] Add missing VDSO_{NAME,HASH}_* macros and use them for PREPARE_VERSION_KNOWN

[Tobias Klauser]

On 2019-05-24 at 16:42:32 +0200, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
> 
> On 24/05/2019 04:23, Tobias Klauser wrote:
> > Thank you for the review.
> > 
> > On 2019-05-23 at 21:36:01 +0200, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> >> On 16/05/2019 08:59, Tobias Klauser wrote:
> >>> Define all currently used Linux versions used for
> >>> PREPARE_VERSION{,_KNOWN} in sysdeps/unix/sysv/linux/dl-vdso.h and use
> >>> them instead of duplicating the versions and precomputed hashes across
> >>> architecture specific files.
> >>>
> >>> 2019-05-16  Tobias Klauser  <tklauser@distanz.ch>
> >>>
> >>> 	* sysdeps/unix/sysv/linux/aarch64/gettimeofday.c (INIT_ARCH): Use
> >>> 	PREPARE_VERSION_KNOWN.
> >>> 	* sysdeps/unix/sysv/linux/aarch64/init-first.c: Likewise.
> >>> 	* sysdeps/unix/sysv/linux/dl-vdso.h (VDSO_NAME_LINUX_2_6_39): New
> >>> 	define.
> >>> 	(VDSO_HASH_LINUX_2_6_39): Likewise.
> >>> 	(VDSO_NAME_LINUX_4_9): Likewise.
> >>> 	(VDSO_HASH_LINUX_4_9): Likewise.
> >>> 	* sysdeps/unix/sysv/linux/m68k/init-first.c (_libc_vdso_platform_setup):
> >>> 	Use PREPARE_VERSION_KNOWN.
> >>> 	* sysdeps/unix/sysv/linux/powerpc/gettimeofday.c (INIT_ARCH): Likewise.
> >>> 	* sysdeps/unix/sysv/linux/powerpc/init-first.c
> >>> 	(_libc_vdso_platform_setup): Likewise.
> >>> 	* sysdeps/unix/sysv/linux/powerpc/time.c (INIT_ARCH): Likewise.
> >>> 	* sysdeps/unix/sysv/linux/s390/init-first.c (_libc_vdso_platform_setup):
> >>> 	Likewise.
> >>> 	* sysdeps/unix/sysv/linux/x86_64/init-first.c (__vdso_platform_setup):
> >>> 	Likewise.
> >>
> >>
> >> LGTM, I assume you checked a build against affected architectures.  As a side
> >> note I think also should just remove PREPARE_VERSION macro and replace it with
> >> a more sane interface:
> > 
> > I (cross-)compile tested on all architectures and ran the test suite on
> > x86_64.
> > 
> >> --
> >> static inline struct r_found_version
> >> prepare_version_base (const char *name, ElfW(Word) hash)
> >> {
> >>   assert (hash == _dl_elf_hash (name));
> >>   return (struct r_found_version) { name, hash, 1, NULL };
> >> }
> >> #define prepare_version(vname) \
> >>   prepare_version_base (VDSO_NAME_##vname, VDSO_HASH_##vname)
> >> --
> > 
> > Should I add this to a v2 of this patch? Or should I send a follow-up
> > patch with the change?
> 
> I don't have a preference here. Refactor the vDSO code is something
> I have in mind for some time.

Sent it as a follow-up patch with Message-Id:
<20190524153941.10128-1-tklauser@distanz.ch>

And will send another follow-up for
sysdeps/unix/sysv/linux/aarch64/gettimeofday.c

> 
> > 
> >>> ---
> >>>  sysdeps/unix/sysv/linux/aarch64/gettimeofday.c | 4 ++--
> >>>  sysdeps/unix/sysv/linux/aarch64/init-first.c   | 4 ++--
> >>>  sysdeps/unix/sysv/linux/dl-vdso.h              | 4 ++++
> >>>  sysdeps/unix/sysv/linux/m68k/init-first.c      | 2 +-
> >>>  sysdeps/unix/sysv/linux/powerpc/gettimeofday.c | 4 ++--
> >>>  sysdeps/unix/sysv/linux/powerpc/init-first.c   | 2 +-
> >>>  sysdeps/unix/sysv/linux/powerpc/time.c         | 4 ++--
> >>>  sysdeps/unix/sysv/linux/s390/init-first.c      | 2 +-
> >>>  sysdeps/unix/sysv/linux/x86_64/init-first.c    | 2 +-
> >>>  9 files changed, 16 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c b/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c
> >>> index 6c008ed9357f..9180b50bf7c3 100644
> >>> --- a/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c
> >>> +++ b/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c
> >>> @@ -38,13 +38,13 @@ __gettimeofday_vsyscall (struct timeval *tv, struct timezone *tz)
> >>>    return INLINE_VSYSCALL (gettimeofday, 2, tv, tz);
> >>>  }
> >>>  
> >>> -/* PREPARE_VERSION will need an __LP64__ ifdef when ILP32 support
> >>> +/* PREPARE_VERSION_KNOWN will need an __LP64__ ifdef when ILP32 support
> >>>     goes in.  See _libc_vdso_platform_setup in
> >>>     sysdeps/unix/sysv/linux/aarch64/init-first.c.  */
> >>>  
> >>>  # undef INIT_ARCH
> >>>  # define INIT_ARCH() \
> >>> -	   PREPARE_VERSION (linux_version, "LINUX_2.6.39", 123718537); \
> >>> +	   PREPARE_VERSION_KNOWN (linux_version, LINUX_2_6_39); \
> >>>  	   void *vdso_gettimeofday = \
> >>>  	     _dl_vdso_vsym ("__kernel_gettimeofday", &linux_version);
> >>
> >> Also a side note, this vsdo setup is not required. The init-first.c
> >> already setups the VDSO_SYMBOL(gettimeofday). So a possible followup
> >> cleanup would be:
> >>
> >> --
> >> # undef INIT_ARCH
> >> # define INIT_ARCH()
> >>
> >> static inline void *
> >> gettimeofday_vdso (void)
> >> { 
> >>   __typeof (VDSO_SYMBOL(gettimeofday)) vdsop = VDSO_SYMBOL(gettimeofday);
> >>   PTR_DEMANGLE (vdsop);
> >>   return vdsop != NULL ? vdsop : (void *) __gettimeofday_vsyscall;
> >> }
> >>
> >> libc_ifunc_hidden (__redirect___gettimeofday, __gettimeofday,
> >>                    gettimeofday_vdso ())
> >> --
> > 
> > Thanks, will send a follow-up patch.
> > 
>