[5/6] Make LD_HWCAP_MASK usable for static binaries

Message ID 1496347928-19432-6-git-send-email-siddhesh@sourceware.org
State New, archived
Headers

Commit Message

Siddhesh Poyarekar June 1, 2017, 8:12 p.m. UTC
  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.
---
 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(-)
  

Comments

Adhemerval Zanella June 6, 2017, 7:06 p.m. UTC | #1
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
>  }
>
  

Patch

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
 }