Re: PATCH: PR 233: Check non-debug local references to discarded sections

[Alan Modra <amodra at bigpond dot net dot au>]

On Wed, Jun 30, 2004 at 10:33:50PM +0930, Alan Modra wrote:
> Hmm, I see.  We want to ignore relocs against discarded section in
> .eh_frame regardless of whether .eh_frame itself is going to be edited
> by the linker.  I guess the same goes for .stabs, where flags like
> info->traditional_format will result in ELF_INFO_TYPE_STABS never
> being set for stabs.  In other words, sec_info_type really isn't the
> right thing to test in elf_section_ignore_discarded_relocs.
> 
> The rest of the patch is OK.  I'll commit the following.
> 
> 	* elflink.c (elf_section_ignore_discarded_relocs): Don't test
> 	sec_info_type, test section name instead.

I didn't get this right.  We don't want to completely ignore relocs
against invalid symbols in these sections, just not complain about them.
.gcc_except_table is another section where we probably shouldn't
complain (PR 247).  In the process of doing this, I decided to tidy this
code a little, and add some comments..

	* elflink.c (elf_section_ignore_discarded_relocs): Revert last
	change.  Comment.
	(elf_section_complain_discarded): New function.  Handle
	.gcc_except_table too.
	(elf_link_input_bfd): Rewrite handling of relocs against symbols
	in discarded sections.
	* elf-bfd.h (elf_discarded_section): Protect macro arg.

	* ld-discard/extern.d: Update.
	* ld-discard/start.d: Update.
	* ld-discard/static.d: Update.

Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.144
diff -u -p -r1.144 elf-bfd.h
--- bfd/elf-bfd.h	28 Jun 2004 13:57:58 -0000	1.144
+++ bfd/elf-bfd.h	1 Jul 2004 09:51:38 -0000
@@ -1067,8 +1067,8 @@ struct bfd_elf_section_data
 #define elf_discarded_section(sec)				\
   (!bfd_is_abs_section (sec)					\
    && bfd_is_abs_section ((sec)->output_section)		\
-   && sec->sec_info_type != ELF_INFO_TYPE_MERGE			\
-   && sec->sec_info_type != ELF_INFO_TYPE_JUST_SYMS)
+   && (sec)->sec_info_type != ELF_INFO_TYPE_MERGE		\
+   && (sec)->sec_info_type != ELF_INFO_TYPE_JUST_SYMS)
 
 #define get_elf_backend_data(abfd) \
   ((const struct elf_backend_data *) (abfd)->xvec->backend_data)
Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.77
diff -c -p -r1.77 elflink.c
*** bfd/elflink.c	30 Jun 2004 16:19:20 -0000	1.77
--- bfd/elflink.c	1 Jul 2004 13:15:23 -0000
*************** elf_link_output_extsym (struct elf_link_
*** 6258,6275 ****
    return TRUE;
  }
  
  static bfd_boolean
  elf_section_ignore_discarded_relocs (asection *sec)
  {
    const struct elf_backend_data *bed;
  
!   if (strncmp (".stab", sec->name, 5) == 0
!       && (!sec->name[5] ||
! 	  (sec->name[5] == '.' && ISDIGIT (sec->name[6]))))
!     return TRUE;
! 
!   if (strcmp (".eh_frame", sec->name) == 0)
!     return TRUE;
  
    bed = get_elf_backend_data (sec->owner);
    if (bed->elf_backend_ignore_discarded_relocs != NULL
--- 6258,6279 ----
    return TRUE;
  }
  
+ /* Return TRUE if special handling is done for relocs in SEC against
+    symbols defined in discarded sections.  */
+ 
  static bfd_boolean
  elf_section_ignore_discarded_relocs (asection *sec)
  {
    const struct elf_backend_data *bed;
  
!   switch (sec->sec_info_type)
!     {
!     case ELF_INFO_TYPE_STABS:
!     case ELF_INFO_TYPE_EH_FRAME:
!       return TRUE;
!     default:
!       break;
!     }
  
    bed = get_elf_backend_data (sec->owner);
    if (bed->elf_backend_ignore_discarded_relocs != NULL
*************** elf_section_ignore_discarded_relocs (ase
*** 6279,6284 ****
--- 6283,6308 ----
    return FALSE;
  }
  
+ /* Return TRUE if we should complain about a reloc in SEC against a
+    symbol defined in a discarded section.  */
+ 
+ static bfd_boolean
+ elf_section_complain_discarded (asection *sec)
+ {
+   if (strncmp (".stab", sec->name, 5) == 0
+       && (!sec->name[5] ||
+ 	  (sec->name[5] == '.' && ISDIGIT (sec->name[6]))))
+     return FALSE;
+ 
+   if (strcmp (".eh_frame", sec->name) == 0)
+     return FALSE;
+ 
+   if (strcmp (".gcc_except_table", sec->name) == 0)
+     return FALSE;
+ 
+   return TRUE;
+ }
+ 
  /* Link an input file into the linker output file.  This function
     handles all the sections and relocations of the input file at once.
     This is so that we only have to read the local symbols once, and
*************** elf_link_input_bfd (struct elf_final_lin
*** 6551,6649 ****
  	  if (!elf_section_ignore_discarded_relocs (o))
  	    {
  	      Elf_Internal_Rela *rel, *relend;
  
  	      rel = internal_relocs;
  	      relend = rel + o->reloc_count * bed->s->int_rels_per_ext_rel;
  	      for ( ; rel < relend; rel++)
  		{
  		  unsigned long r_symndx = rel->r_info >> r_sym_shift;
! 		  asection *sec;
  
  		  if (r_symndx >= locsymcount
  		      || (elf_bad_symtab (input_bfd)
  			  && finfo->sections[r_symndx] == NULL))
  		    {
- 		      struct elf_link_hash_entry *h;
- 
  		      h = sym_hashes[r_symndx - extsymoff];
  		      while (h->root.type == bfd_link_hash_indirect
  			     || h->root.type == bfd_link_hash_warning)
  			h = (struct elf_link_hash_entry *) h->root.u.i.link;
  
! 		      /* Complain if the definition comes from a
! 			 discarded section.  */
! 		      sec = h->root.u.def.section;
! 		      if ((h->root.type == bfd_link_hash_defined
! 			   || h->root.type == bfd_link_hash_defweak)
! 			  && elf_discarded_section (sec))
! 			{
! 			  if ((o->flags & SEC_DEBUGGING) != 0)
! 			    {
! 			      BFD_ASSERT (r_symndx != 0);
! 			      /* Try to preserve debug information.  */
! 			      if (sec->kept_section != NULL
! 				  && sec->size == sec->kept_section->size)
! 				h->root.u.def.section
! 				  = sec->kept_section;
! 			      else
! 				memset (rel, 0, sizeof (*rel));
! 			    }
! 			  else
! 			    {
! 			      char *r_name
! 				= xstrdup (bfd_archive_filename (o->owner));
! 			      finfo->info->callbacks->error_handler
! 				(LD_DEFINITION_IN_DISCARDED_SECTION,
! 				 _("`%T' referenced in section `%s' from %s: discarded in section `%s' from %s\n"),
! 				 h->root.root.string,
! 				 h->root.root.string,
! 				 o->name, r_name,
! 				 h->root.u.def.section->name,
! 				 bfd_archive_filename (h->root.u.def.section->owner));
! 			      if (r_name)
! 				free (r_name);
! 			    }
! 			}
  		    }
  		  else
  		    {
! 		      sec = finfo->sections[r_symndx];
  
! 		      if (sec != NULL && elf_discarded_section (sec))
  			{
! 			  if ((o->flags & SEC_DEBUGGING) != 0)
! 			    {
! 			      BFD_ASSERT (r_symndx != 0);
! 			      /* Try to preserve debug information.  */
! 			      if (sec->kept_section != NULL
! 				  && sec->size == sec->kept_section->size)
! 				finfo->sections[r_symndx]
! 				  = sec->kept_section;
! 			      else
! 				{
! 				  rel->r_info &= r_type_mask;
! 				  rel->r_addend = 0;
! 				}
! 			    }
! 			  else
  			    {
! 			      static int count;
! 			      int ok;
! 			      char *buf;
! 
! 			      ok = asprintf (&buf, "local symbol %d",
! 					     count++);
! 			      if (ok <= 0)
! 				buf = (char *) "local symbol";
! 			      finfo->info->callbacks->error_handler
! 				(LD_DEFINITION_IN_DISCARDED_SECTION,
! 				 _("`%T' referenced in section `%s': discarded in section `%s' from %s\n"),
! 				 buf, buf, o->name, sec->name,
! 				 bfd_archive_filename (input_bfd));
! 			      if (ok != -1)
! 				free (buf);
  			    }
  			}
  		    }
  		}
  	    }
