[PATCH] elf: Fix glibc-hwcaps priorities with cache flags mismatches [BZ #27046]
Szabolcs Nagy
szabolcs.nagy@arm.com
Thu Jun 24 17:19:45 GMT 2021
The 06/22/2021 15:27, Florian Weimer via Libc-alpha wrote:
> If lib->flags (in the cache) did not match GLRO (dl_correct_cache_id),
> searching for further glibc-hwcaps entries did not happen, and it
> was possible that the best glibc-hwcaps was not found. By accident,
> this causes a test failure for elf/tst-glibc-hwcaps-prepend-cache
> on armv7l.
>
> This commit changes the cache lookup logic to continue searching
> if (a) no match has been found, (b) a named glibc-hwcaps match
> has been found(), or (c) non-glibc-hwcaps match has been found
> and the entry flags and cache default flags do not match.
>
> _DL_CACHE_DEFAULT_ID is used instead of GLRO (dl_correct_cache_id)
> because the latter is only written once on i386 if loading
> of libc.so.5 libraries is selected, so GLRO (dl_correct_cache_id)
> should probably removed in a future change.
>
> Tested on i686-linux-gnu, x86_64-linux-gnu, and on an armv7l system that
> exposed the issue. Most of the diff is due to whitespace changes.
Looks good, thanks.
Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
>
> ---
> elf/dl-cache.c | 134 ++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 65 insertions(+), 69 deletions(-)
>
> diff --git a/elf/dl-cache.c b/elf/dl-cache.c
> index 32f3bef5ea..2b8da8650d 100644
> --- a/elf/dl-cache.c
> +++ b/elf/dl-cache.c
> @@ -269,81 +269,77 @@ search_cache (const char *string_table, uint32_t string_table_size,
> if (_dl_cache_check_flags (flags)
> && _dl_cache_verify_ptr (lib->value, string_table_size))
> {
> - if (best == NULL || flags == GLRO (dl_correct_cache_id))
> + /* Named/extension hwcaps get slightly different
> + treatment: We keep searching for a better
> + match. */
> + bool named_hwcap = false;
> +
> + if (entry_size >= sizeof (struct file_entry_new))
> {
> - /* Named/extension hwcaps get slightly different
> - treatment: We keep searching for a better
> - match. */
> - bool named_hwcap = false;
> + /* The entry is large enough to include
> + HWCAP data. Check it. */
> + struct file_entry_new *libnew
> + = (struct file_entry_new *) lib;
>
> - if (entry_size >= sizeof (struct file_entry_new))
> +#ifdef SHARED
> + named_hwcap = dl_cache_hwcap_extension (libnew);
> + if (named_hwcap
> + && !dl_cache_hwcap_isa_level_compatible (libnew))
> + continue;
> +#endif
> +
> + /* The entries with named/extension hwcaps have
> + been exhausted (they are listed before all
> + other entries). Return the best match
> + encountered so far if there is one. */
> + if (!named_hwcap && best != NULL)
> + break;
> +
> + if ((libnew->hwcap & hwcap_exclude) && !named_hwcap)
> + continue;
> + if (GLRO (dl_osversion)
> + && libnew->osversion > GLRO (dl_osversion))
> + continue;
> + if (_DL_PLATFORMS_COUNT
> + && (libnew->hwcap & _DL_HWCAP_PLATFORM) != 0
> + && ((libnew->hwcap & _DL_HWCAP_PLATFORM)
> + != platform))
> + continue;
> +
> +#ifdef SHARED
> + /* For named hwcaps, determine the priority and
> + see if beats what has been found so far. */
> + if (named_hwcap)
> {
> - /* The entry is large enough to include
> - HWCAP data. Check it. */
> - struct file_entry_new *libnew
> - = (struct file_entry_new *) lib;
> -
> -#ifdef SHARED
> - named_hwcap = dl_cache_hwcap_extension (libnew);
> - if (named_hwcap
> - && !dl_cache_hwcap_isa_level_compatible (libnew))
> + uint32_t entry_priority
> + = glibc_hwcaps_priority (libnew->hwcap);
> + if (entry_priority == 0)
> + /* Not usable at all. Skip. */
> + continue;
> + else if (best == NULL
> + || entry_priority < best_priority)
> + /* This entry is of higher priority
> + than the previous one, or it is the
> + first entry. */
> + best_priority = entry_priority;
> + else
> + /* An entry has already been found,
> + but it is a better match. */
> continue;
> -#endif
> -
> - /* The entries with named/extension hwcaps
> - have been exhausted. Return the best
> - match encountered so far if there is
> - one. */
> - if (!named_hwcap && best != NULL)
> - break;
> -
> - if ((libnew->hwcap & hwcap_exclude) && !named_hwcap)
> - continue;
> - if (GLRO (dl_osversion)
> - && libnew->osversion > GLRO (dl_osversion))
> - continue;
> - if (_DL_PLATFORMS_COUNT
> - && (libnew->hwcap & _DL_HWCAP_PLATFORM) != 0
> - && ((libnew->hwcap & _DL_HWCAP_PLATFORM)
> - != platform))
> - continue;
> -
> -#ifdef SHARED
> - /* For named hwcaps, determine the priority
> - and see if beats what has been found so
> - far. */
> - if (named_hwcap)
> - {
> - uint32_t entry_priority
> - = glibc_hwcaps_priority (libnew->hwcap);
> - if (entry_priority == 0)
> - /* Not usable at all. Skip. */
> - continue;
> - else if (best == NULL
> - || entry_priority < best_priority)
> - /* This entry is of higher priority
> - than the previous one, or it is the
> - first entry. */
> - best_priority = entry_priority;
> - else
> - /* An entry has already been found,
> - but it is a better match. */
> - continue;
> - }
> -#endif /* SHARED */
> }
> -
> - best = string_table + lib->value;
> -
> - if (flags == GLRO (dl_correct_cache_id)
> - && !named_hwcap)
> - /* We've found an exact match for the shared
> - object and no general `ELF' release. Stop
> - searching, but not if a named (extension)
> - hwcap is used. In this case, an entry with
> - a higher priority may come up later. */
> - break;
> +#endif /* SHARED */
> }
> +
> + best = string_table + lib->value;
> +
> + if (!named_hwcap && flags == _DL_CACHE_DEFAULT_ID)
> + /* With named hwcaps, we need to keep searching to
> + see if we find a better match. A better match
> + is also possible if the flags of the current
> + entry do not match the expected cache flags.
> + But if the flags match, no better entry will be
> + found. */
> + break;
> }
> }
> while (++middle <= right);
>
More information about the Libc-alpha
mailing list