[SPU] bugs in .fixup section code

[Trevor_Smigiel at playstation dot sony dot com]

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, or is there anything I am
missing?

*************** spu_elf_size_sections (bfd * output_bfd,
*** 5402,5408 ****
  
              /* If there aren't any relocs, then there's nothing more
                 to do.  */
!             if ((isec->flags & SEC_RELOC) == 0
                  || isec->reloc_count == 0)
                continue;
  
--- 5410,5416 ----
  
              /* If there aren't any relocs, then there's nothing more
                 to do.  */
!             if ((isec->flags & (SEC_RELOC | SEC_ALLOC)) != (SEC_RELOC | SEC_ALLOC)
                  || isec->reloc_count == 0)
                continue;


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.

*************** spu_elf_create_sections (struct bfd_link
*** 601,611 ****
        flagword flags;
        ibfd = info->input_bfds;
        flags = SEC_LOAD | SEC_ALLOC | SEC_READONLY | SEC_HAS_CONTENTS
!             | SEC_IN_MEMORY;
        s = bfd_make_section_anyway_with_flags (ibfd, ".fixup", flags);
        if (s == NULL || !bfd_set_section_alignment (ibfd, s, 2))
        return FALSE;
        htab->sfixup = s;
      }
  
    return TRUE;
--- 601,612 ----
        flagword flags;
        ibfd = info->input_bfds;
        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;
        htab->sfixup = s;
+       htab->elf.dynobj = ibfd;
      }
  

Second approach, explicitly call bfd_set_section_contents at the end of
spu_elf_final_link.  This means bfd_set_section_contents gets called
twice for the fixup output section.  


*************** spu_elf_final_link (bfd *output_bfd, str
*** 4702,4708 ****
    if (!spu_elf_build_stubs (info))
      info->callbacks->einfo ("%F%P: can not build overlay stubs: %E\n");
  
!   return bfd_elf_final_link (output_bfd, info);
  }
  
  /* Called when not normally emitting relocs, ie. !info->relocatable
--- 4702,4716 ----
    if (!spu_elf_build_stubs (info))
      info->callbacks->einfo ("%F%P: can not build overlay stubs: %E\n");
  
!   if (!bfd_elf_final_link (output_bfd, info))
!     return FALSE;
! 
!   if (htab->params->emit_fixups
!       && !bfd_set_section_contents (output_bfd, htab->sfixup->output_section, htab->sfixup->contents,
!                                     htab->sfixup->output_offset, htab->sfixup->size))
!     return FALSE;
! 
!   return TRUE;
  }
  
  /* Called when not normally emitting relocs, ie. !info->relocatable


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.

Thanks,
Trevor