Re: [PATCH 01/15] MIPS/GAS: Add .sbss pseudo op

[Richard Sandiford <rdsandiford at googlemail dot com>]

"Maciej W. Rozycki" <macro@linux-mips.org> writes:
> 2010-10-03  Maciej W. Rozycki  <macro@linux-mips.org>
>
> 	gas/
> 	* config/tc-mips.c (mips_pseudo_table): Add "sbss".
> 	(s_change_sec): Handle it.

OK, thanks.

> +    case 'B':
> +      seg = subseg_new (".sbss", (subsegT) get_absolute_expression ());
> +      if (IS_ELF)
> +	{
> +	  bfd_set_section_flags (stdoutput, seg, SEC_ALLOC);
> +	  if (strncmp (TARGET_OS, "elf", 3) != 0)
> +	    record_alignment (seg, 4);
> +	}
> +      demand_empty_rest_of_line ();
> +      break;

So that strncmp made me go "huh?".  Especially given:

      /* On a native system other than VxWorks, sections must be aligned
	 to 16 byte boundaries.  When configured for an embedded ELF
	 target, we don't bother.  */
      if (strncmp (TARGET_OS, "elf", 3) != 0
	  && strncmp (TARGET_OS, "vxworks", 7) != 0)

Did we deliberately not extend the vxworks test to .rdata and .sdata?
Seems unlikely.  The other case of this that has a comment is:

      /* We don't need to align ELF sections to the full alignment.
	 However, Irix 5 may prefer that we align them at least to a 16
	 byte boundary.  We don't bother to align the sections if we
	 are targeted for an embedded system.  */
      if (strncmp (TARGET_OS, "elf", 3) == 0)
        return addr;
      if (align > 4)
        align = 4;

which doesn't really mesh with the "must" in the first comment.  Hm.

Anyway, I agree the patch is right to do the same thing for .sbss for
consistency.  It just seems like you've found yet another thing that
could do with a bit of cleaning.

Richard