This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/2] bfd/elfnn-aarch64.c: Handle static links with ifunc correctly.


On 11/26/13 11:16, Will Newton wrote:
On 26 November 2013 11:06, Yufeng Zhang<Yufeng.Zhang@arm.com>  wrote:
On 11/26/13 08:54, Will Newton wrote:

The code for handling GOT references to ifunc symbols in static links
was missing.

bfd/ChangeLog:

2013-11-25  Will Newton<will.newton@linaro.org>

         * elfnn-aarch64.c (elfNN_aarch64_finish_dynamic_symbol):
         Handle STT_GNU_IFUNC symbols correctly in static links.

2013-11-25  Will Newton<will.newton@linaro.org>

         * ld-aarch64/aarch64-elf.exp: Add ifunc-22.
         * ld-aarch64/ifunc-22.d: New file.
         * ld-aarch64/ifunc-22.s: Likewise.
---
   bfd/elfnn-aarch64.c                     | 30
+++++++++++++++++++++++++++++-
   ld/testsuite/ld-aarch64/aarch64-elf.exp |  1 +
   ld/testsuite/ld-aarch64/ifunc-22.d      | 11 +++++++++++
   ld/testsuite/ld-aarch64/ifunc-22.s      | 14 ++++++++++++++
   4 files changed, 55 insertions(+), 1 deletion(-)
   create mode 100644 ld/testsuite/ld-aarch64/ifunc-22.d
   create mode 100644 ld/testsuite/ld-aarch64/ifunc-22.s

OK for trunk and binutils_2_24-branch?

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index 7cce6f4..1467f5d 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -6824,7 +6824,34 @@ elfNN_aarch64_finish_dynamic_symbol (bfd
*output_bfd,
                        + htab->root.sgot->output_offset
                        + (h->got.offset&   ~(bfd_vma) 1));

-      if (info->shared&&   SYMBOL_REFERENCES_LOCAL (info, h))

+      if (h->def_regular
+&&   h->type == STT_GNU_IFUNC)
+       {
+         if (info->shared)
+           {
+             /* Generate R_AARCH64_GLOB_DAT.  */
+             goto do_glob_dat;
+           }


Can the control flow be optimized so that the outer if condition also checks
!info->shared?  I wonder whether the goto statement be avoided.

It would involve altering the else if condition somewhat so I am
inclined to leave as is.

I am aware that the bfd copy and paste model of code reuse is rather
unsatisfactory, but sometimes I think it is better to have the same
bugs as everyone else rather than different ones!

Now I realized where the code is from.  Not ideal but I see your point.

And I also I have
dreams that one day some kindly bfd hacker will come along and pull
some of this stuff out into common code and at that point the
similarity between ports will make that job easier.

I have a similar dream ;)


Yufeng




  +      if (h->def_regular
  +&&   h->type == STT_GNU_IFUNC
  +&&   !info->shared )



+         else
+           {
+             asection *plt;
+
+             if (!h->pointer_equality_needed)
+               abort ();
+
+             /* For non-shared object, we can't use .got.plt, which
+                contains the real function addres if we need pointer


addres/address

Thanks, fixed.

+                equality.  We load the GOT entry with the PLT entry.  */
+             plt = htab->root.splt ? htab->root.splt : htab->root.iplt;
+             bfd_put_NN (output_bfd, (plt->output_section->vma
+                                      + plt->output_offset
+                                      + h->plt.offset),
+                         htab->root.sgot->contents
+                         + (h->got.offset&   ~(bfd_vma) 1));

+             return TRUE;
+           }
+       }
+      else if (info->shared&&   SYMBOL_REFERENCES_LOCAL (info, h))

         {
           if (!h->def_regular)
             return FALSE;
@@ -6838,6 +6865,7 @@ elfNN_aarch64_finish_dynamic_symbol (bfd
*output_bfd,
         else
         {
           BFD_ASSERT ((h->got.offset&   1) == 0);

+do_glob_dat:
           bfd_put_NN (output_bfd, (bfd_vma) 0,
                       htab->root.sgot->contents + h->got.offset);
           rela.r_info = ELFNN_R_INFO (h->dynindx, AARCH64_R (GLOB_DAT));


Is do_glob_dat placed deliberately after the assertion?

I don't know. But I share your concern about this code, I'll move the
label up above the assert as I don't see a case where the assert could
fail but the code below remain valid.




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]