[PATCH,plugins,head+2.21] Fix pr12365 on COFF.

[PATCH,plugins,head+2.21] Fix pr12365 on COFF.

[Dave Korn]

[-- Attachment #1: Type: text/plain, Size: 3836 bytes --]


    Hi list,

  Here are two patches needed to get the PR12365 fix to work for PE-COFF.
There are two separate problems with the current implementation, hence two
patches.

  To summarize briefly, PR12365 refers to symbols from the dummy IR BFD ending
up still in the symbol table at the final output when they haven't in fact
been generated by LTRANS when it adds new object files.

  The approach taken to solve this problem was to create the sections for the
dummy BFD with the SEC_EXCLUDE flag and output_section set to NULL, which
results in an error message like so:

> `my_bcopy' referenced in section `.text.startup' of
> /tmp/ccJ2jChg.ltrans0.ltrans.o: defined in discarded section `.text' of
> tmpdir/pr12365c.o (symbol from plugin)

  The two problems for COFF targets, and their solutions are,

#1: (coff-pr12365-crash.diff)

  When output_section is NULL, the COFF backend crashes when we turn the BFD
around from writable to readable.  It expects the output_section for each
section to be set to a valid section so it can look up the section index when
writing out the symbols from that section.  In this patch, if no output
section is set, we assume the input section is its own output section.

bfd/ChangeLog:

	* coffgen.c (coff_write_symbol): Assume input section is its own
	output section if output_section member not set.
	(coff_write_alien_symbol): Likewise.

  An alternative approach would be to start off with the dummy BFD sections
not flagged as SEC_EXCLUDE and having their output_sections set to themselves,
and only setting the flag and clearing the output_section member after we've
turned around the BFD and loaded symbols from it.

#2:  (coff-pr12365.diff)

  The code that generates the "defined in discarded section" warning only
exists in the ELF backend linker.  This patch adds a similar check in the COFF
backend linker.  Two of the existing tests needed adjusting to permit an
optional underscore prefix in the symbol name in the error message.

  Doing so revealed that PE-COFF targets are currently relying on the existing
behaviour to get reasonable values assigned to the
___RUNTIME_PSEUDO_RELOC_LIST__ and ___RUNTIME_PSEUDO_RELOC_LIST_END__ symbols,
because in the event that there are no runtime psuedo relocs and nothing else
to go in the .rdata section, the entire output section gets discarded.  The
existing behaviour leaves the symbols from the discarded section all pointing
at an offset zero relative to the place where that section /would have been/
linked into the final image, but with the new error this strategy fails.  I
adjusted the script templates to place the symbols outside the .rdata section
but referred back into it, which swerves around the issue and produces the
desired symbol values whether or not there is any .rdata.

bfd/ChangeLog:

	* cofflink.c (bfd_coff_link_input_bfd): Check for and warn about
	references to symbols defined in discarded sections.

ld/ChangeLog:

	* scripttempl/pe.sc (__rt_psrelocs_start): New symbol definition.
	(__rt_psrelocs_end): Likewise.
	(__rt_psrelocs_size): Likewise difference between the above.
	(__RUNTIME_PSEUDO_RELOC_LIST_END__): Move outside .rdata section
	immediately after end of pseudo-reloc data.
	(___RUNTIME_PSEUDO_RELOC_LIST_END___): Likewise.
	(__RUNTIME_PSEUDO_RELOC_LIST__): Move outside .rdata section and
	calculate backward from list end.
	(___RUNTIME_PSEUDO_RELOC_LIST___): Likewise.
	* scripttempl/pep.sc: Likewise.

ld/testsuite/ChangeLog:

	* ld-plugin/plugin-7.d: Allow underscore in error message.
	* ld-plugin/plugin-8.d: Likewise.

  Tested on i686-pc-cygwin, fixes PR12365 and current failures in the
ld-plugin tests and HJ's lto tests except for the ones that rely on mixed IR
and non-IR relocatable links.

  OK for head and backport to branch in a couple of days time?

    cheers,
      DaveK


[-- Attachment #2: coff-pr12365-crash.diff --]
[-- Type: text/x-c, Size: 1822 bytes --]

Index: bfd/coffgen.c
===================================================================
RCS file: /cvs/src/src/bfd/coffgen.c,v
retrieving revision 1.78
diff -p -u -r1.78 coffgen.c
--- bfd/coffgen.c	27 Apr 2010 14:42:50 -0000	1.78
+++ bfd/coffgen.c	6 May 2011 15:51:53 -0000
@@ -915,6 +915,9 @@ coff_write_symbol (bfd *abfd,
   unsigned int numaux = native->u.syment.n_numaux;
   int type = native->u.syment.n_type;
   int n_sclass = (int) native->u.syment.n_sclass;
+  asection *output_section = symbol->section->output_section
+			       ? symbol->section->output_section
+			       : symbol->section;
   void * buf;
   bfd_size_type symesz;
 
@@ -933,7 +936,7 @@ coff_write_symbol (bfd *abfd,
 
   else
     native->u.syment.n_scnum =
-      symbol->section->output_section->target_index;
+      output_section->target_index;
 
   coff_fix_symbol_name (abfd, symbol, native, string_size_p,
 			debug_string_section_p, debug_string_size_p);
@@ -990,6 +993,9 @@ coff_write_alien_symbol (bfd *abfd,
 {
   combined_entry_type *native;
   combined_entry_type dummy;
+  asection *output_section = symbol->section->output_section
+			       ? symbol->section->output_section
+			       : symbol->section;
 
   native = &dummy;
   native->u.syment.n_type = T_NULL;
@@ -1015,12 +1021,11 @@ coff_write_alien_symbol (bfd *abfd,
     }
   else
     {
-      native->u.syment.n_scnum =
-	symbol->section->output_section->target_index;
+      native->u.syment.n_scnum = output_section->target_index;
       native->u.syment.n_value = (symbol->value
 				  + symbol->section->output_offset);
       if (! obj_pe (abfd))
-	native->u.syment.n_value += symbol->section->output_section->vma;
+	native->u.syment.n_value += output_section->vma;
 
       /* Copy the any flags from the file header into the symbol.
          FIXME: Why?  */

[-- Attachment #3: coff-pr12365.diff --]
[-- Type: text/x-c, Size: 5464 bytes --]

Index: bfd/cofflink.c
===================================================================
RCS file: /cvs/src/src/bfd/cofflink.c,v
retrieving revision 1.75
diff -p -u -r1.75 cofflink.c
--- bfd/cofflink.c	13 Dec 2010 01:06:15 -0000	1.75
+++ bfd/cofflink.c	6 May 2011 17:31:54 -0000
@@ -2365,6 +2365,35 @@ _bfd_coff_link_input_bfd (struct coff_fi
 	  if (internal_relocs == NULL)
 	    return FALSE;
 
+	  /* Run through the relocs looking for relocs against symbols
+	     coming from discarded sections and complain about them.  */
+	  irel = internal_relocs;
+	  for (; irel < &internal_relocs[o->reloc_count]; irel++)
+	    {
+	      struct coff_link_hash_entry *h;
+	      asection *ps = NULL;
+	      long symndx = irel->r_symndx;
+	      if (symndx < 0)
+		continue;
+	      h = obj_coff_sym_hashes (input_bfd)[symndx];
+	      if (h == NULL)
+		continue;
+	      while (h->root.type == bfd_link_hash_indirect
+		     || h->root.type == bfd_link_hash_warning)
+		h = (struct coff_link_hash_entry *) h->root.u.i.link;
+	      if (h->root.type == bfd_link_hash_defined
+		  || h->root.type == bfd_link_hash_defweak)
+		ps = h->root.u.def.section;
+	      if (ps == NULL)
+		continue;
+	      /* Complain if definition comes from a discarded section.  */
+	      if (ps->flags & SEC_EXCLUDE)
+		(*finfo->info->callbacks->einfo)
+		  (_("%X`%s' referenced in section `%A' of %B: "
+		     "defined in discarded section `%A' of %B\n"),
+		   h->root.root.string, o, input_bfd, ps, ps->owner);
+	    }
+
 	  /* Call processor specific code to relocate the section
              contents.  */
 	  if (! bfd_coff_relocate_section (output_bfd, finfo->info,
Index: ld/scripttempl/pe.sc
===================================================================
RCS file: /cvs/src/src/ld/scripttempl/pe.sc,v
retrieving revision 1.25
diff -p -u -r1.25 pe.sc
--- ld/scripttempl/pe.sc	26 Apr 2011 15:28:05 -0000	1.25
+++ ld/scripttempl/pe.sc	6 May 2011 17:31:54 -0000
@@ -106,12 +106,15 @@ SECTIONS
   .rdata ${RELOCATING+BLOCK(__section_alignment__)} :
   {
     ${R_RDATA}
-    ${RELOCATING+___RUNTIME_PSEUDO_RELOC_LIST__ = .;}
-    ${RELOCATING+__RUNTIME_PSEUDO_RELOC_LIST__ = .;}
+    ${RELOCATING+__rt_psrelocs_start = .;}
     *(.rdata_runtime_pseudo_reloc)
-    ${RELOCATING+___RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
-    ${RELOCATING+__RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
+    ${RELOCATING+__rt_psrelocs_end = .;}
   }
+  ${RELOCATING+__rt_psrelocs_size = __rt_psrelocs_end - __rt_psrelocs_start;}
+  ${RELOCATING+___RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
+  ${RELOCATING+__RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
+  ${RELOCATING+___RUNTIME_PSEUDO_RELOC_LIST__ = . - __rt_psrelocs_size;}
+  ${RELOCATING+__RUNTIME_PSEUDO_RELOC_LIST__ = . - __rt_psrelocs_size;}
 
   .eh_frame ${RELOCATING+BLOCK(__section_alignment__)} :
   {
Index: ld/scripttempl/pep.sc
===================================================================
RCS file: /cvs/src/src/ld/scripttempl/pep.sc,v
retrieving revision 1.14
diff -p -u -r1.14 pep.sc
--- ld/scripttempl/pep.sc	26 Apr 2011 15:28:05 -0000	1.14
+++ ld/scripttempl/pep.sc	6 May 2011 17:31:54 -0000
@@ -107,12 +107,15 @@ SECTIONS
   .rdata ${RELOCATING+BLOCK(__section_alignment__)} :
   {
     ${R_RDATA}
-    ${RELOCATING+___RUNTIME_PSEUDO_RELOC_LIST__ = .;}
-    ${RELOCATING+__RUNTIME_PSEUDO_RELOC_LIST__ = .;}
+    ${RELOCATING+__rt_psrelocs_start = .;}
     *(.rdata_runtime_pseudo_reloc)
-    ${RELOCATING+___RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
-    ${RELOCATING+__RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
+    ${RELOCATING+__rt_psrelocs_end = .;}
   }
+  ${RELOCATING+__rt_psrelocs_size = __rt_psrelocs_end - __rt_psrelocs_start;}
+  ${RELOCATING+___RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
+  ${RELOCATING+__RUNTIME_PSEUDO_RELOC_LIST_END__ = .;}
+  ${RELOCATING+___RUNTIME_PSEUDO_RELOC_LIST__ = . - __rt_psrelocs_size;}
+  ${RELOCATING+__RUNTIME_PSEUDO_RELOC_LIST__ = . - __rt_psrelocs_size;}
 
   .eh_frame ${RELOCATING+BLOCK(__section_alignment__)} :
   {
Index: ld/testsuite/ld-plugin/plugin-7.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-plugin/plugin-7.d,v
retrieving revision 1.3
diff -p -u -r1.3 plugin-7.d
--- ld/testsuite/ld-plugin/plugin-7.d	18 Apr 2011 21:45:37 -0000	1.3
+++ ld/testsuite/ld-plugin/plugin-7.d	6 May 2011 17:31:54 -0000
@@ -26,6 +26,6 @@ hook called: claim_file tmpdir/func.o \[
 hook called: claim_file tmpdir/text.o \[@0/.* not claimed
 #...
 hook called: all symbols read.
-`func' referenced in section `\.text.*' of tmpdir/main.o: defined in discarded section .*
+`_?func' referenced in section `\.text.*' of tmpdir/main.o: defined in discarded section .*
 hook called: cleanup.
 #...
Index: ld/testsuite/ld-plugin/plugin-8.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-plugin/plugin-8.d,v
retrieving revision 1.3
diff -p -u -r1.3 plugin-8.d
--- ld/testsuite/ld-plugin/plugin-8.d	18 Apr 2011 21:45:37 -0000	1.3
+++ ld/testsuite/ld-plugin/plugin-8.d	6 May 2011 17:31:54 -0000
@@ -30,6 +30,6 @@ hook called: claim_file tmpdir/text.o \[
 hook called: all symbols read.
 Sym: '_?func' Resolution: LDPR_PREVAILING_DEF
 Sym: '_?func2' Resolution: LDPR_PREVAILING_DEF_IRONLY
-`func' referenced in section `\.text.*' of tmpdir/main.o: defined in discarded section .*
+`_?func' referenced in section `\.text.*' of tmpdir/main.o: defined in discarded section .*
 hook called: cleanup.
 #...

Re: [PATCH,plugins,head+2.21] Fix pr12365 on COFF.

[Alan Modra]

On Fri, May 06, 2011 at 06:37:05PM +0100, Dave Korn wrote:
>   Tested on i686-pc-cygwin, fixes PR12365 and current failures in the
> ld-plugin tests and HJ's lto tests except for the ones that rely on mixed IR
> and non-IR relocatable links.
> 
>   OK for head and backport to branch in a couple of days time?

OK.

> +	      /* Complain if definition comes from a discarded section.  */
> +	      if (ps->flags & SEC_EXCLUDE)

Strictly speaking this only tests for one class of discarded sections.
I'm happy with the test, just the comment is a little misleading.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH,plugins,head+2.21] Fix pr12365 on COFF.

[Dave Korn]

On 07/05/2011 07:13, Alan Modra wrote:
> On Fri, May 06, 2011 at 06:37:05PM +0100, Dave Korn wrote:
>>   Tested on i686-pc-cygwin, fixes PR12365 and current failures in the
>> ld-plugin tests and HJ's lto tests except for the ones that rely on mixed IR
>> and non-IR relocatable links.
>>
>>   OK for head and backport to branch in a couple of days time?
> 
> OK.
> 
>> +	      /* Complain if definition comes from a discarded section.  */
>> +	      if (ps->flags & SEC_EXCLUDE)
> 
> Strictly speaking this only tests for one class of discarded sections.
> I'm happy with the test, just the comment is a little misleading.

  Reworded to "an excluded section", and applied after remembering to
reference the PR in the changelogs.

    cheers,
      DaveK