Re: [PATCH 5/7] tunables: Use glibc.tune.hwcap_mask tunable instead of _dl_hwcap_mask

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

On 11/05/2017 11:51, Siddhesh Poyarekar wrote:
> Drop _dl_hwcap_mask when building with tunables.  This completes the
> transition of hwcap_mask reading from _dl_hwcap_mask to tunables.
> 
> 	* elf/dl-cache.c: Include dl-tunables.h.
> 	(_dl_load_cache_lookup)[HAVE_TUNABLES]: Read
> 	glibc.tune.hwcap_mask.
> 	* elf/dl-hwcaps.c: Include dl-tunables.h.
> 	(_dl_important_hwcaps)[HAVE_TUNABLES]: Read and update
> 	glibc.tune.hwcap_mask.
> 	* sysdeps/sparc/sparc32/dl-machine.h: Likewise.
> 	* elf/dl-support.c (_dl_hwcap2)[HAVE_TUNABLES]: Drop
> 	_dl_hwcap_mask.
> 	* elf/dl-tunables.c (__tunable_set_val): Make a hidden alias.
> 	* elf/dl-tunables.h (__tunable_set_val): Likewise.
> 	* elf/rtld.c (rtld_global_ro)[HAVE_TUNABLES]: Drop
> 	_dl_hwcap_mask.
> 	(process_envvars)[HAVE_TUNABLES]: Likewise.
> 	* sysdeps/generic/ldsodefs.h (rtld_global_ro)[HAVE_TUNABLES]:
> 	Likewise.
> 	* sysdeps/x86/cpu-features.c (init_cpu_features): Don't
> 	initialize dl_hwcap_mask when tunables are enabled.

I would recommend to incorporate the fix for LD_HWCAP_MASK on tst-end-setuid
you sent before [1] to avoid this fail on some configuration (mine for
instance).  Also I think we should open a bugzilla to iron out the
elf/dl-hwcaps.c allocation scheme which is triggering this issue.

[1] https://sourceware.org/ml/libc-alpha/2017-05/msg00340.html


> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index f26a8b2..695ac24 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -515,8 +515,10 @@ struct rtld_global_ro
>    /* Mask for hardware capabilities that are available.  */
>    EXTERN uint64_t _dl_hwcap;
>  
> +#if !HAVE_TUNABLES
>    /* Mask for important hardware capabilities we honour. */
>    EXTERN uint64_t _dl_hwcap_mask;
> +#endif
>  
>  #ifdef HAVE_AUX_VECTOR
>    /* Pointer to the auxv list supplied to the program at startup.  */
> diff --git a/sysdeps/sparc/sparc32/dl-machine.h b/sysdeps/sparc/sparc32/dl-machine.h
> index cf7272f..6923e47 100644
> --- a/sysdeps/sparc/sparc32/dl-machine.h
> +++ b/sysdeps/sparc/sparc32/dl-machine.h
> @@ -39,7 +39,12 @@ elf_machine_matches_host (const Elf32_Ehdr *ehdr)
>        /* XXX The following is wrong!  Dave Miller rejected to implement it
>  	 correctly.  If this causes problems shoot *him*!  */
>  #ifdef SHARED
> -      return GLRO(dl_hwcap) & GLRO(dl_hwcap_mask) & HWCAP_SPARC_V9;
> +# if HAVE_TUNABLES
> +      uint64_t hwcap_mask = TUNABLE_GET (glibc, tune, hwcap_mask);

You are missing '#include <elf/dl-tunables.h>' and the final type argument for
TUNABLE_GET (it fails to build for sparcv9 with tunable enabled).

> +# else
> +      uint64_t hwcap_mask = GLRO(dl_hwcap_mask);
> +# endif
> +      return GLRO(dl_hwcap) & hwcap_mask & HWCAP_SPARC_V9;
>  #else
>        return GLRO(dl_hwcap) & HWCAP_SPARC_V9;
>  #endif
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index b481f50..4fe58bf 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -316,7 +316,11 @@ no_cpuid:
>    /* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86.  */
>    GLRO(dl_platform) = NULL;
>    GLRO(dl_hwcap) = 0;
> +#if !HAVE_TUNABLES
> +  /* The glibc.tune.hwcap_mask tunable is initialized already, so no need to do
> +     this.  */
>    GLRO(dl_hwcap_mask) = HWCAP_IMPORTANT;
> +#endif
>  
>  # ifdef __x86_64__
>    if (cpu_features->kind == arch_kind_intel)
>