Re: [RFC 2/9] elf: Remove loading legacy hwcaps/platform entries in dynamic loader

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

On 07/06/24 08:42, Stefan Liebler wrote:
> The legacy hwcaps mechanism was removed with glibc 2.37:
> See this commit series:
> - d178c67535f0d159df73843e7c18cbdb39b4d25d
> x86_64: Remove platform directory library loading test
> - 6099908fb84debee4c3bcb05d88769410c2aecd1
> elf: Remove legacy hwcaps support from the dynamic loader
> - b78ff5a25dc8ba9d8c6df10bb0a533254bdd193f
> elf: Remove legacy hwcaps support from ldconfig
> - 4a7094119ce05cadf927f52cc5d723e2195e65f9
> elf: Remove hwcap parameter from add_to_cache signature
> - cfbf883db36727a84ef7929af49ef68c195b5972
> elf: Remove hwcap and bits_hwcap fields from struct cache_entry
> - 78d9a1620b840deb0880686e4159eaf70708866a
> Add NEWS entry for legacy hwcaps removal
> - ab40f20364f4a417a63dd51fdd943742070bfe96
> elf: Remove _dl_string_hwcap
> - e76369ed6371734f77f468eab097ef4e5b5db1c5
> elf: Simplify output of hwcap subdirectories in ld.so help
> 
> According to Florian Weimer, this was an oversight and should also
> have been removed.
> 
> As ldconfig does not generate ld.so.cache entries with hwcap/platform
> bits in the hwcap-field anymore, this patch now skips those entries.
> Thus currently only named-hwcap-entries and the default entries are
> allowed.
> For named-hwcap entries bit 62 is set and also the isa-level bits can
> be set.
> For the default entries the hwcap-field is 0.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  elf/dl-cache.c | 22 +++-------------------
>  1 file changed, 3 insertions(+), 19 deletions(-)
> 
> diff --git a/elf/dl-cache.c b/elf/dl-cache.c
> index 85f3f179ed..7c7dc58745 100644
> --- a/elf/dl-cache.c
> +++ b/elf/dl-cache.c
> @@ -27,10 +27,6 @@
>  #include <dl-hwcaps.h>
>  #include <dl-isa-level.h>
>  
> -#ifndef _DL_PLATFORMS_COUNT
> -# define _DL_PLATFORMS_COUNT 0
> -#endif
> -
>  /* This is the starting address and the size of the mmap()ed file.  */
>  static struct cache_file *cache;
>  static struct cache_file_new *cache_new;
> @@ -201,15 +197,6 @@ search_cache (const char *string_table, uint32_t string_table_size,
>  	      struct file_entry *libs, uint32_t nlibs, uint32_t entry_size,
>  	      const char *name)
>  {
> -  /* Used by the HWCAP check in the struct file_entry_new case.  */
> -  uint64_t platform = _dl_string_platform (GLRO (dl_platform));
> -  if (platform != (uint64_t) -1)
> -    platform = 1ULL << platform;
> -  uint64_t hwcap_mask = TUNABLE_GET (glibc, cpu, hwcap_mask, uint64_t, NULL);
> -#define _DL_HWCAP_TLS_MASK (1LL << 63)
> -  uint64_t hwcap_exclude = ~((GLRO (dl_hwcap) & hwcap_mask)
> -			     | _DL_HWCAP_PLATFORM | _DL_HWCAP_TLS_MASK);
> -
>    int left = 0;
>    int right = nlibs - 1;
>    const char *best = NULL;
> @@ -295,12 +282,9 @@ search_cache (const char *string_table, uint32_t string_table_size,
>  		      if (!named_hwcap && best != NULL)
>  			break;
>  
> -		      if ((libnew->hwcap & hwcap_exclude) && !named_hwcap)
> -			continue;
> -		      if (_DL_PLATFORMS_COUNT
> -			  && (libnew->hwcap & _DL_HWCAP_PLATFORM) != 0
> -			  && ((libnew->hwcap & _DL_HWCAP_PLATFORM)
> -			      != platform))
> +		      /* Skip entries with the legacy hwcap/platform mechanism
> +			 which was removed with glibc 2.37.  */
> +		      if (!named_hwcap && libnew->hwcap != 0)
>  			continue;
>  
>  #ifdef SHARED