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

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

[Maciej W. Rozycki]

Hi,

 While preparing this series of patches it became apparent to me that 
inconsistently with .sdata (and the remaining directives in this class) 
the MIPS target lacks the .sbss operator.  This does not really matter for 
ELF, but for ECOFF it means there's no way to select the .sbss section and 
e.g. set its alignment.  The following change adds the missing operator.  
The immediate effect is it makes it easier to prepare file-format 
independent tests in the testsuite, but obviously some may use it in 
regular software if they still care about an ECOFF target.

 The pseudo op is defined by some other targets already BTW.

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 to apply?

  Maciej

binutils-2.20.51-20100925-mips-gas-sbss.patch
Index: binutils-2.20.51/gas/config/tc-mips.c
===================================================================
--- binutils-2.20.51.orig/gas/config/tc-mips.c
+++ binutils-2.20.51/gas/config/tc-mips.c
@@ -1188,6 +1188,9 @@ static const pseudo_typeS mips_pseudo_ta
   {"origin", s_org, 0},
   {"repeat", s_rept, 0},
 
+  /* For MIPS this is non-standard, but we define it for consistency.  */
+  {"sbss", s_change_sec, 'B'},
+
   /* These pseudo-ops are defined in read.c, but must be overridden
      here for one reason or another.  */
   {"align", s_align, 0},
@@ -12644,6 +12647,17 @@ s_change_sec (int sec)
 	}
       demand_empty_rest_of_line ();
       break;
+
+    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;
     }
 
   auto_align = 1;

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

[Richard Sandiford]

"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

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

[Maciej W. Rozycki]

On Mon, 4 Oct 2010, Richard Sandiford wrote:

> 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.

 Committed now, thanks.

  Maciej