[PATCH v10 06/28] gas: implement parsing of object attributes v2

Jan Beulich jbeulich@suse.com
Fri Nov 28 14:29:02 GMT 2025


On 20.11.2025 18:58, Matthieu Longo wrote:
> --- a/bfd/elf-attrs.c
> +++ b/bfd/elf-attrs.c
> @@ -258,9 +258,171 @@ bfd_elf_set_obj_attr_contents (bfd *abfd, bfd_byte *buffer, bfd_vma size)
>    write_obj_attr_section_v1 (abfd, buffer, size);
>  }
>  
> +/* The first two tags in gnu-testing namespace are known (from the perspective
> +   of GNU ld), and so have a name and can be initialized to the default value
> +   ('0' or NULL) depending on the encoding specified on the subsection.  Any
> +   tags above 1 will be considered unknown, so will be default initialized in
> +   the same way but its status will be set to obj_attr_subsection_v2_unknown.
> +   If the tag is unknown, ld can drop it if it is inside an optional subsection,
> +   whereas ld will raise an error in a required subsection.
> +   Note: the array below has to be sorted by the tag's integer value.  */
> +static const obj_attr_info_t known_attrs_gnu_testing[] =
> +{
> +  {
> +    .tag = {"GNUTestTag_0", 0},
> +    .default_value.uint = 0,
> +  },
> +  {
> +    .tag = {"GNUTestTag_1", 1},
> +    .default_value.uint = 0,
> +  },
> +};

I think I said so before, expect perhaps elsewhere: Such initialization to 0
doesn't really need spelling out. Here the argument you (iirc) gave at that
other occasion also doesn't apply: You're still relying on part of the
structures to be initialized by the compiler (as generally
sizeof(obj_attr_value_v2_t) >= sizeof(uint32_t)).

May apply elsewhere as well, of course, ...

