This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [SPU] bugs in .fixup section code


Hi Alan,

No worries.  Thanks for commiting a fix.

Trevor

* Alan Modra <amodra@gmail.com> [2010-09-16 03:36]:
> Hi Trevor,
>   I was going through old email in my inbox, and I don't think I
> replied to this email.  Apologies for that, and hope to make amends by
> the patch I'm about to apply.
> 
> On Tue, Feb 02, 2010 at 06:09:12PM -0800, Trevor_Smigiel@playstation.sony.com wrote:
> > Hi,
> > 
> > I noticed a couple of issues with the .fixup code for the SPU target.
> > The patch was discussed and commited here:
> >   http://sourceware.org/ml/binutils/2009-08/msg00052.html
> > 
> > The first issues is that it allocates fixup records for debug sections.
> > I think that testing the SEC_ALLOC flag is the best way to filter them
> > and other sections out.  Is that correct
> [snip]
> 
> Yes.
> 
> > The second problem is that the contents of the fixup output section are
> > getting set too soon.  The fixup section is attached to the first input
> > bfd, which causes it to get copied to the output section early in
> > bfd_elf_final_link while it is relocating sections, but the fixup
> > section contents are still being updated while relocating later
> > sections.
> > 
> > Here are 2 approaches I'm considering to fix this.  They both have the
> > effect of calling bfd_set_section_contents for the fixup section after
> > all section relocations have been completed.
> > 
> > First approach, create the .fixup section as a dynamic section.  Set the
> > SEC_LINKER_CREATED flag and set htab->elf.dynobj to be the first input
> > bfd, where the .fixup section was created.  In this case, I am not sure
> > of all the effects of setting htab->elf.dynobj.   If there aren't any
> > issues, I think this is the slightly better solution.
> [snip]
> 
> Agreed.
> 
> > And a final question about testing.  To create a test case for this I
> > will need at least 2 source files.  Is the correct approach to write
> > explicit commands for this test in ld-spu/spu.exp?  Similar to
> > embed_test that is in the same file.
> 
> You can specify multiple source files in the .d file.  See
> ld/testsuite/ld-elf/warn1.d for an example.
> 
> 	* elf32-spu.c (spu_elf_size_sections): Omit fixups for non-alloc
> 	sections.
> 	(spu_elf_create_sections): Mark .fixup with SEC_LINKER_CREATED and
> 	set dynobj.
> 	(spu_elf_finish_dynamic_sections): New function.
> 	(elf_backend_finish_dynamic_sections): Define.
> 	
> Index: bfd/elf32-spu.c
> ===================================================================
> RCS file: /cvs/src/src/bfd/elf32-spu.c,v
> retrieving revision 1.97
> diff -u -p -r1.97 elf32-spu.c
> --- bfd/elf32-spu.c	25 Aug 2010 14:53:44 -0000	1.97
> +++ bfd/elf32-spu.c	16 Sep 2010 10:31:09 -0000
> @@ -602,9 +602,12 @@ spu_elf_create_sections (struct bfd_link
>      {
>        asection *s;
>        flagword flags;
> -      ibfd = info->input_bfds;
> -      flags = SEC_LOAD | SEC_ALLOC | SEC_READONLY | SEC_HAS_CONTENTS
> -	      | SEC_IN_MEMORY;
> +
> +      if (htab->elf.dynobj == NULL)
> +	htab->elf.dynobj = ibfd;
> +      ibfd = htab->elf.dynobj;
> +      flags = (SEC_LOAD | SEC_ALLOC | SEC_READONLY | SEC_HAS_CONTENTS
> +	       | SEC_IN_MEMORY | SEC_LINKER_CREATED);
>        s = bfd_make_section_anyway_with_flags (ibfd, ".fixup", flags);
>        if (s == NULL || !bfd_set_section_alignment (ibfd, s, 2))
>  	return FALSE;
> @@ -5096,6 +5099,13 @@ spu_elf_relocate_section (bfd *output_bf
>    return ret;
>  }
>  
> +static bfd_boolean
> +spu_elf_finish_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
> +				 struct bfd_link_info *info ATTRIBUTE_UNUSED)
> +{
> +  return TRUE;
> +}
> +
>  /* Adjust _SPUEAR_ syms to point at their overlay stubs.  */
>  
>  static int
> @@ -5402,7 +5412,8 @@ spu_elf_size_sections (bfd * output_bfd,
>  
>  	      /* If there aren't any relocs, then there's nothing more
>  	         to do.  */
> -	      if ((isec->flags & SEC_RELOC) == 0
> +	      if ((isec->flags & SEC_ALLOC) == 0
> +		  || (isec->flags & SEC_RELOC) == 0
>  		  || isec->reloc_count == 0)
>  		continue;
>  
> @@ -5457,6 +5468,7 @@ spu_elf_size_sections (bfd * output_bfd,
>  #define elf_info_to_howto			spu_elf_info_to_howto
>  #define elf_backend_count_relocs		spu_elf_count_relocs
>  #define elf_backend_relocate_section		spu_elf_relocate_section
> +#define elf_backend_finish_dynamic_sections	spu_elf_finish_dynamic_sections
>  #define elf_backend_symbol_processing		spu_elf_backend_symbol_processing
>  #define elf_backend_link_output_symbol_hook	spu_elf_output_symbol_hook
>  #define elf_backend_object_p			spu_elf_object_p
> 
> -- 
> Alan Modra
> Australia Development Lab, IBM


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]