[PATCH] nios2: Scan dynamic relocations in read-only section

[PATCH] nios2: Scan dynamic relocations in read-only section

[H.J. Lu]

On Sat, May 30, 2020 at 11:11:48AM -0700, H.J. Lu wrote:
> On Sat, May 30, 2020 at 10:26 AM Jeff Law <law@redhat.com> wrote:
> >
> >
> > This change:
> >
> > a6dbf402de65fe66f4ec99b56527dfd00d077cb6 is the first bad commit
> > commit a6dbf402de65fe66f4ec99b56527dfd00d077cb6
> > Author: H.J. Lu <hjl.tools@gmail.com>
> > Date:   Wed May 27 04:53:54 2020 -0700
> >
> >     ld: Add --warn-textrel and obsolete --warn-shared-textrel
> >
> >     --warn-shared-textrel and -z text apply to both shared object and PIE.
> >     Add --warn-textrel and obsolete --warn-shared-textrel.  Consolidate
> >     --warn-textrel and -z text/notext/textoff implementation.
> >
> >     bfd/
> >
> >             PR ld/22909
> >             * elflink.c (bfd_elf_final_link): Use bfd_link_textrel_check.
> >             Check bfd_link_dll when issue a DT_TEXTREL warning.
> >             * elfxx-x86.c (maybe_set_textrel): Likewise.
> >             (_bfd_x86_elf_size_dynamic_sections): Likewise.
> >
> >     include/
> >
> >             PR ld/22909
> >             * bfdlink.h (textrel_check_method): New enum.
> >             (bfd_link_textrel_check): New.
> >             (bfd_link_info): Replace warn_shared_textrel and error_textrel
> >             with textrel_check.
> >
> >     ld/
> >
> >             PR ld/22909
> >             * NEWS: Mention --warn-textrel.
> >             * ld.texi: Update -z text/notext/textoff.  Add --warn-textrel.
> >             Remove --warn-shared-textrel.
> >             * ldlex.h (option_values): Rename OPTION_WARN_SHARED_TEXTREL to
> >             OPTION_WARN_TEXTREL.
> >             * lexsup.c (ld_options): Add --warn-textrel.  Obsolete
> >             --warn-shared-textrel.
> >             (parse_args): Updated.
> >             (elf_shlib_list_options): Check link_info.textrel_check.
> >             * emultempl/elf.em: Updated.
> >             * testsuite/ld-elf/pr19539.d: Replace -z notext with
> >             --warn-textrel.  Expect a warning.
> >             * testsuite/ld-i386/warn1.d: Update expected warning.
> >
> > Causes a regression for nios2-linux-gnu :
> >
> > FAIL: ld-elf/pr19539
> >
> 
> I saw
> 
> FAIL: Common symbol override test
> FAIL: ld-elf/pr19539
> FAIL: DT_TEXTREL in shared lib
> FAIL: DT_TEXTREL map file warning
> 
> Last 3 are related.  I think it is a bug in the nios2 linker backend:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=26066
> 

Add maybe_set_textrel to set DF_TEXTREL by scanning dynamic relocations
in read-only section.

This fixed:

FAIL: ld-elf/pr19539
FAIL: DT_TEXTREL in shared lib
FAIL: DT_TEXTREL map file warning

OK for master?

H.J.
--
	PR ld/26066
	* elf32-nios2.c (readonly_dynrelocs): New function.
	(maybe_set_textrel): Likewise.
	(nios2_elf32_size_dynamic_sections): Use maybe_set_textrel to
	set DF_TEXTREL.
---
 bfd/elf32-nios2.c | 81 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 71 insertions(+), 10 deletions(-)

diff --git a/bfd/elf32-nios2.c b/bfd/elf32-nios2.c
index f0cdd5203a..99f2c476e5 100644
--- a/bfd/elf32-nios2.c
+++ b/bfd/elf32-nios2.c
@@ -5724,6 +5724,63 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, PTR inf)
   return TRUE;
 }
 
