Re: [Patch mach-o/gas] support section stack.

[Tristan Gingold <gingold at adacore dot com>]

On Jan 1, 2012, at 6:05 PM, Iain Sandoe wrote:

> this adds support for an elf-style section stack
> 
> OK?

See comments below.

> Iain
> 
> gas:
> 
> 	* config/obj-macho.c (section_stack): New type.
> 	 (sec_stack): New file scope var.
> 	(obj_mach_o_maybe_quoted_canonical_section_name): New.
> 	(obj_mach_o_push_sect): New.
> 	(obj_mach_o_pop_sect): New.
> 	(mach_o_pseudo_table): Add pushsection, popsection.
> 
> gas/testsuite:
> 
> 	* gas/mach-o/push-pop-1.d: New.
> 	* gas/mach-o/push-pop-1.s: New.
> 	* gas/mach-o/warn-push-pop-1.s: New.
> 
> ===
> 
> gas/config/obj-macho.c                     |  164 ++++++++++++++++++++++++++++
> gas/testsuite/gas/mach-o/push-pop-1.d      |   17 +++
> gas/testsuite/gas/mach-o/push-pop-1.s      |   68 ++++++++++++
> gas/testsuite/gas/mach-o/warn-push-pop-1.s |    7 +
> 4 files changed, 256 insertions(+), 0 deletions(-)
> 
> diff --git a/gas/config/obj-macho.c b/gas/config/obj-macho.c
> index abea09b..804d42b 100644
> --- a/gas/config/obj-macho.c
> +++ b/gas/config/obj-macho.c
> @@ -45,6 +45,8 @@
> #include "mach-o/loader.h"
> #include "obj-macho.h"
> 
> +#include <string.h>
> +
> /* Forward decl.  */
> static segT obj_mach_o_segT_from_bfd_name (const char *nam, int must_succeed);
> 
> @@ -801,6 +803,165 @@ obj_mach_o_previous (int ignore ATTRIBUTE_UNUSED)
>   demand_empty_rest_of_line ();
> }
> 
> +/* Get a (possibly quoted) canonical section name.  */
> +
> +static char *
> +obj_mach_o_maybe_quoted_canonical_section_name (char *bf)

Looks like a little bit far-fetch.
So we accept '.pushsection .text' but not '.section .text' ?

You really need to document the use of BF and the return value.  Why 255 ?  A constant would be better IMHO.

