non_got_ref after adjust_dynamic_relocs

This patch was aimed at a FIXME in elf32-hppa.c, the ludicrous and
confusing fact that non_got_ref after adjust_dynamic_relocs in that
backend means precisely the inverse of what it means before
adjust_dynamic_relocs.  Before, when non_got_ref is set it means there
are dynamic relocs, after, if non_got_ref is clear it means "keep
dynamic relocs" and later, "has dynamic relocs".  There is a reason
why it was done that way..  Some symbols that may have dynamic
relocations pre-allocated in check_relocs turn out to not be dynamic,
and then are not seen by the backend adjust_dynamic_symbols.  We want
those symbols to lose their dynamic relocs when non-pic, so it's handy
that non_got_ref means the opposite after adjust_dynamic_relocs.  But
it's really confusing.

Most other targets, like ppc32, don't always set non_got_ref on
non-GOT references that have dynamic relocations.  This is because the
primary purpose of non_got_ref before adjust_dynamic_relocs is to flag
symbols that might need to be copied to .dynbss, and there are
relocation types that may require dyn_relocs but clearly cannot have
symbols copied into .dynbss, for example, TLS relocations.

Why do we need a flag after adjust_dynamic_relocs to say "keep
dynamic relocations"?  Well, you can discard most unwanted dyn_relocs
in the backend adjust_dynamic_relocs, and for those symbols that
aren't seen by the backend adjust_dynamic_relocs, in
allocate_dynrelocs based on a flag set by adjust_dynamic relocs,
dynamic_adjusted.  That doesn't solve all our difficulties though.
relocate_section needs to know whether a symbol has dyn_relocs, and
many targets transfer dyn_relocs to a weakdef if the symbol has one.
The transfer means relocate_section can't test dyn_relocs itself and
the weakdef field has been overwritten by that time.  So non_got_ref
is used to flag "this symbol has dynamic relocations" for
relocate_section.

Confused still?  Well, let's hope the comments I've added help clarify
things..  The patch also fixes a case where we might wrongly emit
dynamic relocations in an executable for common and undefined symbols.

	* elf32-hppa.c (elf32_hppa_adjust_dynamic_symbol): Set non_got_ref
	to keep dyn_relocs, clear to discard.  Comment.
	(allocate_dynrelocs): Always clear non_got_ref when clearing
	dyn_relocs in non-pic case.  Invert non_got_ref test.  Also test
	dynamic_adjusted and ELF_COMMON_DEF_P.  Move code deleting
	dyn_relocs on undefined syms to handle for non-pic too.
	(elf32_hppa_relocate_section): Simplify test for non-pic dyn relocs.
	* elf32-ppc.c (ppc_elf_adjust_dynamic_symbol): Set non_got_ref
	to keep dyn_relocs, clear to discard.  Comment.
	(allocate_dynrelocs): Always clear non_got_ref when clearing
	dyn_relocs in non-pic case.  Invert non_got_ref test.  Also test
	dynamic_adjusted and ELF_COMMON_DEF_P.  Move code deleting
	dyn_relocs on undefined syms to handle for non-pic too.
	(ppc_elf_relocate_section): Simplify test for non-pic dyn relocs.
	* elf64-ppc.c (ppc64_elf_adjust_dynamic_symbol): Discard
	dyn_relocs here.  Don't bother setting non_got_ref.  Comment.
	(allocate_dynrelocs): Delete special handling of non-pic ELFv2
	ifuncs.  Move code deleting dyn_relocs on undefined symbols to
	handle for non-pic too.  Don't test non_got_ref.  Do test
	dynamic_adjusted and ELF_COMMON_DEF_P.

1 files changed, 36 insertions, 39 deletions

