This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [SPU] bugs in .fixup section code
- From: Trevor_Smigiel at playstation dot sony dot com
- To: Alan Modra <amodra at gmail dot com>
- Cc: binutils <binutils at sourceware dot org>
- Date: Thu, 16 Sep 2010 11:57:35 -0700
- Subject: Re: [SPU] bugs in .fixup section code
- References: <20100203020912.GR6310@playstation.sony.com> <20100916103507.GY16874@bubble.grove.modra.org>
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