Re: [PATCH] Fix hwcap handling in dl

[GOTO Masanori <gotom at debian dot or dot jp>]

Hi,

At Sun, 02 Feb 2003 14:02:56 -0800,
Ulrich Drepper wrote:
> > Currently, hwcap check routine uses below code:
> > 
> >      if (hwcap 
> > 	 && ((lib->hwcap & *hwcap & ~_DL_HWCAP_PLATFORM) > *hwcap))
> > 
> > But this condition does not always fulfill, because left equation does
> > not exceed by right equation.  My fix is if lib->hwcap has a value and
> > it does not match the current hwcap, then continue and use an another
> > library.
> 
> From your desription I don't get what you think is the problem
> 
> Please provide a concrete example.

I'm sorry, my previous patch was incorrect, and I didn't show an
appropriate example.  The correct fix is appended, with more
explanations.

I found this bug that when I added HWCAP_I386_CMOV into
HWCAP_IMPORTANT, hwcap'ed library was linked anytime even on VIA C3 or
i586 machine which does not have CMOV capability.

Use the example.  If a library is put on /lib/i686/cmov, and run
ldconfig, then this library's lib->hwcap becomes 0x80000 00008000.
The value of hwcap on VIA C3 is 0x8001bf.  Now this library is loaded
from a binary, and reaches this hwcap check routine:

   sysdeps/generic/dl-cache.c::_dl_load_cache_lookup():

      if (hwcap                                                               \
          && ((lib->hwcap & *hwcap & ~_DL_HWCAP_PLATFORM) > *hwcap))          \
        continue

This equation with the example is:

	lib->hwcap & ~_DL_HWCAP_PLATFORM & *hwcap > *hwcap
	~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
			0x8000         & 0x8001bf > 0x8001bf
			~~~~~~~~~~~~~~~~~~~~~~~~~
					0	  > 0x8001bf

It become not true, "continue" does not applied, so this hwcap'ed
library (which is not permitted on this machine's hwcap) is loaded.
Then the binary crashes.  Moreover, you find that this equation is not
fulfilled forever, because the left equation (lib->hwcap &
~_DL_HWCAP_PLATFORM & *hwcap) never exceeds right equation (*hwcap).

The left equation (lib->hwcap & ~_DL_HWCAP_PLATFORM & *hwcap) means
"the number of capabilities which is filtered by the current *hwcap".
So, at least the number of capabilities has to be equal to the number
of library's hwcap.  So I modify the equation as follows:

       if (hwcap                                                             \
         && ((lib->hwcap & ~_DL_HWCAP_PLATFORM & *hwcap) <                   \
             (lib->hwcap & ~_DL_HWCAP_PLATFORM)))                            \
        continue

The right equation means "the allowable watermark of this library's
hwcap".  If the left equation does not reach to the number of bit in
the right equation, "continue" is selected; so this hwcap'ed library
is avoided to apply.  Back to the previous example:

	lib->hwcap & ~_DL_HWCAP_PLATFORM & *hwcap < lib->hwcap & ~_DL_HWCAP_PLATFORM
	~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
			0x8000         & 0x8001bf < 0x8000
			~~~~~~~~~~~~~~~~~~~~~~~~~
					0	  < 0x8000

The condition becomes true, it goes to "continue", so this hwcap'ed
library is not used, and the next candidate is selected.

This patch is already worked on some Debian VIA C3 users, and my
environment (I tested on i586) too.  Could you consider to apply this
patch?

Regards,
-- gotom


2002-02-25  GOTO Masanori  <gotom at debian dot or dot jp>

	* sysdeps/generic/dl-cache.c: Fix hwcap check condition.


--- sysdeps/generic/dl-cache.c	2003-02-02 05:33:47.000000000 +0900
+++ sysdeps/generic/dl-cache.c.new1	2003-02-21 09:18:19.000000000 +0900
@@ -232,7 +232,8 @@
 	  && (lib->hwcap & _DL_HWCAP_PLATFORM) != platform)		      \
 	continue;							      \
       if (hwcap								      \
-	  && ((lib->hwcap & *hwcap & ~_DL_HWCAP_PLATFORM) > *hwcap))	      \
+	  && ((lib->hwcap & ~_DL_HWCAP_PLATFORM & *hwcap) <		      \
+	      (lib->hwcap & ~_DL_HWCAP_PLATFORM)))			      \
 	continue
       SEARCH_CACHE (cache_new);
     }