This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 5/6] Make LD_HWCAP_MASK usable for static binaries
On 01/06/2017 17:12, Siddhesh Poyarekar wrote:
> The LD_HWCAP_MASK environment variable was ignored in static binaries,
> which is inconsistent with the behaviour of dynamically linked
> binaries. This seems to have been because of the inability of
> ld_hwcap_mask being read early enough to influence anything but now
> that it is in tunables, the mask is usable in static binaries as well.
>
> This feature is important for aarch64, which relies on HWCAP_CPUID
> being masked out to disable multiarch. A sanity test on x86_64 shows
> that there are no failures. Likewise for aarch64.
>
> * elf/dl-hwcaps.h [HAVE_TUNABLES]: Always read hwcap_mask.
> * sysdeps/sparc/sparc32/dl-machine.h [HAVE_TUNABLES]:
> Likewise.
> * sysdeps/x86/cpu-features.c (init_cpu_features): Always set
> up hwcap and hwcap_mask.
LGTM with a remark below.
> ---
> elf/dl-hwcaps.h | 15 +++++++--------
> sysdeps/sparc/sparc32/dl-machine.h | 2 +-
> sysdeps/x86/cpu-features.c | 8 +++-----
> 3 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/elf/dl-hwcaps.h b/elf/dl-hwcaps.h
> index 9ce3317..2c4fa3d 100644
> --- a/elf/dl-hwcaps.h
> +++ b/elf/dl-hwcaps.h
> @@ -18,14 +18,13 @@
>
> #include <elf/dl-tunables.h>
>
> -#ifdef SHARED
> -# if HAVE_TUNABLES
> -# define GET_HWCAP_MASK() \
> - TUNABLE_GET (glibc, tune, hwcap_mask, uint64_t, NULL)
> +#if HAVE_TUNABLES
> +# define GET_HWCAP_MASK() TUNABLE_GET (glibc, tune, hwcap_mask, uint64_t, NULL)
> +#else
> +# ifdef SHARED
> +# define GET_HWCAP_MASK() GLRO(dl_hwcap_mask)
> # else
> -# define GET_HWCAP_MASK() GLRO(dl_hwcap_mask)
> +/* HWCAP_MASK is ignored in static binaries when built without tunables. */
> +# define GET_HWCAP_MASK() (0)
> # endif
> -#else
> -/* HWCAP_MASK is ignored in static binaries. */
> -# define GET_HWCAP_MASK() (0)
> #endif
> diff --git a/sysdeps/sparc/sparc32/dl-machine.h b/sysdeps/sparc/sparc32/dl-machine.h
> index f9ae133..95f6732 100644
> --- a/sysdeps/sparc/sparc32/dl-machine.h
> +++ b/sysdeps/sparc/sparc32/dl-machine.h
> @@ -37,7 +37,7 @@ elf_machine_matches_host (const Elf32_Ehdr *ehdr)
> return 1;
> else if (ehdr->e_machine == EM_SPARC32PLUS)
> {
> -#ifdef SHARED
> +#if HAVE_TUNABLES || defined SHARED
> uint64_t hwcap_mask = GET_HWCAP_MASK();
> return GLRO(dl_hwcap) & hwcap_mask & HWCAP_SPARC_V9;
Since GET_HWCAP_MASK is define 0 for !SHARED, I think it we remove the
preprocessor altogether and just add a comment that for static binaries
with tunable enables it hwcap_mask will be a no-op.
> #else
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index 4fe58bf..4288001 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -312,17 +312,16 @@ no_cpuid:
> cpu_features->model = model;
> cpu_features->kind = kind;
>
> -#if IS_IN (rtld)
> /* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86. */
> GLRO(dl_platform) = NULL;
> GLRO(dl_hwcap) = 0;
> -#if !HAVE_TUNABLES
> +#if !HAVE_TUNABLES && defined SHARED
> /* The glibc.tune.hwcap_mask tunable is initialized already, so no need to do
> this. */
> GLRO(dl_hwcap_mask) = HWCAP_IMPORTANT;
> #endif
Just a though: with a SET_HWCAP_MASK macro we can get rid of preprocessor
macros (since we can define it to a no-op for static binaries without
tunables support).
>
> -# ifdef __x86_64__
> +#ifdef __x86_64__
> if (cpu_features->kind == arch_kind_intel)
> {
> if (CPU_FEATURES_ARCH_P (cpu_features, AVX512F_Usable)
> @@ -352,7 +351,7 @@ no_cpuid:
> && CPU_FEATURES_CPU_P (cpu_features, POPCNT))
> GLRO(dl_platform) = "haswell";
> }
> -# else
> +#else
> if (CPU_FEATURES_CPU_P (cpu_features, SSE2))
> GLRO(dl_hwcap) |= HWCAP_X86_SSE2;
>
> @@ -360,6 +359,5 @@ no_cpuid:
> GLRO(dl_platform) = "i686";
> else if (CPU_FEATURES_ARCH_P (cpu_features, I586))
> GLRO(dl_platform) = "i586";
> -# endif
> #endif
> }
>