[PATCH v3 07/26] gas: move code for object attribute parsing into obj-attr.c

Richard Earnshaw (lists) Richard.Earnshaw@arm.com
Tue Jun 17 12:56:32 GMT 2025


On 09/05/2025 16:12, Matthieu Longo wrote:
> ---

I think some justification as to why you've chosen gas/obj-attr.c over, say, gas/config/obj-elf-attr.c is needed here.  Are you intending to enable this for all supported object formats on all targets?

Iff a top-level file is justified, I'd also be inclined to use something other than obj- as the prefix.  That's normally taken to mean that it's code specific to the object file format we're targeting.

R.

>  gas/Makefile.am      |   2 +
>  gas/Makefile.in      |  13 ++-
>  gas/config/obj-elf.c | 210 +-------------------------------------
>  gas/config/obj-elf.h |   4 +-
>  gas/obj-attr.c       | 233 +++++++++++++++++++++++++++++++++++++++++++
>  gas/obj-attr.h       |  32 ++++++
>  gas/po/POTFILES.in   |   2 +
>  7 files changed, 279 insertions(+), 217 deletions(-)
>  create mode 100644 gas/obj-attr.c
>  create mode 100644 gas/obj-attr.h
> 
> diff --git a/gas/Makefile.am b/gas/Makefile.am
> index 5d0358ce345..3043dd4a92f 100644
> --- a/gas/Makefile.am
> +++ b/gas/Makefile.am
> @@ -90,6 +90,7 @@ GAS_CFILES = \
>  	literal.c \
>  	macro.c \
>  	messages.c \
> +	obj-attr.c \
>  	output-file.c \
>  	read.c \
>  	remap.c \
> @@ -129,6 +130,7 @@ HFILES = \
>  	listing.h \
>  	macro.h \
>  	obj.h \
> +	obj-attr.h \
>  	output-file.h \
>  	read.h \
>  	sb.h \
> diff --git a/gas/Makefile.in b/gas/Makefile.in
> index 1f24d4a5bbc..0cd70ae7b64 100644
> --- a/gas/Makefile.in
> +++ b/gas/Makefile.in
> @@ -175,11 +175,11 @@ am__objects_1 = app.$(OBJEXT) as.$(OBJEXT) atof-generic.$(OBJEXT) \
>  	flonum-mult.$(OBJEXT) frags.$(OBJEXT) gen-sframe.$(OBJEXT) \
>  	ginsn.$(OBJEXT) hash.$(OBJEXT) input-file.$(OBJEXT) \
>  	input-scrub.$(OBJEXT) listing.$(OBJEXT) literal.$(OBJEXT) \
> -	macro.$(OBJEXT) messages.$(OBJEXT) output-file.$(OBJEXT) \
> -	read.$(OBJEXT) remap.$(OBJEXT) sb.$(OBJEXT) \
> -	scfidw2gen.$(OBJEXT) scfi.$(OBJEXT) sframe-opt.$(OBJEXT) \
> -	stabs.$(OBJEXT) subsegs.$(OBJEXT) symbols.$(OBJEXT) \
> -	write.$(OBJEXT)
> +	macro.$(OBJEXT) messages.$(OBJEXT) obj-attr.$(OBJEXT) \
> +	output-file.$(OBJEXT) read.$(OBJEXT) remap.$(OBJEXT) \
> +	sb.$(OBJEXT) scfidw2gen.$(OBJEXT) scfi.$(OBJEXT) \
> +	sframe-opt.$(OBJEXT) stabs.$(OBJEXT) subsegs.$(OBJEXT) \
> +	symbols.$(OBJEXT) write.$(OBJEXT)
>  am_as_new_OBJECTS = $(am__objects_1)
>  am__dirstamp = $(am__leading_dot)dirstamp
>  as_new_OBJECTS = $(am_as_new_OBJECTS)
> @@ -590,6 +590,7 @@ GAS_CFILES = \
>  	literal.c \
>  	macro.c \
>  	messages.c \
> +	obj-attr.c \
>  	output-file.c \
>  	read.c \
>  	remap.c \
> @@ -628,6 +629,7 @@ HFILES = \
>  	listing.h \
>  	macro.h \
>  	obj.h \
> +	obj-attr.h \
>  	output-file.h \
>  	read.h \
>  	sb.h \
> @@ -1337,6 +1339,7 @@ distclean-compile:
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/literal.Po@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/macro.Po@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/messages.Po@am__quote@
> +@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/obj-attr.Po@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/output-file.Po@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/read.Po@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/remap.Po@am__quote@
> diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
> index b738c2dbb62..4a16ac329a8 100644
> --- a/gas/config/obj-elf.c
> +++ b/gas/config/obj-elf.c
> @@ -25,6 +25,7 @@
>  #include "obstack.h"
>  #include "dwarf2dbg.h"
>  #include "ginsn.h"
> +#include "obj-attr.h"
>  
>  #ifndef ECOFF_DEBUGGING
>  #define ECOFF_DEBUGGING 0
> @@ -2056,215 +2057,6 @@ obj_elf_vtable_entry (int ignore ATTRIBUTE_UNUSED)
>    (void) obj_elf_get_vtable_entry ();
>  }
>  
> -#define skip_whitespace(str)  do { if (is_whitespace (*(str))) ++(str); } while (0)
> -
> -static inline int
> -skip_past_char (char ** str, char c)
> -{
> -  if (**str == c)
> -    {
> -      (*str)++;
> -      return 0;
> -    }
> -  else
> -    return -1;
> -}
> -#define skip_past_comma(str) skip_past_char (str, ',')
> -
> -/* A list of attributes that have been explicitly set by the assembly code.
> -   VENDOR is the vendor id, BASE is the tag shifted right by the number
> -   of bits in MASK, and bit N of MASK is set if tag BASE+N has been set.  */
> -typedef struct recorded_attribute_info {
> -  struct recorded_attribute_info *next;
> -  obj_attr_vendor vendor;
> -  unsigned int base;
> -  unsigned long mask;
> -} recorded_attribute_info;
> -static recorded_attribute_info *recorded_attributes;
> -
> -static void
> -obj_attr_v1_rai_free (recorded_attribute_info *node)
> -{
> -  recorded_attribute_info *next;
> -  while (node != NULL)
> -    {
> -      next = node->next;
> -      free (node);
> -      node = next;
> -    }
> -}
> -
> -static void
> -obj_attr_v1_rai_enter (void)
> -{
> -  recorded_attributes = NULL;
> -}
> -
> -static void
> -obj_attr_v1_rai_exit (void)
> -{
> -  obj_attr_v1_rai_free (recorded_attributes);
> -}
> -
> -/* Record that we have seen an explicit specification of attribute TAG
> -   for vendor VENDOR.  */
> -
> -static void
> -obj_attr_v1_record_seen (obj_attr_vendor vendor, uint32_t tag)
> -{
> -  unsigned int base;
> -  unsigned long mask;
> -  recorded_attribute_info *rai;
> -
> -  base = tag / (8 * sizeof (rai->mask));
> -  mask = 1UL << (tag % (8 * sizeof (rai->mask)));
> -  for (rai = recorded_attributes; rai; rai = rai->next)
> -    if (rai->vendor == vendor && rai->base == base)
> -      {
> -	rai->mask |= mask;
> -	return;
> -      }
> -
> -  rai = XNEW (recorded_attribute_info);
> -  rai->next = recorded_attributes;
> -  rai->vendor = vendor;
> -  rai->base = base;
> -  rai->mask = mask;
> -  recorded_attributes = rai;
> -}
> -
> -/* Return true if we have seen an explicit specification of attribute TAG
> -   for vendor VENDOR.  */
> -
> -bool
> -obj_attr_v1_rai_seen (obj_attr_vendor vendor, uint32_t tag)
> -{
> -  unsigned int base;
> -  unsigned long mask;
> -  recorded_attribute_info *rai;
> -
> -  base = tag / (8 * sizeof (rai->mask));
> -  mask = 1UL << (tag % (8 * sizeof (rai->mask)));
> -  for (rai = recorded_attributes; rai; rai = rai->next)
> -    if (rai->vendor == vendor && rai->base == base)
> -      return (rai->mask & mask) != 0;
> -  return false;
> -}
> -
> -/* Parse an attribute directive for VENDOR.
> -   Returns the attribute number read, or zero on error.  */
> -
> -uint32_t
> -obj_attr_v1_process_attribute (obj_attr_vendor vendor)
> -{
> -  expressionS exp;
> -  int type;
> -  int tag;
> -  unsigned int i = 0;
> -  char *s = NULL;
> -
> -  /* Read the first number or name.  */
> -  skip_whitespace (input_line_pointer);
> -  s = input_line_pointer;
> -  if (ISDIGIT (*input_line_pointer))
> -    {
> -      expression (& exp);
> -      if (exp.X_op != O_constant)
> -	goto bad;
> -      tag = exp.X_add_number;
> -    }
> -  else
> -    {
> -      char *name;
> -
> -      /* A name may contain '_', but no other punctuation.  */
> -      for (; ISALNUM (*input_line_pointer) || *input_line_pointer == '_';
> -	   ++input_line_pointer)
> -	i++;
> -      if (i == 0)
> -	goto bad;
> -
> -      name = xmemdup0 (s, i);
> -
> -#ifndef CONVERT_SYMBOLIC_ATTRIBUTE
> -#define CONVERT_SYMBOLIC_ATTRIBUTE(a) -1
> -#endif
> -
> -      tag = CONVERT_SYMBOLIC_ATTRIBUTE (name);
> -      if (tag == -1)
> -	{
> -	  as_bad (_("Attribute name not recognised: %s"), name);
> -	  ignore_rest_of_line ();
> -	  free (name);
> -	  return 0;
> -	}
> -      free (name);
> -    }
> -
> -  type = _bfd_elf_obj_attrs_arg_type (stdoutput, vendor, tag);
> -
> -  if (skip_past_comma (&input_line_pointer) == -1)
> -    goto bad;
> -  if (type & 1)
> -    {
> -      expression (& exp);
> -      if (exp.X_op != O_constant)
> -	{
> -	  as_bad (_("expected numeric constant"));
> -	  ignore_rest_of_line ();
> -	  return 0;
> -	}
> -      i = exp.X_add_number;
> -    }
> -  if ((type & 3) == 3
> -      && skip_past_comma (&input_line_pointer) == -1)
> -    {
> -      as_bad (_("expected comma"));
> -      ignore_rest_of_line ();
> -      return 0;
> -    }
> -  if (type & 2)
> -    {
> -      int len;
> -
> -      skip_whitespace (input_line_pointer);
> -      if (*input_line_pointer != '"')
> -	goto bad_string;
> -      s = demand_copy_C_string (&len);
> -    }
> -
> -  obj_attr_v1_record_seen (vendor, tag);
> -  bool ok = false;
> -  switch (type & 3)
> -    {
> -    case 3:
> -      ok = bfd_elf_add_obj_attr_int_string (stdoutput, vendor, tag, i, s);
> -      break;
> -    case 2:
> -      ok = bfd_elf_add_obj_attr_string (stdoutput, vendor, tag, s);
> -      break;
> -    case 1:
> -      ok = bfd_elf_add_obj_attr_int (stdoutput, vendor, tag, i);
> -      break;
> -    default:
> -      abort ();
> -    }
> -  if (!ok)
> -    as_fatal (_("error adding attribute: %s"),
> -	      bfd_errmsg (bfd_get_error ()));
> -
> -  demand_empty_rest_of_line ();
> -  return tag;
> - bad_string:
> -  as_bad (_("bad string constant"));
> -  ignore_rest_of_line ();
> -  return 0;
> - bad:
> -  as_bad (_("expected <tag> , <value>"));
> -  ignore_rest_of_line ();
> -  return 0;
> -}
> -
>  /* Parse a .gnu_attribute directive.  */
>  
>  static void
> diff --git a/gas/config/obj-elf.h b/gas/config/obj-elf.h
> index 87f4dc6d3cf..36f019b35c3 100644
> --- a/gas/config/obj-elf.h
> +++ b/gas/config/obj-elf.h
> @@ -205,9 +205,7 @@ extern void obj_elf_vtable_entry (int);
>  extern struct fix * obj_elf_get_vtable_inherit (void);
>  extern struct fix * obj_elf_get_vtable_entry (void);
>  
> -/* Object attributes v1.  */
> -extern bool obj_attr_v1_rai_seen (obj_attr_vendor, uint32_t);
> -extern uint32_t obj_attr_v1_process_attribute (obj_attr_vendor);
> +#include "obj-attr.h"
>  
>  /* BFD wants to write the udata field, which is a no-no for the
>     predefined section symbols in bfd/section.c.  They are read-only.  */
> diff --git a/gas/obj-attr.c b/gas/obj-attr.c
> new file mode 100644
> index 00000000000..1b9db3ac7b1
> --- /dev/null
> +++ b/gas/obj-attr.c
> @@ -0,0 +1,233 @@
> +/* Object attributes parsing.
> +   Copyright (C) 2025 Free Software Foundation, Inc.
> +
> +   This file is part of GAS, the GNU Assembler.
> +
> +   GAS is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3, or (at your option)
> +   any later version.
> +
> +   GAS is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GAS; see the file COPYING.  If not, write to the Free
> +   Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA
> +   02110-1301, USA.  */
> +
> +#include "as.h"
> +#include "obj-attr.h"
> +#include "safe-ctype.h"
> +
> +
> +#define skip_whitespace(str)  do { if (is_whitespace (*(str))) ++(str); } while (0)
> +
> +static inline int
> +skip_past_char (char ** str, char c)
> +{
> +  if (**str == c)
> +    {
> +      (*str)++;
> +      return 0;
> +    }
> +  else
> +    return -1;
> +}
> +#define skip_past_comma(str) skip_past_char (str, ',')
> +
> +/* A list of attributes that have been explicitly set by the assembly code.
> +   VENDOR is the vendor id, BASE is the tag shifted right by the number
> +   of bits in MASK, and bit N of MASK is set if tag BASE+N has been set.  */
> +typedef struct recorded_attribute_info {
> +  struct recorded_attribute_info *next;
> +  obj_attr_vendor vendor;
> +  unsigned int base;
> +  unsigned long mask;
> +} recorded_attribute_info;
> +static recorded_attribute_info *recorded_attributes;
> +
> +static void
> +obj_attr_v1_rai_free (recorded_attribute_info *node)
> +{
> +  recorded_attribute_info *next;
> +  while (node != NULL)
> +    {
> +      next = node->next;
> +      free (node);
> +      node = next;
> +    }
> +}
> +
> +void
> +obj_attr_v1_rai_enter ()
> +{
> +  recorded_attributes = NULL;
> +}
> +
> +void
> +obj_attr_v1_rai_exit ()
> +{
> +  obj_attr_v1_rai_free (recorded_attributes);
> +}
> +
> +/* Record that we have seen an explicit specification of attribute TAG
> +   for vendor VENDOR.  */
> +
> +static void
> +obj_attr_v1_rai_mark_as_seen (obj_attr_vendor vendor, uint32_t tag)
> +{
> +  unsigned int base;
> +  unsigned long mask;
> +  recorded_attribute_info *rai;
> +
> +  base = tag / (8 * sizeof (rai->mask));
> +  mask = 1UL << (tag % (8 * sizeof (rai->mask)));
> +  for (rai = recorded_attributes; rai; rai = rai->next)
> +    if (rai->vendor == vendor && rai->base == base)
> +      {
> +	rai->mask |= mask;
> +	return;
> +      }
> +
> +  rai = XNEW (recorded_attribute_info);
> +  rai->next = recorded_attributes;
> +  rai->vendor = vendor;
> +  rai->base = base;
> +  rai->mask = mask;
> +  recorded_attributes = rai;
> +}
> +
> +/* Return true if we have seen an explicit specification of attribute TAG
> +   for vendor VENDOR.  */
> +
> +bool
> +obj_attr_v1_rai_seen (obj_attr_vendor vendor, uint32_t tag)
> +{
> +  unsigned int base;
> +  unsigned long mask;
> +  recorded_attribute_info *rai;
> +
> +  base = tag / (8 * sizeof (rai->mask));
> +  mask = 1UL << (tag % (8 * sizeof (rai->mask)));
> +  for (rai = recorded_attributes; rai; rai = rai->next)
> +    if (rai->vendor == vendor && rai->base == base)
> +      return (rai->mask & mask) != 0;
> +  return false;
> +}
> +
> +/* Parse an attribute directive for VENDOR.
> +   Returns the attribute number read, or zero on error.  */
> +
> +uint32_t
> +obj_attr_v1_process_attribute (obj_attr_vendor vendor)
> +{
> +  expressionS exp;
> +  int type;
> +  int tag;
> +  unsigned int i = 0;
> +  char *s = NULL;
> +
> +  /* Read the first number or name.  */
> +  skip_whitespace (input_line_pointer);
> +  s = input_line_pointer;
> +  if (ISDIGIT (*input_line_pointer))
> +    {
> +      expression (& exp);
> +      if (exp.X_op != O_constant)
> +	goto bad;
> +      tag = exp.X_add_number;
> +    }
> +  else
> +    {
> +      char *name;
> +
> +      /* A name may contain '_', but no other punctuation.  */
> +      for (; ISALNUM (*input_line_pointer) || *input_line_pointer == '_';
> +	   ++input_line_pointer)
> +	i++;
> +      if (i == 0)
> +	goto bad;
> +
> +      name = xmemdup0 (s, i);
> +
> +#ifndef CONVERT_SYMBOLIC_ATTRIBUTE
> +#define CONVERT_SYMBOLIC_ATTRIBUTE(a) -1
> +#endif
> +
> +      tag = CONVERT_SYMBOLIC_ATTRIBUTE (name);
> +      if (tag == -1)
> +	{
> +	  as_bad (_("Attribute name not recognised: %s"), name);
> +	  ignore_rest_of_line ();
> +	  free (name);
> +	  return 0;
> +	}
> +      free (name);
> +    }
> +
> +  type = _bfd_elf_obj_attrs_arg_type (stdoutput, vendor, tag);
> +
> +  if (skip_past_comma (&input_line_pointer) == -1)
> +    goto bad;
> +  if (type & 1)
> +    {
> +      expression (& exp);
> +      if (exp.X_op != O_constant)
> +	{
> +	  as_bad (_("expected numeric constant"));
> +	  ignore_rest_of_line ();
> +	  return 0;
> +	}
> +      i = exp.X_add_number;
> +    }
> +  if ((type & 3) == 3
> +      && skip_past_comma (&input_line_pointer) == -1)
> +    {
> +      as_bad (_("expected comma"));
> +      ignore_rest_of_line ();
> +      return 0;
> +    }
> +  if (type & 2)
> +    {
> +      int len;
> +
> +      skip_whitespace (input_line_pointer);
> +      if (*input_line_pointer != '"')
> +	goto bad_string;
> +      s = demand_copy_C_string (&len);
> +    }
> +
> +  obj_attr_v1_rai_mark_as_seen (vendor, tag);
> +  bool ok = false;
> +  switch (type & 3)
> +    {
> +    case 3:
> +      ok = bfd_elf_add_obj_attr_int_string (stdoutput, vendor, tag, i, s);
> +      break;
> +    case 2:
> +      ok = bfd_elf_add_obj_attr_string (stdoutput, vendor, tag, s);
> +      break;
> +    case 1:
> +      ok = bfd_elf_add_obj_attr_int (stdoutput, vendor, tag, i);
> +      break;
> +    default:
> +      abort ();
> +    }
> +  if (!ok)
> +    as_fatal (_("error adding attribute: %s"),
> +	      bfd_errmsg (bfd_get_error ()));
> +
> +  demand_empty_rest_of_line ();
> +  return tag;
> + bad_string:
> +  as_bad (_("bad string constant"));
> +  ignore_rest_of_line ();
> +  return 0;
> + bad:
> +  as_bad (_("expected <tag> , <value>"));
> +  ignore_rest_of_line ();
> +  return 0;
> +}
> diff --git a/gas/obj-attr.h b/gas/obj-attr.h
> new file mode 100644
> index 00000000000..3578a96c31a
> --- /dev/null
> +++ b/gas/obj-attr.h
> @@ -0,0 +1,32 @@
> +/* Object attributes parsing.
> +   Copyright (C) 2025 Free Software Foundation, Inc.
> +
> +   This file is part of GAS, the GNU Assembler.
> +
> +   GAS is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3, or (at your option)
> +   any later version.
> +
> +   GAS is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GAS; see the file COPYING.  If not, write to the Free
> +   Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA
> +   02110-1301, USA.  */
> +
> +#ifndef _OBJ_ATTR_H
> +#define _OBJ_ATTR_H
> +
> +#include "bfd/elf-bfd.h"
> +
> +/* Object attributes v1.  */
> +extern void obj_attr_v1_rai_enter (void);
> +extern void obj_attr_v1_rai_exit (void);
> +extern bool obj_attr_v1_rai_seen (obj_attr_vendor, uint32_t);
> +extern uint32_t obj_attr_v1_process_attribute (obj_attr_vendor);
> +
> +#endif /* _OBJ_ATTR_H */
> diff --git a/gas/po/POTFILES.in b/gas/po/POTFILES.in
> index c600b179e43..541051f3c9d 100644
> --- a/gas/po/POTFILES.in
> +++ b/gas/po/POTFILES.in
> @@ -249,6 +249,8 @@ literal.c
>  macro.c
>  macro.h
>  messages.c
> +obj-attr.c
> +obj-attr.h
>  obj.h
>  output-file.c
>  output-file.h



More information about the Binutils mailing list