[binutils-gdb] PR22394, hppa-linux-ld fails to emit dynamic relocations

[Alan Modra <amodra at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=127e8e9f62ed0e7145dfa1172c6253275fc150dd

commit 127e8e9f62ed0e7145dfa1172c6253275fc150dd
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Oct 31 18:13:03 2017 +1030

    PR22394, hppa-linux-ld fails to emit dynamic relocations
    
    gcc -mfast-indirect-calls emits a function pointer initialization
    without a P% (plabel) modifier.  ld does not create the necessary
    dynamic relocations for this to work.  It turns out that the problem
    is caused by the non_got_ref symbol flag.  This flag is set for
    non-pic by check_relocs to indicate that the symbol might need copy
    relocations or dynamic relocations.  Later, the backend
    adjust_dynamic_symbol clears the flag to indicate dynamic relocations
    are needed, but leaves it set when copy relocations were created.  The
    inversion in meaning is insane, but it's that way because the backend
    adjust_dynamic_symbol function doesn't get to look at all symbols..
    Anyway, the insanity works for non-function symbols.  However, the
    flag is left set on any function symbol with a dynamic relocation.
    
    This patch fixes the non_got_ref handling for function symbols, adds
    -z nocopyreloc for hppa-elf, reports where textrel occurs, and expands
    comments.  The check_relocs change just stops creation of dyn_relocs
    we always threw away later.
    
    	PR 22394
    	* elf32-hppa.c (elf32_hppa_check_relocs): Don't create dyn_relocs
    	for plabels when non-pic.
    	(maybe_set_textrel): New function.
    	(readonly_dynrelocs): Move and rewrite.
    	(elf32_hppa_adjust_dynamic_symbol): Use it.  Don't create copy
    	relocs when def_regular or -z nocopyreloc.  Handle non_got_ref
    	for functions.  Expand non_got_ref comments.
    	(elf32_hppa_size_dynamic_sections): Use maybe_set_textrel.

Diff:
---
 bfd/ChangeLog    |  12 ++++++
 bfd/elf32-hppa.c | 119 ++++++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 94 insertions(+), 37 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index ed33136..9bdc0f0 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,15 @@
+2017-11-04  Alan Modra  <amodra@gmail.com>
+
+	PR 22394
+	* elf32-hppa.c (elf32_hppa_check_relocs): Don't create dyn_relocs
+	for plabels when non-pic.
+	(maybe_set_textrel): New function.
+	(readonly_dynrelocs): Move and rewrite.
+	(elf32_hppa_adjust_dynamic_symbol): Use it.  Don't create copy
+	relocs when def_regular or -z nocopyreloc.  Handle non_got_ref
+	for functions.  Expand non_got_ref comments.
+	(elf32_hppa_size_dynamic_sections): Use maybe_set_textrel.
+
 2017-11-03  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* elf32-i386.c (elf_i386_check_relocs): Set plt.refcount to 1.
diff --git a/bfd/elf32-hppa.c b/bfd/elf32-hppa.c
index be88d87..acc96af 100644
--- a/bfd/elf32-hppa.c
+++ b/bfd/elf32-hppa.c
@@ -1210,7 +1210,9 @@ elf32_hppa_check_relocs (bfd *abfd,
 	     functions indirectly or to compare function pointers.
 	     We avoid the mess by always pointing a PLABEL into the
 	     .plt, even for local functions.  */
-	  need_entry = PLT_PLABEL | NEED_PLT | NEED_DYNREL;
+	  need_entry = PLT_PLABEL | NEED_PLT;
+	  if (bfd_link_pic (info))
+	    need_entry |= NEED_DYNREL;
 	  break;
 
 	case R_PARISC_PCREL12F:
@@ -1658,6 +1660,25 @@ elf32_hppa_hide_symbol (struct bfd_link_info *info,
     }
 }
 
+/* Find any dynamic relocs that apply to read-only sections.  */
+
+static asection *
+readonly_dynrelocs (struct elf_link_hash_entry *eh)
+{
+  struct elf32_hppa_link_hash_entry *hh;
+  struct elf32_hppa_dyn_reloc_entry *hdh_p;
+
+  hh = hppa_elf_hash_entry (eh);
+  for (hdh_p = hh->dyn_relocs; hdh_p != NULL; hdh_p = hdh_p->hdh_next)
+    {
+      asection *sec = hdh_p->sec->output_section;
+
+      if (sec != NULL && (sec->flags & SEC_READONLY) != 0)
+	return hdh_p->sec;
+    }
+  return NULL;
+}
+
 /* 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
@@ -1676,15 +1697,41 @@ 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
+	 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
+	 definition" for non-function symbols may be due to creating a
+	 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.  */
+      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;
+
       /* 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
 	 hide_symbol can be called before the plabel flag is set.  */
       if (hppa_elf_hash_entry (eh)->plabel)
 	eh->plt.refcount = 1;
 
+      /* Note that unlike some other backends, the refcount is not
+	 incremented for a non-call (and non-plabel) function reference.  */
       else if (eh->plt.refcount <= 0
-	       || SYMBOL_CALLS_LOCAL (info, eh)
-	       || UNDEFWEAK_NO_DYNAMIC_RELOC (info, eh))
+	       || local)
 	{
 	  /* The .plt entry is not needed when:
 	     a) Garbage collection has removed all references to the
@@ -1693,11 +1740,11 @@ elf32_hppa_adjust_dynamic_symbol (struct bfd_link_info *info,
 	     object, and it's not a weak definition, nor is the symbol
 	     used by a plabel relocation.  Either this object is the
 	     application or we are doing a shared symbolic link.  */
-
 	  eh->plt.offset = (bfd_vma) -1;
 	  eh->needs_plt = 0;
 	}
 
