Re: [PATCH] Fix localplt check for GNU_IFUNC

[Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>]

On 19-03-2014 15:47, Carlos O'Donell wrote:
> On 03/08/2014 01:39 PM, Adhemerval Zanella wrote:
>> While reviewing the strncat optimization for powerpc64 I noted in the
>> disassembled object that a call to strlen (bl strlen) generates a PLT
>> call to strlen (since now strlen is an IFUNC for powerpc64). Strangely
>> it was not being caught by check-localplt testcase.
>>
>> GNU_IFUNC are shown by readelf in 'Relocation section' value as (symbol)
>> instead of expected hexadecimal value. This causes the check-localplt
>> script to igore potential PLT stub begin generated by wrong IFUNC usage.
>> This patch changes the localplt script to emit such PLT cases.
>>
>> Checked on PPC64 and X86_64.
>>
>> --
>>
>> 2014-03-03  Adhemerval Zanella  <azanella@linux.vnet.ibm.com>
>>
>>         * scripts/localplt.awk: Do not ignore IFUNC relocation while parsing
>> 	relocation sections.
> Can you show example output?
>
> On x86-64 I see:
> 0000003000bba030  0000034400000007 R_X86_64_JUMP_SLOT     memset()         memset + 0
> 0000003000bba048  000007ba00000007 R_X86_64_JUMP_SLOT     memmove()        memmove + 0
>
> Not (memset) or (memmove).
>
> I'd like to see this fixed... and this is the first patch in patchtracker :-)

Thanks for the review Carlos. I rechecked the patch here and I did some confusion (although
the patch itself was not really incorrect). Indeed from binutils/readelf.c:1315 comment:

 1315                   /* Relocations against GNU_IFUNC symbols do not use the value
 1316                      of the symbol as the address to relocate against.  Instead
 1317                      they invoke the function named by the symbol and use its
 1318                      result as the address for relocation.
 1319 
 1320                      To indicate this to the user, do not display the value of
 1321                      the symbol in the "Symbols's Value" field.  Instead show
 1322                      its name followed by () as a hint that the symbol is
 1323                      invoked.  */

And by changing the strncat.S powerpc implementation to call the ifunc directly:

diff --git a/sysdeps/powerpc/powerpc64/power7/strncat.S b/sysdeps/powerpc/powerpc64/power7/strncat.S
index 1a1a95e..23861f6 100644
--- a/sysdeps/powerpc/powerpc64/power7/strncat.S
+++ b/sysdeps/powerpc/powerpc64/power7/strncat.S
@@ -69,7 +69,7 @@ EALIGN(STRNCAT, 4, 0)
        beq cr0,L(done)
 
        mr r31, r4                      /* Save "s2" in r31 from r4.  */
-       bl STRLEN                       /* Call optimized strlen on s1; goto
+       bl strlen                       /* Call optimized strlen on s1; goto
                                           end of s1.  */
        nop
        cmpldi cr7, r29, 7              /* If s2 is <=7 process

And rebuilding it I see:

$ LC_ALL=C readelf -W -S -d -r libc.so | grep strlen
00000000001f2bc8  0000033b00000015 R_PPC64_JMP_SLOT       strlen()         strlen + 0

So below there is a slight modified patch to address it (I didn't add the ChangeLog).
What do you think ?

---

diff --git a/scripts/localplt.awk b/scripts/localplt.awk
index 2265b02..f55c41a 100644
--- a/scripts/localplt.awk
+++ b/scripts/localplt.awk
@@ -32,9 +32,15 @@ $1 == "Offset" && $2 == "Info" { in_relocs = 1; next }
 NF == 0 { in_relocs = 0 }

 in_relocs && relocs_offset == jmprel_offset && NF >= 5 {
-  symval = strtonum("0x" $4);
-  if (symval != 0)
+  # Relocations against GNU_IFUNC symbols are not shown as an hexadecimal
+  # value, but rather as the resolver symbol followed by ().
+  if ($4 ~ /\(\)/) {
     print whatfile, $5
+  } else {
+    symval = strtonum("0x" $4);
+    if (symval != 0)
+      print whatfile, $5
+  }
 }

 in_relocs { next }