> +{
> +  SKIP_WHITESPACE ();
> +  /* All of our 'canonical' section names start with a '.'.  */
> +  if (*input_line_pointer == '.')
> +    {
> +      char c;
> +      int l = 0;
> +
> +      /* There's nothing to add to a canonical section name - so we end with
> +	 space or newline.  */
> +      while ((c = *input_line_pointer) != ' '
> +	      && c != '\t'
> +	      && !is_end_of_line[(unsigned char) c])
> +	{
> +	  input_line_pointer++;
> +	  bf[l++] = c;
> +	  if (l > 255)
> +	    return NULL;
> +	}
> +      bf[l] = '\0';
> +
> +      /* We're not going to accept just '.' as a canonical section name.  */
> +      if (l == 1)
> +	{
> +	  as_bad (_("missing name"));
> +	  return NULL;
> +	}
> +
> +      return bf;
> +    }
> +  else if (*input_line_pointer == '"'
> +	    && *(input_line_pointer+1) == '.' ) /* Allow elf-like quoted name.  */
> +    {
> +      int dummy;
> +      char *sn;
> +
> +      sn = demand_copy_C_string (&dummy);
> +      if (sn == NULL)
> +	return NULL;
> +
> +      strncpy (bf, sn, 255);
> +      return bf;
> +    }
> +
> +  return NULL;
> +}
> +
> +typedef struct section_stack
> +{
> +  struct section_stack *next;
> +  segT seg;
> +  segT prev_seg;
> +} section_stack;
> +
> +static section_stack *sec_stack;
> +
> +static void
> +obj_mach_o_push_sect (int ignore ATTRIBUTE_UNUSED)
> +{
> +  char canname[256];
> +  segT new_sec = NULL;
> +  segT old_sec = now_seg;
> +  section_stack *top;
> +  char *cn;
> +
> +#ifdef md_flush_pending_output
> +  md_flush_pending_output ();
> +#endif
> +
> +  /* See if it might be a canonical name - i.e. begins with a '.'.  */
> +  cn = obj_mach_o_maybe_quoted_canonical_section_name (canname);
> +  if (cn != NULL)
> +    {
> +      /* Cater for the fact we might be creating one of the three base
> +	 sections.  */
> +      if (strcmp (canname, TEXT_SECTION_NAME) == 0)
> +	new_sec = obj_mach_o_get_base_section (1);
> +      else if (strcmp (canname, DATA_SECTION_NAME) == 0)
> +	new_sec = obj_mach_o_get_base_section (2);
> +      else if (strcmp (canname, BSS_SECTION_NAME) == 0)
> +	new_sec = obj_mach_o_get_base_section (3);
> +      else
> +	{
> +	  /* It's not mandatory for this to work, the user might want
> +	     to declare a segment starting with a '.' - thus the somewhat
> +	     convoluted logic here.  */
> +	  new_sec = obj_mach_o_segT_from_bfd_name (canname, 0);
> +	  if (new_sec != NULL)
> +	    /* We just dump the rest of the line - ignoring any elf-style
> +	       qualifiers that might have been present.  */
> +	    ignore_rest_of_line ();
> +	  else
> +	    {
> +	      new_sec = obj_mach_o_parse_section ();
> +	      if (new_sec != NULL)
> +		demand_empty_rest_of_line ();
> +	    }	
> +	}
> +    }
> +  else
> +    {
> +      new_sec = obj_mach_o_parse_section ();
> +      if (new_sec != NULL)
> +	demand_empty_rest_of_line ();
> +    }
> +
> +  if (new_sec == NULL)
> +    {
> +      as_bad (_("couldn't parse section description"));
> +      ignore_rest_of_line ();
> +      return;
> +    }
> +
> +  top = (section_stack *) xmalloc (sizeof (section_stack));
> +  if (top == NULL)
> +    {
> +      as_bad (_("internal error: failed to allocate section stack"));
> +      ignore_rest_of_line ();
> +      return;
> +    }
> +
> +  top->next = sec_stack;
> +  top->seg = old_sec;
> +  top->prev_seg = previous_section;
> +  sec_stack = top;
> +
> +  if (old_sec != new_sec)
> +    previous_section = old_sec;
> +
> +  subseg_set (new_sec, 0);
> +}
> +
> +static void
> +obj_mach_o_pop_sect (int ignore ATTRIBUTE_UNUSED)
> +{
> +  section_stack *top = sec_stack;
> +
> +#ifdef md_flush_pending_output
> +  md_flush_pending_output ();
> +#endif
> +
> +  /* pop the section stack, if there's anything on it.  */
> +  if (top == NULL)
> +    {
> +      as_warn (_(".popsection without corresponding .pushsection; ignored"));
> +      return;
> +    }
> +
> +  sec_stack = top->next;
> +  previous_section = top->prev_seg;
> +  subseg_set (top->seg, 0);
> +  xfree (top);
> +  demand_empty_rest_of_line ();
> +}
> +
> /* Set properties that apply to the whole file.  At present, the only
>    one defined, is subsections_via_symbols.  */
> 
> @@ -920,6 +1081,9 @@ const pseudo_typeS mach_o_pseudo_table[] =
> 
>   /* Support the elf-style previous.  */
>   { "previous", obj_mach_o_previous, 0},
> +  /* Support an elf-style section stack.  */
> +  { "pushsection", obj_mach_o_push_sect, 0},
> +  { "popsection", obj_mach_o_pop_sect, 0},
> 
>   /* Symbol-related.  */
>   { "indirect_symbol", obj_mach_o_placeholder, 0},
> diff --git a/gas/testsuite/gas/mach-o/push-pop-1.d b/gas/testsuite/gas/mach-o/push-pop-1.d
> new file mode 100644
> index 0000000..eb48ccc
> --- /dev/null
> +++ b/gas/testsuite/gas/mach-o/push-pop-1.d
> @@ -0,0 +1,17 @@
> +#objdump: -t
> +.*: +file format mach-o.*
> +#...
> +SYMBOL TABLE:
> +(00000000)?00000000 g.*0f SECT.*01 0000 \[.text\] a
> +(00000000)?00000000 g.*0f SECT.*02 0000 \[.debug_info\] b
> +(00000000)?00000001 g.*0f SECT.*01 0000 \[.text\] c
> +(00000000)?00000000 g.*0f SECT.*03 0000 \[__SOMEWHERE.__else\] d
> +(00000000)?00000002 g.*0f SECT.*01 0000 \[.text\] e
> +(00000000)?00000001 g.*0f SECT.*03 0000 \[__SOMEWHERE.__else\] f
> +(00000000)?00000000 g.*0f SECT.*04 0000 \[.data\] g
> +(00000000)?00000000 g.*0f SECT.*05 0000 \[.debug_abbrev\] h
> +(00000000)?00000001 g.*0f SECT.*04 0000 \[.data\] i
> +(00000000)?00000002 g.*0f SECT.*03 0000 \[__SOMEWHERE.__else\] j
> +(00000000)?00000003 g.*0f SECT.*01 0000 \[.text\] k
> +(00000000)?00000000 g.*0f SECT.*06 0000 \[.debug_aranges\] m
> +(00000000)?00000004 g.*0f SECT.*01 0000 \[.text\] n
> \ No newline at end of file
> diff --git a/gas/testsuite/gas/mach-o/push-pop-1.s b/gas/testsuite/gas/mach-o/push-pop-1.s
> new file mode 100644
> index 0000000..addfef3
> --- /dev/null
> +++ b/gas/testsuite/gas/mach-o/push-pop-1.s
> @@ -0,0 +1,68 @@
> +	.text
> +
> +	.globl a
> +a:	.space 1
> +	
> +	.pushsection .debug_info
> +
> +	.globl b
> +b:	.space 1
> +
> +	.popsection
> +
> +	.globl c
> +c:	.space 1
> +	
> +	.pushsection __SOMEWHERE,__else,regular
> +	
> +	.globl d
> +d:	.space 1
> +
> +	.popsection
> +	
> +	.globl e
> +e:	.space 1
> +
> +	.pushsection __SOMEWHERE,__else,regular
> +	
> +	.globl f
> +f:	.space 1
> +
> +	.pushsection .data
> +
> +	.globl g
> +g:	.space 1
> +
> +	.pushsection ".debug_abbrev"
> +
> +	.globl h
> +h:	.space 1
> +
> +	.popsection
> +
> +	.globl i
> +i:	.space 1
> +
> +	.popsection
> +
> +	.globl j
> +j:	.space 1
> +
> +	.popsection
> +
> +	.globl k
> +k:	.space 1
> +
> +# in a bid to make (easier) compatibility with elf code - we ignore the
> +# remainder of the line when the section named is a well-known canonical one.
> +# We can`t act on the information anyway.
> +
> +	.pushsection ".debug_aranges", "MS",@progbits,1
> +
> +	.globl m
> +m:	.space 1
> +
> +	.popsection
> +
> +	.globl n
> +n:	.space 1
> diff --git a/gas/testsuite/gas/mach-o/warn-push-pop-1.s b/gas/testsuite/gas/mach-o/warn-push-pop-1.s
> new file mode 100644
> index 0000000..a67cf27
> --- /dev/null
> +++ b/gas/testsuite/gas/mach-o/warn-push-pop-1.s
> @@ -0,0 +1,7 @@
> +# { dg-do assemble }
> +	.text
> +	.pushsection .debug_info
> +	.popsection
> +	.popsection
> +
> +# { dg-warning ".popsection without corresponding .pushsection; ignored" "" { target *-*-darwin*} 5 }
> 

Otherwise OK, but I am a bit dubious about the use of it.

Tristan.