Re: RFC: Automatically test IFUNC implementations

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, Sep 27, 2012 at 4:31 AM, Andreas Jaeger <aj@suse.com> wrote:
> On Wednesday, September 26, 2012 19:55:58 H.J. Lu wrote:
>> On Wed, Sep 26, 2012 at 10:43 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> > On Wed, Sep 26, 2012 at 10:17 AM, Richard Henderson
> <rth@twiddle.net> wrote:
>> >> On 09/26/2012 10:11 AM, H.J. Lu wrote:
>> >>> It is hard to avoid malloc since __libc_func is called from
>> >>> test_init. I will first convert it to this.
>> >>
>> >> Statically allocated array (or even statically sized malloc) in
>> >> test_init? If __libc_func indicates overflow, print an error?  It
>> >> should be fairly easy to set this at, say, 32 and have that
>> >> suffice for all calls.>
>> > This should work.  I will update hjl/ifunc/test branch in the next
>> > few days.
>> >
>> > Other feedbacks?
>>
>> I updated hjl/ifunc/test branch to avoid relocations to return IFUNC
>> list. I am enclosing the framework and i686/x86-64 patches here.
>
> Does this pass the testsuite? I'm interested also in the abi-tests.

It passes "make check" and "make xcheck" on x32, x86-64 and ia32,
with and without multi-arch.  __libc_func is added to GLIBC_PRIVATE
and doesn't affect public ABI.

> Looking at sysdeps/x86_64/multiarch/libc-func.c:
>
>> +static struct libc_func_test memmove_list[4];
>
> Why not have MAX_MEMMOVE instead of 4 here?
>> +static struct libc_func_test *
>> +find_memmove (void)
>> +{
>> +  size_t i;
>
> I would assign i here:
>    i=0;
>> +  if (HAS_SSSE3)
>> +    {
>> +      memmove_list[0] = LIBC_FUNC_INIT (__memmove_ssse3_back);
>> +      memmove_list[1] = LIBC_FUNC_INIT (__memmove_ssse3);
>> +      i = 2;
>> +    }
>> +  else
>> +    i = 0;
>
>
> And remove the above two lines, let the compiler optimize it.
>
> Or even make i=-1 initially and increase i every time.
>
> Also, I would add an assert so that we remember to increase the size of
> the array:
> assert (i < MAX_MEMMOVE);
>
>> +  memmove_list[i] = LIBC_FUNC_INIT (__memmove_sse2);
>

Changed on  hjl/ifunc/test branch.  Here are updated i686/x86-64
patches.

Thanks.

-- 
H.J.

Attachment: 0002-Add-i686-multiarch-__libc_func.patch
Description: Binary data

Attachment: 0003-Add-x86_64-multiarch-__libc_func.patch
Description: Binary data