public inbox for libc-stable@sourceware.org
 help / color / mirror / Atom feed
From: Aurelien Jarno <aurelien@aurel32.net>
To: libc-stable@sourceware.org
Cc: Florian Weimer <fweimer@redhat.com>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>
Subject: [COMMITTED 2.33] elf: Fix glibc-hwcaps priorities with cache flags mismatches [BZ #27046]
Date: Tue, 18 Jan 2022 23:12:21 +0100	[thread overview]
Message-ID: <20220118221221.1926142-1-aurelien@aurel32.net> (raw)

From: Florian Weimer <fweimer@redhat.com>

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.

Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
(cherry picked from commit 66db95b6e8264c5a6307f6a9e5285fec76907254)
---
 elf/dl-cache.c | 124 ++++++++++++++++++++++++-------------------------
 1 file changed, 60 insertions(+), 64 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;
+		  /* 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))
-			{
-			  /* 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))
+		    {
+		      /* 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))
-			    continue;
+		      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.  Return the best
-			     match encountered so far if there is
-			     one.  */
-			  if (!named_hwcap && best != NULL)
-			    break;
+		      /* 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;
+		      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 */
+		      /* 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;
+		  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;
-		    }
+		  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);
-- 
2.34.1


                 reply	other threads:[~2022-01-18 22:12 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220118221221.1926142-1-aurelien@aurel32.net \
    --to=aurelien@aurel32.net \
    --cc=fweimer@redhat.com \
    --cc=libc-stable@sourceware.org \
    --cc=szabolcs.nagy@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).