This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Add missing VDSO_{NAME,HASH}_* macros and use them for PREPARE_VERSION_KNOWN
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?
> > ---
> > 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.