--- 6575,6661 ----
  	  if (!elf_section_ignore_discarded_relocs (o))
  	    {
  	      Elf_Internal_Rela *rel, *relend;
+ 	      bfd_boolean complain = elf_section_complain_discarded (o);
  
  	      rel = internal_relocs;
  	      relend = rel + o->reloc_count * bed->s->int_rels_per_ext_rel;
  	      for ( ; rel < relend; rel++)
  		{
  		  unsigned long r_symndx = rel->r_info >> r_sym_shift;
! 		  asection **ps, *sec;
! 		  struct elf_link_hash_entry *h = NULL;
! 		  const char *sym_name;
  
  		  if (r_symndx >= locsymcount
  		      || (elf_bad_symtab (input_bfd)
  			  && finfo->sections[r_symndx] == NULL))
  		    {
  		      h = sym_hashes[r_symndx - extsymoff];
  		      while (h->root.type == bfd_link_hash_indirect
  			     || h->root.type == bfd_link_hash_warning)
  			h = (struct elf_link_hash_entry *) h->root.u.i.link;
  
! 		      if (h->root.type != bfd_link_hash_defined
! 			  && h->root.type != bfd_link_hash_defweak)
! 			continue;
! 
! 		      ps = &h->root.u.def.section;
! 		      sym_name = h->root.root.string;
  		    }
  		  else
  		    {
! 		      Elf_Internal_Sym *sym = isymbuf + r_symndx;
! 		      ps = &finfo->sections[r_symndx];
! 		      sym_name = bfd_elf_local_sym_name (input_bfd, sym);
! 		    }
  
! 		  /* Complain if the definition comes from a
! 		     discarded section.  */
! 		  if ((sec = *ps) != NULL && elf_discarded_section (sec))
! 		    {
! 		      if ((o->flags & SEC_DEBUGGING) != 0)
  			{
! 			  BFD_ASSERT (r_symndx != 0);
! 
! 			  /* Try to preserve debug information.
! 			     FIXME: This is quite broken.  Modifying
! 			     the symbol here means we will be changing
! 			     all uses of the symbol, not just those in
! 			     debug sections.  The only thing that makes
! 			     this half reasonable is that debug sections
! 			     tend to come after other sections.  Of
! 			     course, that doesn't help with globals.
! 			     ??? All link-once sections of the same name
! 			     ought to define the same set of symbols, so
! 			     it would seem that globals ought to always
! 			     be defined in the kept section.  */
! 			  if (sec->kept_section != NULL
! 			      && sec->size == sec->kept_section->size)
  			    {
! 			      *ps = sec->kept_section;
! 			      continue;
  			    }
  			}
+ 		      else if (complain)
+ 			{
+ 			  finfo->info->callbacks->error_handler
+ 			    (LD_DEFINITION_IN_DISCARDED_SECTION,
+ 			     _("`%T' referenced in section `%s' of %B: "
+ 			       "defined in discarded section `%s' of %B\n"),
+ 			     sym_name,
+ 			     sym_name, o->name, input_bfd,
+ 			     sec->name, sec->owner);
+ 			}
+ 
+ 		      /* Remove the symbol reference from the reloc, but
+ 			 don't kill the reloc completely.  This is so that
+ 			 a zero value will be written into the section,
+ 			 which may have non-zero contents put there by the
+ 			 assembler.  Zero in things like an eh_frame fde
+ 			 pc_begin allows stack unwinders to recognize the
+ 			 fde as bogus.  */
+ 		      rel->r_info &= r_type_mask;
+ 		      rel->r_addend = 0;
  		    }
  		}
  	    }
