PR 20472, PowerPC64 ifunc confusion

This patch fixes quite a lot of confusion in allocate_dynrelocs over
ifuncs.  Function descriptors make ELFv1 quite different to ELFv2.

	PR 20472
	* elf64-ppc.c (ppc64_elf_before_check_relocs): Tweak abiversion test.
	(readonly_dynrelocs): Comment fix.
	(global_entry_stub): New function.
	(ppc64_elf_adjust_dynamic_symbol): Tweak abiversion test.  Match
	ELFv2 code deciding on dynamic relocs vs. global entry stubs to
	that in size_global_entry_stubs, handling ifunc too.  Delete dead
	weak sym code.
	(allocate_dynrelocs): Ensure dyn_relocs field is cleared when no
	dyn_relocs are needed.  Correct handling of ifunc dyn_relocs.
	Tidy ELIMINATE_COPY_RELOCS code, only setting dynindx for
	undefweak syms.  Expand and correct comments.
	(size_global_entry_stubs): Ensure symbol is defined.
	(ppc64_elf_relocate_section): Match condition under which
	dyn_relocs are emitted to that in allocate_dynrelocs.

2 files changed, 111 insertions, 65 deletions

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 7326ecd..c579a02 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,21 @@
+2016-08-19  Alan Modra  <amodra@gmail.com>
+
+	PR 20472
+	* elf64-ppc.c (ppc64_elf_before_check_relocs): Tweak abiversion test.
+	(readonly_dynrelocs): Comment fix.
+	(global_entry_stub): New function.
+	(ppc64_elf_adjust_dynamic_symbol): Tweak abiversion test.  Match
+	ELFv2 code deciding on dynamic relocs vs. global entry stubs to
+	that in size_global_entry_stubs, handling ifunc too.  Delete dead
+	weak sym code.
+	(allocate_dynrelocs): Ensure dyn_relocs field is cleared when no
+	dyn_relocs are needed.  Correct handling of ifunc dyn_relocs.
+	Tidy ELIMINATE_COPY_RELOCS code, only setting dynindx for
+	undefweak syms.  Expand and correct comments.
+	(size_global_entry_stubs): Ensure symbol is defined.
+	(ppc64_elf_relocate_section): Match condition under which
+	dyn_relocs are emitted to that in allocate_dynrelocs.
+
 2016-08-12  Alan Modra  <amodra@gmail.com>
 
 	* elf-bfd.h (struct elf_link_hash_table): Add local_dynsymcount.
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 4f854c7..bcf7170 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -5081,7 +5081,7 @@ ppc64_elf_before_check_relocs (bfd *ibfd, struct bfd_link_info *info)
     {
       if (abiversion (ibfd) == 0)
 	set_abiversion (ibfd, 1);
-      else if (abiversion (ibfd) == 2)
+      else if (abiversion (ibfd) >= 2)
 	{
 	  info->callbacks->einfo (_("%P: %B .opd not allowed in ABI"
 				    " version %d\n"),
@@ -7102,7 +7102,8 @@ ppc64_elf_func_desc_adjust (bfd *obfd ATTRIBUTE_UNUSED,
   return TRUE;
 }
 
-/* Return true if we have dynamic relocs that apply to read-only sections.  */
+/* Return true if we have dynamic relocs against H that apply to
+   read-only sections.  */
 
 static bfd_boolean
 readonly_dynrelocs (struct elf_link_hash_entry *h)
@@ -7121,6 +7122,27 @@ readonly_dynrelocs (struct elf_link_hash_entry *h)
   return FALSE;
 }
 
+
+/* Return true if a global entry stub will be created for H.  Valid
+   for ELFv2 before plt entries have been allocated.  */
+
+static bfd_boolean
+global_entry_stub (struct elf_link_hash_entry *h)
+{
+  struct plt_entry *pent;
+
+  if (!h->pointer_equality_needed
+      || h->def_regular)
+    return FALSE;
+
+  for (pent = h->plt.plist; pent != NULL; pent = pent->next)
+    if (pent->plt.refcount > 0
+	&& pent->addend == 0)
+      return TRUE;
+
+  return FALSE;
+}
+
 /* Adjust a symbol defined by a dynamic object and referenced by a
    regular object.  The current definition is in some section of the
    dynamic object, but we're not including those sections.  We have to
@@ -7160,35 +7182,25 @@ ppc64_elf_adjust_dynamic_symbol (struct bfd_link_info *info,
 	  h->needs_plt = 0;
 	  h->pointer_equality_needed = 0;
 	}
-      else if (abiversion (info->output_bfd) == 2)
+      else if (abiversion (info->output_bfd) >= 2)
 	{
 	  /* Taking a function's address in a read/write section
 	     doesn't require us to define the function symbol in the
 	     executable on a global entry stub.  A dynamic reloc can
-	     be used instead.  */
-	  if (h->pointer_equality_needed
-	      && h->type != STT_GNU_IFUNC
+	     be used instead.  The reason we prefer a few more dynamic
+	     relocs is that calling via a global entry stub costs a
+	     few more instructions, and pointer_equality_needed causes
+	     extra work in ld.so when resolving these symbols.  */
+	  if (global_entry_stub (h)
 	      && !readonly_dynrelocs (h))
 	    {
 	      h->pointer_equality_needed = 0;
+	      /* After adjust_dynamic_symbol, non_got_ref set in
+		 the non-pic case means that dyn_relocs for this
+		 symbol should be discarded.  */
 	      h->non_got_ref = 0;
 	    }
 
-	  /* After adjust_dynamic_symbol, non_got_ref set in the
-	     non-shared case means that we have allocated space in
-	     .dynbss for the symbol and thus dyn_relocs for this
-	     symbol should be discarded.
-	     If we get here we know we are making a PLT entry for this
-	     symbol, and in an executable we'd normally resolve
-	     relocations against this symbol to the PLT entry.  Allow
-	     dynamic relocs if the reference is weak, and the dynamic
-	     relocs will not cause text relocation.  */
-	  else if (!h->ref_regular_nonweak
-		   && h->non_got_ref
-		   && h->type != STT_GNU_IFUNC
-		   && !readonly_dynrelocs (h))
-	    h->non_got_ref = 0;
-
 	  /* If making a plt entry, then we don't need copy relocs.  */
 	  return TRUE;
 	}
@@ -9538,7 +9550,6 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
   struct ppc_link_hash_table *htab;
   asection *s;
   struct ppc_link_hash_entry *eh;
-  struct elf_dyn_relocs *p;
   struct got_entry **pgent, *gent;
 
   if (h->root.type == bfd_link_hash_indirect)
@@ -9618,10 +9629,14 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
 	allocate_got (h, info, gent);
       }
 
-  if (eh->dyn_relocs != NULL
-      && (htab->elf.dynamic_sections_created
-	  || h->type == STT_GNU_IFUNC))
+  if (!htab->elf.dynamic_sections_created
+      && h->type != STT_GNU_IFUNC)
+    eh->dyn_relocs = NULL;
+
+  if (eh->dyn_relocs != NULL)
     {
+      struct elf_dyn_relocs *p, **pp;
+
       /* In the shared -Bsymbolic case, discard space allocated for
 	 dynamic pc-relative relocs against symbols which turn out to
 	 be defined in regular objects.  For the normal shared case,
@@ -9639,8 +9654,6 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
 	     avoid writing weird assembly.  */
 	  if (SYMBOL_CALLS_LOCAL (info, h))
 	    {
-	      struct elf_dyn_relocs **pp;
-
 	      for (pp = &eh->dyn_relocs; (p = *pp) != NULL; )
 		{
 		  p->count -= p->pc_count;
@@ -9672,36 +9685,47 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
 	}
       else if (h->type == STT_GNU_IFUNC)
 	{
-	  if (!h->non_got_ref)
+	  /* A plt entry is always created when making direct calls to
+	     an ifunc, even when building a static executable, but
+	     that doesn't cover all cases.  We may have only an ifunc
+	     initialised function pointer for a given ifunc symbol.
+
+	     For ELFv2, dynamic relocations are not required when
+	     generating a global entry PLT stub.  */
+	  if (abiversion (info->output_bfd) >= 2)
+	    {
+	      if (global_entry_stub (h))
+		eh->dyn_relocs = NULL;
+	    }
+
+	  /* For ELFv1 we have function descriptors.  Descriptors need
+	     to be treated like PLT entries and thus have dynamic
+	     relocations.  One exception is when the function
+	     descriptor is copied into .dynbss (which should only
+	     happen with ancient versions of gcc).  */
+	  else if (h->needs_copy)
 	    eh->dyn_relocs = NULL;
 	}
       else if (ELIMINATE_COPY_RELOCS)
 	{
-	  /* For the non-shared case, discard space for relocs against
+	  /* For the non-pic case, discard space for relocs against
 	     symbols which turn out to need copy relocs or are not
 	     dynamic.  */
 
-	  if (!h->non_got_ref
-	      && !h->def_regular)
-	    {
-	      /* Make sure this symbol is output as a dynamic symbol.
-		 Undefined weak syms won't yet be marked as dynamic.  */
-	      if (h->dynindx == -1
-		  && !h->forced_local)
-		{
-		  if (! bfd_elf_link_record_dynamic_symbol (info, h))
-		    return FALSE;
-		}
-
-	      /* If that succeeded, we know we'll be keeping all the
-		 relocs.  */
-	      if (h->dynindx != -1)
-		goto keep;
-	    }
-
-	  eh->dyn_relocs = NULL;
+	  /* First make sure this symbol is output as a dynamic symbol.
+	     Undefined weak syms won't yet be marked as dynamic.  */
+	  if (h->root.type == bfd_link_hash_undefweak
+	      && !h->non_got_ref
+	      && !h->def_regular
+	      && h->dynindx == -1
+	      && !h->forced_local
+	      && !bfd_elf_link_record_dynamic_symbol (info, h))
+	    return FALSE;
 
-	keep: ;
+	  if (h->non_got_ref
+	      || h->def_regular
+	      || h->dynindx == -1)
+	    eh->dyn_relocs = NULL;
 	}
 
       /* Finally, allocate space.  */
@@ -9818,6 +9842,7 @@ size_global_entry_stubs (struct elf_link_hash_entry *h, void *inf)
 	   need to define the symbol in the executable on a call stub.
 	   This is to avoid text relocations.  */
 	s->size = (s->size + 15) & -16;
+	h->root.type = bfd_link_hash_defined;
 	h->root.u.def.section = s;
 	h->root.u.def.value = s->size;
 	s->size += 16;
@@ -14670,22 +14695,25 @@ ppc64_elf_relocate_section (bfd *output_bfd,
 	  if (NO_OPD_RELOCS && is_opd)
 	    break;
 
-	  if ((bfd_link_pic (info)
-	       && (h == NULL
-		   || ELF_ST_VISIBILITY (h->elf.other) == STV_DEFAULT
-		   || h->elf.root.type != bfd_link_hash_undefweak)
-	       && (must_be_dyn_reloc (info, r_type)
-		   || !SYMBOL_CALLS_LOCAL (info, &h->elf)))
-	      || (ELIMINATE_COPY_RELOCS
-		  && !bfd_link_pic (info)
-		  && h != NULL
-		  && h->elf.dynindx != -1
-		  && !h->elf.non_got_ref
-		  && !h->elf.def_regular)
-	      || (!bfd_link_pic (info)
-		  && (h != NULL
-		      ? h->elf.type == STT_GNU_IFUNC
-		      : ELF_ST_TYPE (sym->st_info) == STT_GNU_IFUNC)))
+	  if (bfd_link_pic (info)
+	      ? ((h == NULL
+		  || ELF_ST_VISIBILITY (h->elf.other) == STV_DEFAULT
+		  || h->elf.root.type != bfd_link_hash_undefweak)
+		 && (must_be_dyn_reloc (info, r_type)
+		     || !SYMBOL_CALLS_LOCAL (info, &h->elf)))
+	      : (h == NULL
+		 ? ELF_ST_TYPE (sym->st_info) == STT_GNU_IFUNC
+		 : (h->elf.type == STT_GNU_IFUNC
+		    ? (abiversion (output_bfd) >= 2
+		       ? !(h->elf.pointer_equality_needed
+			   && !h->elf.def_regular
+			   && h->elf.root.type == bfd_link_hash_defined
+			   && h->elf.root.u.def.section == htab->glink)
+		       : !h->elf.needs_copy)
+		    : (ELIMINATE_COPY_RELOCS
+		       && !(h->elf.non_got_ref
+			    || h->elf.def_regular
+			    || h->elf.dynindx == -1)))))
 	    {
 	      bfd_boolean skip, relocate;
 	      asection *sreloc;