This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [Patch mach-o/gas] support .previous
- From: Tristan Gingold <gingold at adacore dot com>
- To: Iain Sandoe <developer at sandoe-acoustics dot co dot uk>
- Cc: binutils Development <binutils at sourceware dot org>
- Date: Thu, 5 Jan 2012 10:10:44 +0100
- Subject: Re: [Patch mach-o/gas] support .previous
- References: <968AE063-EA11-47FB-9EE2-49E2E9A930CE@sandoe-acoustics.co.uk>
On Jan 1, 2012, at 5:11 PM, Iain Sandoe wrote:
> .previous is a nice feature and will help us with compatibility with elf-style code.
>
> OK?
I have a few questions, see below.
> Iain
>
> gas:
>
> * config/obj-macho.c (previous_section): New file-scope var.
> (obj_mach_o_parse_section): Split from obj_mach_o_section.
> (obj_mach_o_section): Use obj_mach_o_parse_section, update previous.
> (obj_mach_o_known_section): Update previous.
> (obj_mach_o_objc_section): Likewise.
> (obj_mach_o_debug_section): Likewise.
> (obj_mach_o_opt_tgt_section): Likewise.
> (obj_mach_o_get_base_section): Split from obj_mach_o_base_section.
> (obj_mach_o_base_section): Use obj_mach_o_get_base_section, update previous.
> (obj_mach_o_previous): New.
> (mach_o_pseudo_table): Add previous.
>
> gas/testsuite:
>
> * gas/mach-o/previous-1.d: New.
> * gas/mach-o/previous-1.s: New.
> * gas/mach-o/warn-prev-1.s: New.
>
> ==
>
> gas/config/obj-macho.c | 185 +++++++++++++++++++++++---------
> gas/testsuite/gas/mach-o/previous-1.d | 8 ++
> gas/testsuite/gas/mach-o/previous-1.s | 20 ++++
> gas/testsuite/gas/mach-o/warn-prev-1.s | 5 +
> 4 files changed, 167 insertions(+), 51 deletions(-)
>
> diff --git a/gas/config/obj-macho.c b/gas/config/obj-macho.c
> index 74fb0c9..abea09b 100644
> --- a/gas/config/obj-macho.c
> +++ b/gas/config/obj-macho.c
> @@ -1,5 +1,5 @@
> /* Mach-O object file format
> - Copyright 2009, 2011 Free Software Foundation, Inc.
> + Copyright 2009, 2011, 2012 Free Software Foundation, Inc.
>
> This file is part of GAS, the GNU Assembler.
>
> @@ -48,6 +48,9 @@
> /* Forward decl. */
> static segT obj_mach_o_segT_from_bfd_name (const char *nam, int must_succeed);
>
> +/* Support for .previous for improved compatibility with elf-style code. */
> +static segT previous_section;
> +
> /* TODO: Implement "-dynamic"/"-static" command line options. */
>
> static int obj_mach_o_is_static;
> @@ -175,8 +178,8 @@ collect_16char_name (char *dest, const char *msg, int require_comma)
> Not all section types and attributes are accepted by the Darwin system
> assemblers as user-specifiable - although, at present, we do here. */
>
> -static void
> -obj_mach_o_section (int ignore ATTRIBUTE_UNUSED)
> +static segT
> +obj_mach_o_parse_section (void)
> {
> char *p;
> char c;
> @@ -196,17 +199,13 @@ obj_mach_o_section (int ignore ATTRIBUTE_UNUSED)
> char segname[17];
> char sectname[17];
>
> -#ifdef md_flush_pending_output
> - md_flush_pending_output ();
> -#endif
> -
> /* Zero-length segment and section names are allowed. */
> /* Parse segment name. */
> memset (segname, 0, sizeof(segname));
> if (collect_16char_name (segname, "segment", 1))
> {
> ignore_rest_of_line ();
> - return;
> + return NULL;
> }
> input_line_pointer++; /* Skip the terminating ',' */
>
> @@ -240,7 +239,7 @@ obj_mach_o_section (int ignore ATTRIBUTE_UNUSED)
> as_bad (_("unknown or invalid section type '%s'"), p);
> p[len] = tmpc;
> ignore_rest_of_line ();
> - return;
> + return NULL;
> }
> else
> sectype_given = 1;
> @@ -278,7 +277,7 @@ obj_mach_o_section (int ignore ATTRIBUTE_UNUSED)
> as_bad (_("unknown or invalid section attribute '%s'"), p);
> p[len] = tmpc;
> ignore_rest_of_line ();
> - return;
> + return NULL;
> }
> else
> {
> @@ -297,7 +296,7 @@ obj_mach_o_section (int ignore ATTRIBUTE_UNUSED)
> {
> as_bad (_("unexpected section size information"));
> ignore_rest_of_line ();
> - return;
> + return NULL;
> }
>
> input_line_pointer++;
> @@ -307,7 +306,7 @@ obj_mach_o_section (int ignore ATTRIBUTE_UNUSED)
> {
> as_bad (_("missing sizeof_stub expression"));
> ignore_rest_of_line ();
> - return;
> + return NULL;
> }
> }
> }
> @@ -341,8 +340,8 @@ obj_mach_o_section (int ignore ATTRIBUTE_UNUSED)
> name = n;
> }
>
> - /* Sub-segments don't exists as is on Mach-O. */
> - sec = subseg_new (name, 0);
> + /* Get or create the section. */
> + sec = subseg_get (name, 0);
>
> oldflags = bfd_get_section_flags (stdoutput, sec);
> msect = bfd_mach_o_get_mach_o_section (sec);
> @@ -375,7 +374,30 @@ obj_mach_o_section (int ignore ATTRIBUTE_UNUSED)
> || msect->flags != (secattr | sectype))
> as_warn (_("Ignoring changed section attributes for %s"), name);
> }
> - demand_empty_rest_of_line ();
> +
> + return sec;
> +}
> +
> +static void
> +obj_mach_o_section (int ignore ATTRIBUTE_UNUSED)
> +{
> + segT new_seg;
> + segT old_seg = now_seg;
> +
> +#ifdef md_flush_pending_output
> + md_flush_pending_output ();
> +#endif
> +
> + new_seg = obj_mach_o_parse_section ();
> +
> + if (new_seg != NULL)
> + {
> + demand_empty_rest_of_line ();
> +
> + subseg_set (new_seg, 0);
> + if (now_seg != old_seg)
> + previous_section = old_seg;
What is the reason of this guard ? It seems to be that it prevents user to write code such as:
asm ("
.section x
[…]
.previous
");
as it would make .previous behavior undefined for the user
> + }
> }
>
> static segT
> @@ -443,17 +465,23 @@ static const char * const known_sections[] =
> static void
> obj_mach_o_known_section (int sect_index)
> {
> - segT section;
> + segT new_seg;
> + segT old_seg = now_seg;
>
> #ifdef md_flush_pending_output
> md_flush_pending_output ();
> #endif
>
> - section = obj_mach_o_segT_from_bfd_name (known_sections[sect_index], 1);
> - if (section != NULL)
> - subseg_set (section, 0);
> + new_seg = obj_mach_o_segT_from_bfd_name (known_sections[sect_index], 1);
>
> + if (new_seg != NULL)
> + {
> + subseg_set (new_seg, 0);
> + if (now_seg != old_seg)
> + previous_section = old_seg;
> + }
> /* else, we leave the section as it was; there was a fatal error anyway. */
> + demand_empty_rest_of_line ();
> }
>
> static const char * const objc_sections[] =
> @@ -491,22 +519,25 @@ static const char * const objc_sections[] =
> static void
> obj_mach_o_objc_section (int sect_index)
> {
> - segT section;
> + segT new_seg;
> + segT old_seg = now_seg;
>
> #ifdef md_flush_pending_output
> md_flush_pending_output ();
> #endif
>
> - section = obj_mach_o_segT_from_bfd_name (objc_sections[sect_index], 1);
> - if (section != NULL)
> + new_seg = obj_mach_o_segT_from_bfd_name (objc_sections[sect_index], 1);
> + if (new_seg != NULL)
> {
> obj_mach_o_seen_objc_section = 1; /* We need to ensure that certain
> sections are present and in the
> right order. */
> - subseg_set (section, 0);
> + subseg_set (new_seg, 0);
> + if (now_seg != old_seg)
> + previous_section = old_seg;
> }
> -
> /* else, we leave the section as it was; there was a fatal error anyway. */
> + demand_empty_rest_of_line ();
> }
BTW, obj_mach_o_objc_section looks furiously like obj_mach_o_known_section. Maybe we can reserve a range for objc sections and merge both implementations.
>
> /* Debug section directives. */
> @@ -536,17 +567,22 @@ static const char * const debug_sections[] =
> static void
> obj_mach_o_debug_section (int sect_index)
> {
> - segT section;
> + segT new_seg;
> + segT old_seg = now_seg;
>
> #ifdef md_flush_pending_output
> md_flush_pending_output ();
> #endif
>
> - section = obj_mach_o_segT_from_bfd_name (debug_sections[sect_index], 1);
> - if (section != NULL)
> - subseg_set (section, 0);
> -
> + new_seg = obj_mach_o_segT_from_bfd_name (debug_sections[sect_index], 1);
> + if (new_seg != NULL)
> + {
> + subseg_set (new_seg, 0);
> + if (now_seg != old_seg)
> + previous_section = old_seg;
> + }
> /* else, we leave the section as it was; there was a fatal error anyway. */
> + demand_empty_rest_of_line ();
> }
Ditto for the merge.
>
> /* This could be moved to the tc-xx files, but there is so little dependency
> @@ -584,22 +620,25 @@ static void
> obj_mach_o_opt_tgt_section (int sect_index)
> {
> const struct opt_tgt_sect *tgtsct = &tgt_sections[sect_index];
> - segT section;
> + segT new_seg;
> + segT old_seg = now_seg;
>
> #ifdef md_flush_pending_output
> md_flush_pending_output ();
> #endif
>
> - section = obj_mach_o_segT_from_bfd_name (tgtsct->name, 0);
> - if (section == NULL)
> + new_seg = obj_mach_o_segT_from_bfd_name (tgtsct->name, 0);
> + if (new_seg == NULL)
> {
> as_bad (_("%s is not used for the selected target"), tgtsct->name);
> /* Leave the section as it is. */
> }
> else
> {
> - bfd_mach_o_section *mo_sec = bfd_mach_o_get_mach_o_section (section);
> - subseg_set (section, 0);
> + bfd_mach_o_section *mo_sec = bfd_mach_o_get_mach_o_section (new_seg);
> + subseg_set (new_seg, 0);
> + if (now_seg != old_seg)
> + previous_section = old_seg;
> #if defined (TC_I386)
> mo_sec->reserved2 = tgtsct->x86_val;
> #elif defined (TC_PPC)
> @@ -608,26 +647,14 @@ obj_mach_o_opt_tgt_section (int sect_index)
> mo_sec->reserved2 = 0;
> #endif
> }
> + demand_empty_rest_of_line ();
> }
>
> -/* We don't necessarily have the three 'base' sections on mach-o.
> - Normally, we would start up with only the 'text' section defined.
> - However, even that can be suppressed with (TODO) c/l option "-n".
> - Thus, we have to be able to create all three sections on-demand. */
> -
> -static void
> -obj_mach_o_base_section (int sect_index)
> +static segT
> +obj_mach_o_get_base_section (int sect_index)
> {
> segT section;
>
> -#ifdef md_flush_pending_output
> - md_flush_pending_output ();
> -#endif
> -
> - /* We don't support numeric (or any other) qualifications on the
> - well-known section shorthands. */
> - demand_empty_rest_of_line ();
> -
> switch (sect_index)
> {
> /* Handle the three sections that are globally known within GAS.
> @@ -659,10 +686,39 @@ obj_mach_o_base_section (int sect_index)
> break;
> default:
> as_fatal (_("internal error: base section index out of range"));
> - return;
> + return NULL;
> break;
> }
> - subseg_set (section, 0);
> + return section;
> +}
> +
> +/* We don't necessarily have the three 'base' sections on mach-o.
> + Normally, we would start up with only the 'text' section defined.
> + However, even that can be suppressed with (TODO) c/l option "-n".
> + Thus, we have to be able to create all three sections on-demand. */
> +
> +static void
> +obj_mach_o_base_section (int sect_index)
> +{
> + segT new_seg;
> + segT old_seg = now_seg;
> +
> +#ifdef md_flush_pending_output
> + md_flush_pending_output ();
> +#endif
> +
> + new_seg = obj_mach_o_get_base_section (sect_index);
> + if (new_seg != NULL)
> + {
> + subseg_set (new_seg, 0);
> + if (now_seg != old_seg)
> + previous_section = old_seg;
> + }
> + /* else, we leave the section as it was; there was a fatal error anyway. */
> +
> + /* We don't support numeric (or any other) qualifications on canonical
> + section names. */
> + demand_empty_rest_of_line ();
> }
>
> /* This finishes off parsing a .comm or .lcomm statement, which both can have
> @@ -718,7 +774,31 @@ obj_mach_o_common_parse (int is_local, symbolS *symbolP,
> static void
> obj_mach_o_comm (int is_local)
> {
> + segT old_seg = now_seg;
> s_comm_internal (is_local, obj_mach_o_common_parse);
> + if (now_seg != old_seg)
> + previous_section = old_seg;
> +}
Humm, why does it set previous_section ?
> +
> +static void
> +obj_mach_o_previous (int ignore ATTRIBUTE_UNUSED)
> +{
> + segT old_prev;
> +
> +#ifdef md_flush_pending_output
> + md_flush_pending_output ();
> +#endif
> +
> + if (previous_section == 0)
> + {
> + as_warn (_(".previous, but no section switch seen; ignored"));
> + return;
> + }
> +
> + old_prev = previous_section;
> + previous_section = now_seg;
> + subseg_set (old_prev, 0);
> + demand_empty_rest_of_line ();
> }
>
> /* Set properties that apply to the whole file. At present, the only
> @@ -838,6 +918,9 @@ const pseudo_typeS mach_o_pseudo_table[] =
>
> { "section", obj_mach_o_section, 0},
>
> + /* Support the elf-style previous. */
> + { "previous", obj_mach_o_previous, 0},
> +
> /* Symbol-related. */
> { "indirect_symbol", obj_mach_o_placeholder, 0},
> { "weak_definition", obj_mach_o_placeholder, 0},
> diff --git a/gas/testsuite/gas/mach-o/previous-1.d b/gas/testsuite/gas/mach-o/previous-1.d
> new file mode 100644
> index 0000000..64d4ee5
> --- /dev/null
> +++ b/gas/testsuite/gas/mach-o/previous-1.d
> @@ -0,0 +1,8 @@
> +#objdump: -t
> +.*: +file format mach-o.*
> +#...
> +SYMBOL TABLE:
> +(00000000)?00000000 g.*0f SECT.*01 0000 \[.text\] a
> +(00000000)?00000000 g.*0f SECT.*02 0000 \[.data\] a1
> +(00000000)?00000001 g.*0f SECT.*01 0000 \[.text\] a2
> +(00000000)?00000001 g.*0f SECT.*02 0000 \[.data\] a3
> \ No newline at end of file
> diff --git a/gas/testsuite/gas/mach-o/previous-1.s b/gas/testsuite/gas/mach-o/previous-1.s
> new file mode 100644
> index 0000000..5522db7
> --- /dev/null
> +++ b/gas/testsuite/gas/mach-o/previous-1.s
> @@ -0,0 +1,20 @@
> + .text
> +
> + .globl a
> +a: .space 1
> +
> + .data
> +
> + .globl a1
> +a1: .space 1
> +
> + .previous
> +
> + .globl a2
> +a2: .space 1
> +
> + .previous
> +
> + .globl a3
> +a3: .space 1
> +
> diff --git a/gas/testsuite/gas/mach-o/warn-prev-1.s b/gas/testsuite/gas/mach-o/warn-prev-1.s
> new file mode 100644
> index 0000000..fd0c0b5
> --- /dev/null
> +++ b/gas/testsuite/gas/mach-o/warn-prev-1.s
> @@ -0,0 +1,5 @@
> +# { dg-do assemble }
> +
> + .previous
> +
> +# { dg-warning ".previous, but no section switch seen; ignored" "" { target *-*-darwin*} 3 }
>
Overall, it looks ok.
Thanks,
Tristan.