Re: [PATCH] mips: don't force the .sdata, .srdata and .lit* sections to be PROGBITS

[Richard Sandiford <rdsandiford at googlemail dot com>]

James Cowgill <James.Cowgill@imgtec.com> writes:
> Here's a patch for the first testsuite failure.
>
> Previously when writing elf files, the .sdata section was forced to be
> PROGBITS. However in some cases (eg objcopy --only-keep-debug) the
> section will have no data stored in the final file and should be set to
> NOBITS. The default section type is already setup in
> _bfd_mips_elf_special_sections so removing the assignment in
> bfd_mips_elf_section_processing should still keep the section as the
> right type.

Despite what the current comment says, I don't think we rely on the
special sections array to get the sh_type right; it's only really there
for SHF_MIPS_* stuff.  Without the special section entry we choose
between SHT_PROGBITS and SHT_NOBITS based on the bfd section flags,
via a combination of elf_fake_sections and:

	  for (i = 0; i < m->count; i++)
	    if ((m->sections[i]->flags & (SEC_LOAD | SEC_HAS_CONTENTS)) == 0)
	      /* If we aren't making room for this section, then
		 it must be SHT_NOBITS regardless of what we've
		 set via struct bfd_elf_special_section.  */
	      elf_section_type (m->sections[i]) = SHT_NOBITS;

in assign_file_positions_for_load_sections.  E.g. for:

  SECTIONS {
    .data : { BYTE(1); }
    .lit4 : { . += 4; }
    .lit8 : { . += 4; }
    .sdata : { . += 4; }
    .srdata : { . += 4; }
  }

linked on its own, the last four sections are SHT_PROGBITS before the
patch and SHT_NOBITS with just the _bfd_mips_elf_section_processing part
of your patch applied, despite the middle three having a special section
entry and .srdata still not having one (because I didn't apply that part).

Which is a long way of saying that I don't think we want to add .srdata
to the special sections array, because there are no MIPS-specific flags
or types to set.  We should get the section type right without it.

Maybe the code you're removing was there because of some weirdness in
the IRIX loader, or maybe not, but the IRIX target is pretty much dead
and no modern tools should care.

So please just drop the _bfd_mips_elf_special_sections part and
remove all three:

	  hdr->sh_type = SHT_PROGBITS;

lines from _bfd_mips_elf_section_processing.  Now that the if bodies
have only a single line, we should reformat them without braces too.
I.e.:

      if (strcmp (name, ".sdata") == 0
	  || strcmp (name, ".lit8") == 0
	  || strcmp (name, ".lit4") == 0)
	hdr->sh_flags |= SHF_ALLOC | SHF_WRITE | SHF_MIPS_GPREL;
      else if (strcmp (name, ".srdata") == 0)
	hdr->sh_flags |= SHF_ALLOC | SHF_MIPS_GPREL;
      else if (strcmp (name, ".compact_rel") == 0)
	hdr->sh_flags = 0;

(Although TBH I don't know why we need this code at all.  Which cases
does it handle that the special section array doesn't?)

The patch is OK with that change, thanks.

Richard