diff --git a/bfd/elf32-hppa.c b/bfd/elf32-hppa.c
index f9629e4..7fc447e 100644
--- a/bfd/elf32-hppa.c
+++ b/bfd/elf32-hppa.c
@@ -1692,7 +1692,9 @@ elf32_hppa_adjust_dynamic_symbol (struct bfd_link_info *info,
   if (eh->type == STT_FUNC
       || eh->needs_plt)
     {
-      /* After adjust_dynamic_symbol, non_got_ref set in the non-pic
+      /* Prior to adjust_dynamic_symbol, non_got_ref set means that
+	 check_relocs generated dyn_relocs for this symbol.
+	 After adjust_dynamic_symbol, non_got_ref clear in the non-pic
 	 case means that dyn_relocs for this symbol should be
 	 discarded;  We either want the symbol to remain undefined, or
 	 we have a local definition of some sort.  The "local
@@ -1700,22 +1702,13 @@ elf32_hppa_adjust_dynamic_symbol (struct bfd_link_info *info,
 	 local definition in .dynbss.
 	 Unlike other targets, elf32-hppa.c does not define a function
 	 symbol in a non-pic executable on PLT stub code, so we don't
-	 have a local definition in that case.  dyn_relocs therefore
-	 should not be discarded for function symbols, generally.
-	 However we should discard dyn_relocs if we've decided that an
-	 undefined function symbol is local, for example due to
-	 non-default visibility, or UNDEFWEAK_NO_DYNAMIC_RELOC is
-	 true for an undefined weak symbol.  */
+	 have a local definition in that case.  */
       bfd_boolean local = (SYMBOL_CALLS_LOCAL (info, eh)
 			   || UNDEFWEAK_NO_DYNAMIC_RELOC (info, eh));
-      /* Prior to adjust_dynamic_symbol, non_got_ref set means that
-	 check_relocs set up some dyn_relocs for this symbol.
-	 The !non_got_ref term here is saying that if we didn't have
-	 any dyn_relocs set up by check_relocs, then we don't want
-	 relocate_section looking for them.
-	 FIXME: Get rid of the inversion, so non_got_ref set after
-	 dyn_relocs means we do have dyn_relocs.  */
-      eh->non_got_ref = local || !eh->non_got_ref;
+      /* Arrange to discard dyn_relocs if we've decided that a
+	 function symbol is local.  */
+      if (local)
+	eh->non_got_ref = 0;
 
       /* If the symbol is used by a plabel, we must allocate a PLT slot.
 	 The refcounts are not reliable when it has been hidden since
@@ -1773,24 +1766,17 @@ elf32_hppa_adjust_dynamic_symbol (struct bfd_link_info *info,
   /* If there are no references to this symbol that do not use the
      GOT, we don't need to generate a copy reloc.  */
   if (!eh->non_got_ref)
-    {
-      eh->non_got_ref = 1;
-      return TRUE;
-    }
+    return TRUE;
 
   /* If -z nocopyreloc was given, we won't generate them either.  */
   if (info->nocopyreloc)
-    {
-      eh->non_got_ref = 0;
-      return TRUE;
-    }
+    return TRUE;
 
   if (ELIMINATE_COPY_RELOCS
       && !readonly_dynrelocs (eh))
     {
       /* If we didn't find any dynamic relocs in read-only sections, then
 	 we'll be keeping the dynamic relocs and avoiding the copy reloc.  */
-      eh->non_got_ref = 0;
       return TRUE;
     }
 
@@ -1827,6 +1813,8 @@ elf32_hppa_adjust_dynamic_symbol (struct bfd_link_info *info,
       eh->needs_copy = 1;
     }
 
+  /* We no longer want dyn_relocs.  */
+  eh->non_got_ref = 0;
   return _bfd_elf_adjust_dynamic_copy (info, eh, sec);
 }
 
@@ -2011,8 +1999,17 @@ allocate_dynrelocs (struct elf_link_hash_entry *eh, void *inf)
   if (!htab->etab.dynamic_sections_created)
     hh->dyn_relocs = NULL;
 
+  /* Discard relocs on undefined syms with non-default visibility.  */
+  else if ((eh->root.type == bfd_link_hash_undefined
+	    && ELF_ST_VISIBILITY (eh->other) != STV_DEFAULT)
+	   || UNDEFWEAK_NO_DYNAMIC_RELOC (info, eh))
+    hh->dyn_relocs = NULL;
+
   if (hh->dyn_relocs == NULL)
-    return TRUE;
+    {
+      eh->non_got_ref = 0;
+      return TRUE;
+    }
 
   /* If this is a -Bsymbolic shared link, then we need to discard all
      space allocated for dynamic pc-relative relocs against symbols
@@ -2021,14 +2018,8 @@ allocate_dynrelocs (struct elf_link_hash_entry *eh, void *inf)
      changes.  */
   if (bfd_link_pic (info))
     {
-      /* Discard relocs on undefined syms with non-default visibility.  */
-      if ((eh->root.type == bfd_link_hash_undefined
-	   && ELF_ST_VISIBILITY (eh->other) != STV_DEFAULT)
-	  || UNDEFWEAK_NO_DYNAMIC_RELOC (info, eh))
-	hh->dyn_relocs = NULL;
-
 #if RELATIVE_DYNRELOCS
-      else if (SYMBOL_CALLS_LOCAL (info, eh))
+      if (SYMBOL_CALLS_LOCAL (info, eh))
 	{
 	  struct elf32_hppa_dyn_reloc_entry **hdh_pp;
 
@@ -2056,17 +2047,25 @@ allocate_dynrelocs (struct elf_link_hash_entry *eh, void *inf)
 	 symbols which turn out to need copy relocs or are not
 	 dynamic.  */
 
-      if (!eh->non_got_ref
-	  && !eh->def_regular)
+      if (eh->dynamic_adjusted
+	  && eh->non_got_ref
+	  && !eh->def_regular
+	  && !ELF_COMMON_DEF_P (eh))
 	{
 	  if (!ensure_undef_dynamic (info, eh))
 	    return FALSE;
 
 	  if (eh->dynindx == -1)
-	    hh->dyn_relocs = NULL;
+	    {
+	      eh->non_got_ref = 0;
+	      hh->dyn_relocs = NULL;
+	    }
 	}
       else
-	hh->dyn_relocs = NULL;
+	{
+	  eh->non_got_ref = 0;
+	  hh->dyn_relocs = NULL;
+	}
     }
 
   /* Finally, allocate space.  */
@@ -3913,9 +3912,7 @@ elf32_hppa_relocate_section (bfd *output_bfd,
 	      || (ELIMINATE_COPY_RELOCS
 		  && !bfd_link_pic (info)
 		  && hh != NULL
-		  && hh->eh.dynindx != -1
-		  && !hh->eh.non_got_ref
-		  && !hh->eh.def_regular))
+		  && hh->eh.non_got_ref))
 	    {
 	      Elf_Internal_Rela outrel;
 	      bfd_boolean skip;