+/* Find dynamic relocs for H that apply to read-only sections.  */
+
+static asection *
+readonly_dynrelocs (struct elf_link_hash_entry *h)
+{
+  struct elf_dyn_relocs *p;
+  struct elf32_nios2_link_hash_entry *eh;
+
+  eh = (struct elf32_nios2_link_hash_entry *) h;
+  for (p = eh->dyn_relocs; p != NULL; p = p->next)
+    {
+      asection *s = p->sec->output_section;
+
+      if (s != NULL && (s->flags & SEC_READONLY) != 0)
+	return p->sec;
+    }
+  return NULL;
+}
+
+/* Set DF_TEXTREL if we find any dynamic relocs that apply to
+   read-only sections.  */
+
+static bfd_boolean
+maybe_set_textrel (struct elf_link_hash_entry *h, void *inf)
+{
+  asection *sec;
+
+  if (h->root.type == bfd_link_hash_indirect)
+    return TRUE;
+
+  /* Skip local IFUNC symbols. */
+  if (h->forced_local && h->type == STT_GNU_IFUNC)
+    return TRUE;
+
+  sec = readonly_dynrelocs (h);
+  if (sec != NULL)
+    {
+      struct bfd_link_info *info = (struct bfd_link_info *) inf;
+
+      info->flags |= DF_TEXTREL;
+      /* xgettext:c-format */
+      info->callbacks->minfo (_("%pB: dynamic relocation against `%pT' "
+				"in read-only section `%pA'\n"),
+			      sec->owner, h->root.root.string, sec);
+
+      if (bfd_link_textrel_check (info))
+	/* xgettext:c-format */
+	info->callbacks->einfo (_("%P: %pB: warning: relocation against `%s' "
+				  "in read-only section `%pA'\n"),
+				sec->owner, h->root.root.string, sec);
+
+      /* Not an error, just cut short the traversal.  */
+      return FALSE;
+    }
+  return TRUE;
+}
+
 /* Implement elf_backend_size_dynamic_sections:
    Set the sizes of the dynamic sections.  */
 static bfd_boolean
@@ -5796,8 +5853,6 @@ nios2_elf32_size_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
 		{
 		  srel = elf_section_data (p->sec)->sreloc;
 		  srel->size += p->count * sizeof (Elf32_External_Rela);
-		  if ((p->sec->output_section->flags & SEC_READONLY) != 0)
-		    info->flags |= DF_TEXTREL;
 		}
 	    }
 	}
@@ -5946,17 +6001,23 @@ nios2_elf32_size_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
 	      || !add_dynamic_entry (DT_JMPREL, 0)))
 	return FALSE;
 
-      if (relocs
-	  && (!add_dynamic_entry (DT_RELA, 0)
+      if (relocs)
+	{
+	  if (!add_dynamic_entry (DT_RELA, 0)
 	      || !add_dynamic_entry (DT_RELASZ, 0)
-	      || !add_dynamic_entry (DT_RELAENT, sizeof (Elf32_External_Rela))))
-	return FALSE;
+	      || !add_dynamic_entry (DT_RELAENT,
+				     sizeof (Elf32_External_Rela)))
+	    return FALSE;
 
-      if (!bfd_link_pic (info) && !add_dynamic_entry (DT_NIOS2_GP, 0))
-	return FALSE;
+	  if ((info->flags & DF_TEXTREL) == 0)
+	    elf_link_hash_traverse (&htab->root, maybe_set_textrel, info);
 
-      if ((info->flags & DF_TEXTREL) != 0
-	  && !add_dynamic_entry (DT_TEXTREL, 0))
+	  if ((info->flags & DF_TEXTREL) != 0
+	      && !add_dynamic_entry (DT_TEXTREL, 0))
+	    return FALSE;
+	}
+
+      if (!bfd_link_pic (info) && !add_dynamic_entry (DT_NIOS2_GP, 0))
 	return FALSE;
     }
 #undef add_dynamic_entry
-- 
2.26.2

Re: [PATCH] nios2: Scan dynamic relocations in read-only section

[Nick Clifton]

Hi H.J.

> 	PR ld/26066
> 	* elf32-nios2.c (readonly_dynrelocs): New function.
> 	(maybe_set_textrel): Likewise.
> 	(nios2_elf32_size_dynamic_sections): Use maybe_set_textrel to
> 	set DF_TEXTREL.
 
Approved - please apply.

Cheers
  Nick