+      /* Function symbols can't have copy relocs.  */
       return TRUE;
     }
   else
@@ -1731,28 +1778,25 @@ 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)
-    return TRUE;
-
-  if (ELIMINATE_COPY_RELOCS)
     {
-      struct elf32_hppa_link_hash_entry *hh;
-      struct elf32_hppa_dyn_reloc_entry *hdh_p;
+      eh->non_got_ref = 1;
+      return TRUE;
+    }
 
-      hh = hppa_elf_hash_entry (eh);
-      for (hdh_p = hh->dyn_relocs; hdh_p != NULL; hdh_p = hdh_p->hdh_next)
-	{
-	  sec = hdh_p->sec->output_section;
-	  if (sec != NULL && (sec->flags & SEC_READONLY) != 0)
-	    break;
-	}
+  /* If -z nocopyreloc was given, we won't generate them either.  */
+  if (info->nocopyreloc)
+    {
+      eh->non_got_ref = 0;
+      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.  */
-      if (hdh_p == NULL)
-	{
-	  eh->non_got_ref = 0;
-	  return TRUE;
-	}
+      eh->non_got_ref = 0;
+      return TRUE;
     }
 
   /* We must allocate the symbol in our .dynbss section, which will
@@ -2029,28 +2073,29 @@ clobber_millicode_symbols (struct elf_link_hash_entry *eh,
   return TRUE;
 }
 
-/* Find any dynamic relocs that apply to read-only sections.  */
+/* Set DF_TEXTREL if we find any dynamic relocs that apply to
+   read-only sections.  */
 
 static bfd_boolean
-readonly_dynrelocs (struct elf_link_hash_entry *eh, void *inf)
+maybe_set_textrel (struct elf_link_hash_entry *eh, void *inf)
 {
-  struct elf32_hppa_link_hash_entry *hh;
-  struct elf32_hppa_dyn_reloc_entry *hdh_p;
+  asection *sec;
 
-  hh = hppa_elf_hash_entry (eh);
-  for (hdh_p = hh->dyn_relocs; hdh_p != NULL; hdh_p = hdh_p->hdh_next)
-    {
-      asection *sec = hdh_p->sec->output_section;
+  if (eh->root.type == bfd_link_hash_indirect)
+    return TRUE;
 
-      if (sec != NULL && (sec->flags & SEC_READONLY) != 0)
-	{
-	  struct bfd_link_info *info = inf;
+  sec = readonly_dynrelocs (eh);
+  if (sec != NULL)
+    {
+      struct bfd_link_info *info = (struct bfd_link_info *) inf;
 
-	  info->flags |= DF_TEXTREL;
+      info->flags |= DF_TEXTREL;
+      info->callbacks->minfo
+	(_("%B: dynamic relocation in read-only section `%A'\n"),
+	 sec->owner, sec);
 
-	  /* Not an error, just cut short the traversal.  */
-	  return FALSE;
-	}
+      /* Not an error, just cut short the traversal.  */
+      return FALSE;
     }
   return TRUE;
 }
@@ -2334,7 +2379,7 @@ elf32_hppa_size_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
 	  /* If any dynamic relocs apply to a read-only section,
 	     then we need a DT_TEXTREL entry.  */
 	  if ((info->flags & DF_TEXTREL) == 0)
-	    elf_link_hash_traverse (&htab->etab, readonly_dynrelocs, info);
+	    elf_link_hash_traverse (&htab->etab, maybe_set_textrel, info);
 
 	  if ((info->flags & DF_TEXTREL) != 0)
 	    {