> +/* List of known GNU subsections.
> +   Note: this array has to be sorted using the same criteria as in
> +   _bfd_elf_obj_attr_subsection_v2_cmp().  */
> +static const known_subsection_v2_t obj_attr_v2_known_gnu_subsections[] =
> +{
> +  {
> +    /* Note: the currently set values for the subsection name, its optionality,
> +       and encoding are irrelevant for a testing subsection.  These values are
> +       unused.  This entry is only a placeholder for list of known GNU testing
> +       tags.  */
> +    .subsec_name = NULL,

... e.g. here.

> +/* To-string function for the pair <SUBSEC, TAG>.
> +   Returns the attribute information associated to TAG if it is found,
> +   or "Tag_unknown_<N>" otherwise.  */
> +const char *
> +obj_attr_v2_tag_to_string (const struct elf_backend_data *bed,
> +			   const char *subsec_name,
> +			   obj_attr_tag_t tag)
> +{
> +  const obj_attr_info_t *attr_info
> +    = obj_attr_v2_find_known_by_tag (bed, subsec_name, tag);
> +  if (attr_info != NULL)
> +    return xstrdup (attr_info->tag.name);
> +
> +  /* 33 because:
> +       "Tag_unknown_"   (12)
> +     + max(uint64_t)    (20) i.e 18,446,744,073,709,551,615
> +     + '\0'             (1)  */
> +  char *tag_s = xmalloc (33);
> +  snprintf (tag_s, 33, "Tag_unknown_%lu", tag);
> +  return tag_s;

Simply

  return xasprintf ("Tag_unknown_%lu", tag);

?

>  /* Allocate/find an object attribute.  */
> -static obj_attribute *
> -elf_new_obj_attr (bfd *abfd, obj_attr_vendor_t vendor, obj_attr_tag_t tag)
> +obj_attribute *
> +bfd_elf_new_obj_attr (bfd *abfd, obj_attr_vendor_t vendor, obj_attr_tag_t tag)
>  {
>    obj_attribute *attr;
>    obj_attribute_list *list;
> @@ -335,7 +497,7 @@ bfd_elf_add_obj_attr_int (bfd *abfd,
>  {
>    obj_attribute *attr;
>  
> -  attr = elf_new_obj_attr (abfd, vendor, tag);
> +  attr = bfd_elf_new_obj_attr (abfd, vendor, tag);
>    if (attr != NULL)
>      {
>        attr->type = bfd_elf_obj_attrs_arg_type (abfd, vendor, tag);
> @@ -378,7 +540,7 @@ elf_add_obj_attr_string (bfd *abfd, obj_attr_vendor_t vendor, obj_attr_tag_t tag
>  {
>    obj_attribute *attr;
>  
> -  attr = elf_new_obj_attr (abfd, vendor, tag);
> +  attr = bfd_elf_new_obj_attr (abfd, vendor, tag);
>    if (attr != NULL)
>      {
>        attr->type = bfd_elf_obj_attrs_arg_type (abfd, vendor, tag);
> @@ -409,7 +571,7 @@ elf_add_obj_attr_int_string (bfd *abfd,
>  {
>    obj_attribute *attr;
>  
> -  attr = elf_new_obj_attr (abfd, vendor, tag);
> +  attr = bfd_elf_new_obj_attr (abfd, vendor, tag);
>    if (attr != NULL)
>      {
>        attr->type = bfd_elf_obj_attrs_arg_type (abfd, vendor, tag);

Especially when a patch is already pretty big, such renaming could easily (and
helpfully) be broken out.

> --- a/bfd/elf-attrs.h
> +++ b/bfd/elf-attrs.h
> @@ -47,6 +47,9 @@ typedef enum obj_attr_encoding_v2
>  #define obj_attr_encoding_v2_to_u8(value) \
>    ((uint8_t) ((value) - 1))
>  
> +extern const char *
> +oav2_encoding_to_string (obj_attr_encoding_v2_t);

ATTRIBUTE_HIDDEN?

> @@ -108,6 +111,9 @@ typedef struct obj_attr_subsection_v2 {
>    struct obj_attr_v2 *last;
>  } obj_attr_subsection_v2_t;
>  
> +extern const char *
> +oav2_comprehension_to_string (bool);

And again?

> @@ -119,3 +125,36 @@ typedef struct obj_attr_subsection_list
>    /* The size of the list.  */
>    unsigned int size;
>  } obj_attr_subsection_list_t;
> +
> +typedef struct {
> +  const char *const name;

As before - the latter "const" can easily get in the way, so I'd like to
ask for it to be omitted.

> +  obj_attr_tag_t value;
> +} obj_attr_tag_info_t;
> +
> +/* Attribute information.  */
> +typedef struct {
> +  const obj_attr_tag_info_t tag;
> +  const obj_attr_value_v2_t default_value;
> +} obj_attr_info_t;
> +
> +typedef struct
> +{
> +  const char *const subsec_name;

Same here (and possibly elsewhere).

> +  const obj_attr_info_t *known_attrs;
> +  const bool optional;
> +  const obj_attr_encoding_v2_t encoding;
> +  const size_t len;
> +} known_subsection_v2_t;
> +
> +struct elf_backend_data;
> +
> +extern const known_subsection_v2_t *
> +obj_attr_v2_identify_subsection (const struct elf_backend_data *, const char*);
> +
> +extern const obj_attr_info_t *
> +obj_attr_v2_find_known_by_tag (const struct elf_backend_data *,
> +			       const char*, obj_attr_tag_t);
> +
> +extern const char *
> +obj_attr_v2_tag_to_string (const struct elf_backend_data *, const char*,
> +			   obj_attr_tag_t);

And yet more candidates?

> --- a/gas/config/obj-elf-attr.c
> +++ b/gas/config/obj-elf-attr.c
> @@ -22,8 +22,64 @@
>  
>  #ifdef TC_OBJ_ATTR
>  
> +#include "obstack.h"
>  #include "safe-ctype.h"
>  
> +/* A variant type to store information about known OAv2 identifiers.  */
> +typedef union {
> +  uint8_t u8;
> +  bool b;
> +} oav2_identifier_variant_value_t;
> +
> +typedef enum {
> +  OAv2_ASM_ID_VALUE_UNDEFINED = 0,
> +  OAv2_ASM_ID_VALUE_U8,
> +  OAv2_ASM_ID_VALUE_BOOL,
> +} oav2_identifier_variant_type_info_t;
> +
> +typedef struct {
> +  oav2_identifier_variant_value_t val;
> +  oav2_identifier_variant_type_info_t vtype;
> +} oav2_identifier_variant_t;
> +
> +typedef struct {
> +  const char *const name;
> +  const oav2_identifier_variant_t value;
> +} oav2_identifier_t;
> +
> +/* A variant type to store the argument values of an assembly directive.  */
> +struct arg_variant_t;

This forward decl isn't really needed in C; it is introduced (into global
scope) ...

> +typedef struct {
> +  size_t len;
> +  struct arg_variant_t *elts;

... by its first use.

> +/* Extract a string literal ("[^.]+") from the input.  */
> +static bool
> +extract_string_literal (arg_t *arg_out)
> +{
> +  int len;
> +  char *obstack_buf = demand_copy_C_string (&len);
> +  if (obstack_buf != NULL)
> +    {
> +      arg_out->val.string = xstrdup (obstack_buf);
> +      obstack_free (&notes, obstack_buf);
> +      arg_out->vtype = VALUE_STRING;
> +      return true;
> +    }
> +
> +  arg_out->val.string = NULL;
> +  return false;
> +}
> +
> +/* Extract an integer literal from the input.
> +   Anything matched by O_constant is considered an integer literal (see the
> +   usage of O_constant in expr.c to see all the matches.
> +   Return true on success, false otherwise. If a signedness issue is detected,
> +   'signedness_issue' is also set to true.  */
> +static bool
> +extract_integer_literal (arg_t *arg_out,
> +			 bool want_unsigned,
> +			 bool *signedness_issue)
> +{
> +  const char *cursor_begin = input_line_pointer;
> +  expressionS exp;
> +  expression_and_evaluate (&exp);
> +  if (exp.X_op != O_constant)
> +    {
> +      char backup_c = *input_line_pointer;
> +      *input_line_pointer = '\0';
> +      as_bad (_("expression '%s' does not resolve to an integer literal"),
> +	      cursor_begin);
> +      /* Restore the character pointed by the current cursor position,
> +	 otherwise '\0' misleads ignore_rest_of_line().  */
> +      *input_line_pointer = backup_c;
> +      return false;
> +    }
> +
> +  int64_t val = (int64_t) exp.X_add_number;
> +  if (want_unsigned)
> +    {
> +      if (! exp.X_unsigned && val < 0)
> +	{
> +	  as_bad (_("unexpected value %" PRId64 ", expected `unsigned integer'"
> +		    " instead"), val);
> +	  *signedness_issue = true;
> +	  return false;
> +	}
> +      arg_out->val.u64 = val;
> +      arg_out->vtype = VALUE_UNSIGNED_INTEGER;
> +    }
> +  else
> +    {
> +      arg_out->val.i64 = val;
> +      arg_out->vtype = VALUE_SIGNED_INTEGER;
> +    }
> +  return true;
> +}
> +
> +/* Extract an identifier based on the provided character matcher.  */
> +static bool
> +extract_identifier (bool (*char_predicate) (char), arg_t *arg_out)

Is the function pointer here still needed? I can only see is_tag_identifier()
being passed in.

> +#if (TC_OBJ_ATTR_v1)
> +/* Look up attribute tags defined in the backend (object attribute v1).  */
> +static bool
> +obj_attr_v1_lookup_known_attr_tag_symbol
> +  (const char *identifier  ATTRIBUTE_UNUSED,
> +   arg_token_t token_type,
> +   arg_t *val_out)
> +{
> +  gas_assert (token_type & UNSIGNED_INTEGER);
> +
> +#ifndef CONVERT_SYMBOLIC_ATTRIBUTE
> +  #define CONVERT_SYMBOLIC_ATTRIBUTE(a) -1

Nit: # always in first column please.

> +#if (TC_OBJ_ATTR_v2)
> +/* Look up attribute tags defined in the backend (object attribute v2).  */
> +static bool
> +obj_attr_v2_lookup_known_attr_tag_symbol (const char *identifier,
> +					  arg_token_t token_type,
> +					  arg_t *val_out)
> +{
> +  obj_attr_subsection_v2_t *subsec = elf_obj_attr_subsections (stdoutput).last;
> +  /* If there is no current subsection, this function was called wrongly before
> +     setting one (usually via the subsection directive).  */
> +  gas_assert (subsec != NULL);

The comment (in particular what is being said in parentheses) suggests to me that
here you're asserting correctness of user input. That would want to be an as_bad()
then, though.

> +/* Parse an argument, and set its type accordingly depending on the input
> +   value, and the constraints on the expected argument.  */
> +static bool
> +obj_attr_parse_arg (arg_token_t expected_ttype,
> +		    bool (*match_identifier) (char c),
> +		    bool resolve_identifier,
> +		    bool optional,
> +		    arg_t *arg_out)
> +{
> +  const arg_token_t low_ttype = (expected_ttype & LT_MASK);
> +
> +  if (optional && is_end_of_stmt (*input_line_pointer))
> +    {
> +      arg_out->vtype = VALUE_OPTIONAL_ABSENT;
> +      return true;
> +    }
> +
> +  /* Check whether this looks like a string literal
> +     Note: symbol look-up for string literals is not available.  */
> +  if (*input_line_pointer == '"')
> +    {
> +      bool status = extract_string_literal (arg_out);
> +      if (status && (low_ttype & STRING))
> +	return true;
> +
> +      if (status)
> +	{
> +	  char sbuffer[100];
> +	  as_bad (_("unexpected `string' \"%s\", expected %s instead"),
> +		  arg_out->val.string,
> +		  expectations_to_string (low_ttype, sbuffer, sizeof(sbuffer)));
> +	  free ((char *) arg_out->val.string);
> +	  arg_out->val.string = NULL;
> +	  arg_out->vtype = VALUE_UNDEFINED;
> +	}
> +      return false;
> +    }
> +  /* Check whether this looks like an identifier.  */
> +  else if (ISALPHA (*input_line_pointer) || *input_line_pointer == '_')

Aren't you open-coding is_tag_identifier() here? Oh, wait - it's is ALNUM()
there and ISALPHA() here. Why the difference? A symbol (identifier) can't
start with a number, can it (seeing that is_tag_identifier() is only an
alias of is_symbol())?

> +    {
> +      bool status = extract_identifier (match_identifier, arg_out);
> +      /* match_identifier() confirmed that it was the beginning of an
> +	 identifier, so we don't expect the extraction to fail.  */
> +      gas_assert (status);
> +      gas_assert (arg_out->vtype == VALUE_STRING);
> +
> +      if (status && ! (low_ttype & IDENTIFIER))
> +	{
> +	  char sbuffer[100];
> +	  as_bad (_("unexpected `identifier' \"%s\", expected %s instead"),
> +		  arg_out->val.string,
> +		  expectations_to_string (low_ttype, sbuffer, sizeof(sbuffer)));
> +	  free ((char *) arg_out->val.string);
> +	  arg_out->val.string = NULL;
> +	  arg_out->vtype = VALUE_UNDEFINED;
> +	  return false;
> +	}
> +
> +      /* In some cases, we don't want to resolve the identifier because it is the
> +	 actual value.  */
> +      if (! resolve_identifier)
> +	return true;
> +
> +      /* Move the identifier out of arg_out.  */
> +      const char *identifier = arg_out->val.string;
> +      arg_out->val.string = NULL;
> +      bool resolved = true;
> +
> +      /* The identifier is a symbol, let's try to resolve it by:
> +	 1. using the provided list of known symbols.
> +	   a) backend-independent
> +	   b) backend-specific.  */
> +      if (lookup_known_symbols (identifier, expected_ttype, arg_out))
> +	goto free_identifier;
> +
> +      /* 2. using the symbol table for this compilation unit.
> +	 Note: this is the last attempt before failure.  */
> +      if (lookup_symbol_table (identifier, low_ttype, arg_out))
> +	goto free_identifier;
> +
> +      as_bad (_("unknown identifier '%s' in this context"), identifier);
> +      arg_out->val.string = NULL;
> +      arg_out->vtype = VALUE_UNDEFINED;
> +      resolved = false;
> +
> +free_identifier:

Nit: PLease indent labels by (at least) a blank, for "diff -p" ansd alike to
not pick up labels.

> +      free ((char *) identifier);
> +      return resolved;
> +    }
> +  /* If it is neither a string nor an identifier, it must be an expression.  */
> +  else
> +    {
> +      bool signedness_issue = false;
> +      bool status = extract_integer_literal (arg_out,
> +					     (low_ttype & UNSIGNED_INTEGER),
> +					     &signedness_issue);
> +      if (status && (low_ttype & (UNSIGNED_INTEGER | SIGNED_INTEGER)))
> +	return true;
> +
> +      char sbuffer[100];
> +      if (status)
> +	as_bad (_("unexpected integer %lu, expected %s instead"),
> +		arg_out->val.u64,
> +		expectations_to_string (low_ttype, sbuffer, sizeof(sbuffer)));
> +      else if ((low_ttype & UNSIGNED_INTEGER) && signedness_issue) {}
> +	/* Already handled by extract_integer_literal(), nothing to do.  */
> +      else
> +	as_bad (_("fell back to integer literal extraction from expression, "
> +		  "but expected %s instead"),
> +		expectations_to_string (low_ttype, sbuffer, sizeof(sbuffer)));
> +      arg_out->vtype = VALUE_UNDEFINED;
> +      return false;
> +    }
> +}

This is one of the cases where imo use of "else" is particularly harmful.

> +/* Trim white spaces before a parameter.
> +   Error if it meets a parameter separator before a parameter.  */
> +static bool
> +trim_whitespaces_before_param (unsigned int n)
> +{
> +  bool error = false;
> +  do
> +    {
> +      skip_whitespace (input_line_pointer);
> +      if (*input_line_pointer == ',')
> +	{
> +	  error = true;
> +	  if (n == 0)
> +	    as_bad (_("unexpected comma before value"));
> +	  else
> +	    as_bad (_("unexpected comma before parameter %d"), n);

%u please with unsigned int.

> +	  skip_past_comma (&input_line_pointer);
> +	}
> +    }
> +  while (is_whitespace (*input_line_pointer));
> +
> +  return !error;
> +}
> +
> +/* Skip white spaces + parameter separator after a parameter.
> +   Error if it does not meet a parameter separator after a parameter.  */
> +static bool
> +skip_whitespaces_past_comma (unsigned int n)

Since it repeats: Aiui there's no plural of "whitespace". (A native speaker may
want to correct me, though.)

> +#if (TC_OBJ_ATTR_v2)
> +static bool
> +is_valid_boolean (uint64_t value)
> +{
> +  return value == 0 || value == 1;

Simply

  return value <= 1;

?

> +#if (TC_OBJ_ATTR_v2)
> +/* Parse the arguments of [vendor]_subsection directive (v2 only).  */
> +static arg_t *
> +vendor_subsection_parse_args (unsigned int nargs, ...)
> +{
> +  va_list args;
> +  va_start (args, nargs);
> +
> +  arg_t *args_out = xcalloc (nargs, sizeof (arg_t));
> +
> +  for (unsigned int n = 0; n < nargs; ++n)
> +    {
> +      if (! trim_whitespaces_before_param (n + 1))
> +	goto bad;
> +
> +      arg_t *arg_out = &args_out[n];
> +
> +      arg_token_t expected_ttype = va_arg (args, arg_token_t);
> +      arg_token_t high_ttype = (expected_ttype & HT_MASK);
> +      /* Make sure that we called the right parse_args().  */
> +      gas_assert (high_ttype == SUBSECTION_NAME
> +	       || high_ttype == SUBSECTION_OPTION_1
> +	       || high_ttype == SUBSECTION_OPTION_2);

Nit: Indentation. Also in the gas_assert() in obj_attr_v1_record(), as I
noticed only later.

> +      if (high_ttype == SUBSECTION_NAME)
> +	{
> +	  if (! obj_attr_parse_arg (expected_ttype, is_subsection_identifier,
> +				    false, false, arg_out))
> +	    {
> +	      as_bad (_("expected <subsection_name>, <comprehension>, "
> +			"<encoding>"));
> +	      goto bad;
> +	    }
> +	}
> +      else if (high_ttype == SUBSECTION_OPTION_1
> +	    || high_ttype == SUBSECTION_OPTION_2)
> +	{
> +	  if (! obj_attr_parse_arg (expected_ttype, is_symbol, true, true,
> +				    arg_out))
> +	    goto bad;
> +
> +	  if (arg_out->vtype == VALUE_OPTIONAL_ABSENT)
> +	    continue;
> +
> +	  if (high_ttype == SUBSECTION_OPTION_1
> +	      && ! is_valid_comprehension (arg_out->val.u64))
> +	    {
> +	      as_bad (("invalid value %lu, expected values for <comprehension> "
> +		       "are 0 (=`required') or 1 (=`optional')"), arg_out->val.u64);
> +	      goto bad;
> +	    }
> +	  else if (high_ttype == SUBSECTION_OPTION_2
> +		&& ! is_valid_encoding (arg_out->val.u64))
> +	    {
> +	      as_bad (("invalid value %lu, expected values for <encoding> are 0"
> +		       " (=`ULEB128') or 1 (=`NTBS')"), arg_out->val.u64);
> +	      goto bad;
> +	    }
> +	}
> +      else
> +	abort ();
> +
> +      if (n + 1 < nargs
> +	  && ! is_end_of_stmt (*input_line_pointer)
> +	  && ! skip_whitespaces_past_comma (n + 1))
> +	goto bad;
> +    }
> +
> +  va_end (args);
> +  if (! demand_empty_rest_of_line ())
> +    goto bad_no_line_claim;

Does the return value really need checking here? Assembly will fail anyway
due to the as_bad() there. And without this you then also won't need to
alter demand_empty_rest_of_line() itself. It really wants to keep returning
void.

> +/* Record a subsection (object attribute v2 only).
> +   Note: this function takes the ownership of 'name', so is responsible to free
> +   it if an issue occurs.  */
> +static void
> +obj_attr_v2_subsection_record (const char *name,
> +			       arg_t *arg_comprehension,
> +			       arg_t *arg_encoding)
> +{
> +  obj_attr_subsection_v2_t *already_recorded_subsec
> +    = obj_attr_subsection_v2_find_by_name
> +      (elf_obj_attr_subsections (stdoutput).first, name, false);
> +
> +  bool comprehension_optional = arg_comprehension->val.u64;
> +  obj_attr_encoding_v2_t encoding
> +    = obj_attr_encoding_v2_from_u8 (arg_encoding->val.u64);
> +
> +  if (already_recorded_subsec != NULL)
> +    {
> +      bool error_redeclaration = false;
> +
> +      if (arg_comprehension->vtype == VALUE_OPTIONAL_ABSENT)
> +	gas_assert (arg_encoding->vtype == VALUE_OPTIONAL_ABSENT);
> +      else if (comprehension_optional != already_recorded_subsec->optional)
> +	error_redeclaration = true;
> +
> +      if (arg_encoding->vtype != VALUE_OPTIONAL_ABSENT
> +	  && encoding != already_recorded_subsec->encoding)
> +	error_redeclaration = true;
> +
> +      /* Check for mismatching redefinition of the subsection, i.e. the names
> +	 match but the properties are different.  */
> +      if (error_redeclaration)
> +	{
> +	  const char *prev_comprehension = oav2_comprehension_to_string (
> +	    already_recorded_subsec->optional);
> +	  const char *prev_encoding = oav2_encoding_to_string (
> +	    already_recorded_subsec->encoding);
> +	  as_bad (_("incompatible redeclaration of subsection %s.  Previous "
> +		    "declaration had properties: %s=%s, %s=%s"),
> +		    name, "comprehension", prev_comprehension,
> +		    "encoding", prev_encoding);
> +	  goto error;
> +	}
> +
> +      /* Move the existing subsection to the last position.  */
> +      LINKED_LIST_REMOVE(obj_attr_subsection_v2_t)
> +	(&elf_obj_attr_subsections (stdoutput), already_recorded_subsec);
> +      LINKED_LIST_APPEND(obj_attr_subsection_v2_t)
> +	(&elf_obj_attr_subsections (stdoutput), already_recorded_subsec);
> +      /* Note: 'name' was unused, and will be freed on exit.  */
> +    }

>From here (which apparently isn't an error path) ...

> +  else
> +    {
> +      if (arg_comprehension->vtype == VALUE_OPTIONAL_ABSENT
> +	  || arg_encoding->vtype == VALUE_OPTIONAL_ABSENT)
> +	{
> +	  as_bad (_("comprehension and encoding of a subsection cannot be "
> +		    "omitted on the first declaration"));
> +	  goto error;
> +	}
> +
> +      obj_attr_subsection_scope_v2_t scope
> +	= bfd_elf_obj_attr_subsection_v2_scope (stdoutput, name);
> +
> +      /* Note: ownership of 'name' is transfered to the callee when initializing
> +	 the subsection.  That is why we skip free() at the end.  */
> +      obj_attr_subsection_v2_t *new_subsection
> +	= _bfd_elf_obj_attr_subsection_v2_init (name, scope,
> +						comprehension_optional,
> +						encoding);
> +      LINKED_LIST_APPEND(obj_attr_subsection_v2_t)
> +	(&elf_obj_attr_subsections (stdoutput), new_subsection);
> +      return;
> +    }
> +
> + error:

... you fall through to here, which is an error path if the label name is to
be trusted.

> +/* Parse an attribute directive (supports both v1 & v2).  */
> +obj_attr_tag_t
> +obj_attr_process_attribute (obj_attr_vendor_t vendor)
> +{
> +  obj_attr_version_t version = elf_obj_attr_version (stdoutput);
> +
> +  obj_attr_subsection_v2_t *subsec = NULL;
> +#if (TC_OBJ_ATTR_v2)

Please can the blank line be after the declarations, not in the middle of them?

> --- a/gas/config/obj-elf-attr.h
> +++ b/gas/config/obj-elf-attr.h
> @@ -23,12 +23,34 @@
>  
>  #include "as.h"
>  
> +#ifndef TC_OBJ_ATTR_V1
> +#define TC_OBJ_ATTR_V1 0
> +#endif
> +#ifndef TC_OBJ_ATTR_V2
> +#define TC_OBJ_ATTR_V2 0
> +#endif
> +
> +#if (TC_OBJ_ATTR_v1 || TC_OBJ_ATTR_v2)
> +  #define TC_OBJ_ATTR 1

Same remark as before towards the placement of #.

> @@ -2493,6 +2507,8 @@ const pseudo_typeS md_pseudo_table[] = {
>    {"tlsdesccall", s_tlsdesccall, 0},
>    {"tlsdescldr", s_tlsdescldr, 0},
>    {"variant_pcs", s_variant_pcs, 0},
> +  {"aeabi_subsection", s_aarch64_aeabi_subsection, 0},
> +  {"aeabi_attribute", s_aarch64_aeabi_attribute, 0},

Ths introduction 

> --- a/gas/read.c
> +++ b/gas/read.c
> @@ -4056,15 +4056,17 @@ s_weakref (int ignore ATTRIBUTE_UNUSED)
>     dereference input_line_pointer unconditionally.  Note that when the
>     gas parser is switched to handling a string (where buffer_limit
>     should be the size of the string excluding the NUL terminator) this
> -   will be one past the NUL; is_end_of_line(0) returns true.  */
> +   will be one past the NUL; is_end_of_line(0) returns true.
> +   Return true on success, false when if junks at the end of line is found.  */
>  
> -void
> +bool
>  demand_empty_rest_of_line (void)
>  {
> +  bool ret = true;
>    SKIP_WHITESPACE ();
>    if (input_line_pointer > buffer_limit)
> -    return;
> -  if (is_end_of_stmt (*input_line_pointer))
> +    {}
> +  else if (is_end_of_stmt (*input_line_pointer))
>      input_line_pointer++;
>    else
>      {
> @@ -4075,8 +4077,10 @@ demand_empty_rest_of_line (void)
>  	as_bad (_("junk at end of line, first unrecognized character valued 0x%x"),
>  		 *input_line_pointer);
>        ignore_rest_of_line ();
> +      ret = false;
>      }
>    /* Return pointing just after end-of-line.  */
> +  return ret;
>  }

As said - please can we get away without changing this function?

Jan


More information about the Binutils mailing list