This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

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

From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
To: Tobias Klauser <tklauser at distanz dot ch>, libc-alpha at sourceware dot org
Date: Thu, 23 May 2019 16:36:01 -0300
Subject: Re: [PATCH] Add missing VDSO_{NAME,HASH}_* macros and use them for PREPARE_VERSION_KNOWN
References: <20190516115924.29027-1-tklauser@distanz.ch>


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:

--
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)
--

> ---
>  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 ())
--

>  
> diff --git a/sysdeps/unix/sysv/linux/aarch64/init-first.c b/sysdeps/unix/sysv/linux/aarch64/init-first.c
> index ce7319f3c95b..80f7ed91ef0a 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/init-first.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/init-first.c
> @@ -27,9 +27,9 @@ static inline void
>  _libc_vdso_platform_setup (void)
>  {
>  #ifdef __LP64__
> -  PREPARE_VERSION (linux_version, "LINUX_2.6.39", 123718537);
> +  PREPARE_VERSION_KNOWN (linux_version, LINUX_2_6_39);
>  #else
> -  PREPARE_VERSION (linux_version, "LINUX_4.9", 61765625);
> +  PREPARE_VERSION_KNOWN (linux_version, LINUX_4_9);
>  #endif
>  
>    void *p = _dl_vdso_vsym ("__kernel_gettimeofday", &linux_version);

Ok.

> diff --git a/sysdeps/unix/sysv/linux/dl-vdso.h b/sysdeps/unix/sysv/linux/dl-vdso.h
> index 50ee29067d0b..9e61ca742363 100644
> --- a/sysdeps/unix/sysv/linux/dl-vdso.h
> +++ b/sysdeps/unix/sysv/linux/dl-vdso.h
> @@ -44,6 +44,10 @@
>  #define VDSO_HASH_LINUX_2_6_15	123718565
>  #define VDSO_NAME_LINUX_2_6_29	"LINUX_2.6.29"
>  #define VDSO_HASH_LINUX_2_6_29	123718585
> +#define VDSO_NAME_LINUX_2_6_39	"LINUX_2.6.39"
> +#define VDSO_HASH_LINUX_2_6_39	123718537
> +#define VDSO_NAME_LINUX_4_9	"LINUX_4.9"
> +#define VDSO_HASH_LINUX_4_9	61765625
>  #define VDSO_NAME_LINUX_4_15	"LINUX_4.15"
>  #define VDSO_HASH_LINUX_4_15	182943605
>  

Ok.

> diff --git a/sysdeps/unix/sysv/linux/m68k/init-first.c b/sysdeps/unix/sysv/linux/m68k/init-first.c
> index f8f09108393d..53caba218cfa 100644
> --- a/sysdeps/unix/sysv/linux/m68k/init-first.c
> +++ b/sysdeps/unix/sysv/linux/m68k/init-first.c
> @@ -29,7 +29,7 @@ _libc_vdso_platform_setup (void)
>  {
>    void *p;
>  
> -  PREPARE_VERSION (linux26, "LINUX_2.6", 61765110);
> +  PREPARE_VERSION_KNOWN (linux26, LINUX_2_6);
>  
>    /* It may happen that rtld didn't initialize the vDSO, so fallback
>       to the syscall implementations if _dl_vdso_vsym returns NULL.

Ok.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
> index c8d7790d8adc..463b678ad935 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
> @@ -57,8 +57,8 @@ __gettimeofday_syscall (struct timeval *tv, struct timezone *tz)
>    return INLINE_SYSCALL (gettimeofday, 2, tv, tz);
>  }
>  
> -# define INIT_ARCH()							\
> -  PREPARE_VERSION (linux2615, "LINUX_2.6.15", 123718565);		\
> +# define INIT_ARCH()						\
> +  PREPARE_VERSION_KNOWN (linux2615, LINUX_2_6_15);		\
>    void *vdso_gettimeofday = _dl_vdso_vsym ("__kernel_gettimeofday", &linux2615);
>  
>  /* If the vDSO is not available we fall back syscall.  */

Ok.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/init-first.c b/sysdeps/unix/sysv/linux/powerpc/init-first.c
> index 237b5abbf9b0..831f910788b3 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/init-first.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/init-first.c
> @@ -37,7 +37,7 @@ void *VDSO_SYMBOL(sigtramp_rt32);
>  static inline void
>  _libc_vdso_platform_setup (void)
>  {
> -  PREPARE_VERSION (linux2615, "LINUX_2.6.15", 123718565);
> +  PREPARE_VERSION_KNOWN (linux2615, LINUX_2_6_15);
>  
>    void *p = _dl_vdso_vsym ("__kernel_gettimeofday", &linux2615);
>    PTR_MANGLE (p);

Ok.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/time.c b/sysdeps/unix/sysv/linux/powerpc/time.c
> index c2f171b6228c..cb3e8b9a73a5 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/time.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/time.c
> @@ -66,8 +66,8 @@ time_syscall (time_t *t)
>    return result;
>  }
>  
> -# define INIT_ARCH()							\
> -  PREPARE_VERSION (linux2615, "LINUX_2.6.15", 123718565);		\
> +# define INIT_ARCH()						\
> +  PREPARE_VERSION_KNOWN (linux2615, LINUX_2_6_15);		\
>    void *vdso_time = _dl_vdso_vsym ("__kernel_time", &linux2615);
>  
>  /* If the vDSO is not available we fall back to the syscall.  */

Ok.

> diff --git a/sysdeps/unix/sysv/linux/s390/init-first.c b/sysdeps/unix/sysv/linux/s390/init-first.c
> index 71dd43f738bf..1f46e8052a37 100644
> --- a/sysdeps/unix/sysv/linux/s390/init-first.c
> +++ b/sysdeps/unix/sysv/linux/s390/init-first.c
> @@ -35,7 +35,7 @@ long int (*VDSO_SYMBOL(getcpu)) (unsigned *, unsigned *, void *)
>  static inline void
>  _libc_vdso_platform_setup (void)
>  {
> -  PREPARE_VERSION (linux2629, "LINUX_2.6.29", 123718585);
> +  PREPARE_VERSION_KNOWN (linux2629, LINUX_2_6_29);
>  
>    void *p = _dl_vdso_vsym ("__kernel_gettimeofday", &linux2629);
>    PTR_MANGLE (p);

Ok.

> diff --git a/sysdeps/unix/sysv/linux/x86_64/init-first.c b/sysdeps/unix/sysv/linux/x86_64/init-first.c
> index 6a347becbe0d..85333e5ff309 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/init-first.c
> +++ b/sysdeps/unix/sysv/linux/x86_64/init-first.c
> @@ -32,7 +32,7 @@ extern __typeof (clock_gettime) __syscall_clock_gettime attribute_hidden;
>  static inline void
>  __vdso_platform_setup (void)
>  {
> -  PREPARE_VERSION (linux26, "LINUX_2.6", 61765110);
> +  PREPARE_VERSION_KNOWN (linux26, LINUX_2_6);
>  
>    void *p = _dl_vdso_vsym ("__vdso_clock_gettime", &linux26);
>    if (p == NULL)
> 

Ok.

Follow-Ups:
- Re: [PATCH] Add missing VDSO_{NAME,HASH}_* macros and use them for PREPARE_VERSION_KNOWN
  - From: Tobias Klauser

References:
- [PATCH] Add missing VDSO_{NAME,HASH}_* macros and use them for PREPARE_VERSION_KNOWN
  - From: Tobias Klauser

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]