Index: ld/testsuite/ld-discard/extern.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-discard/extern.d,v
retrieving revision 1.5
diff -u -p -r1.5 extern.d
--- ld/testsuite/ld-discard/extern.d	30 Jun 2004 16:19:17 -0000	1.5
+++ ld/testsuite/ld-discard/extern.d	1 Jul 2004 13:23:05 -0000
@@ -1,3 +1,3 @@
 #source: extern.s
 #ld: -T discard.ld
-#error: `data' referenced in section `\.text' from tmpdir/dump0.o: discarded in section `\.data\.exit' from tmpdir/dump0.o
+#error: `data' referenced in section `\.text' of tmpdir/dump0.o: defined in discarded section `\.data\.exit' of tmpdir/dump0.o
Index: ld/testsuite/ld-discard/start.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-discard/start.d,v
retrieving revision 1.4
diff -u -p -r1.4 start.d
--- ld/testsuite/ld-discard/start.d	30 Jun 2004 16:19:17 -0000	1.4
+++ ld/testsuite/ld-discard/start.d	1 Jul 2004 13:23:05 -0000
@@ -1,4 +1,4 @@
 #source: start.s
 #source: exit.s
 #ld: -T discard.ld
-#error: `data' referenced in section `\.text' from tmpdir/dump0.o: discarded in section `\.data\.exit' from tmpdir/dump1.o
+#error: `data' referenced in section `\.text' of tmpdir/dump0.o: defined in discarded section `\.data\.exit' of tmpdir/dump1.o
Index: ld/testsuite/ld-discard/static.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-discard/static.d,v
retrieving revision 1.4
diff -u -p -r1.4 static.d
--- ld/testsuite/ld-discard/static.d	30 Jun 2004 16:19:17 -0000	1.4
+++ ld/testsuite/ld-discard/static.d	1 Jul 2004 13:23:05 -0000
@@ -1,3 +1,3 @@
 #source: static.s
 #ld: -T discard.ld
-#error: `local symbol 0' referenced in section `\.text': discarded in section `\.data\.exit' from tmpdir/dump0.o
+#error: `\.data\.exit' referenced in section `\.text' of tmpdir/dump0.o: defined in discarded section `\.data\.exit' of tmpdir/dump0.o

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre