Message ID | 1496347928-19432-6-git-send-email-siddhesh@sourceware.org |
---|---|
State | New |
Headers | show |
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 > } >
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; #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 -# 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 }