[PATCH v12 03/25] gas: implement parsing of object attributes v2

[Jan Beulich]

On 16.01.2026 19:59, Matthieu Longo wrote:
> --- a/gas/config/obj-elf-attr.c
> +++ b/gas/config/obj-elf-attr.c
> @@ -22,8 +22,63 @@
>  
>  #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;
> +
> +
> +typedef struct {

Nit: No double blank lines please.

> +/* Resolve the identifier if it matches the given symbol.  */
> +static bool
> +resolve_if_matching (const char *identifier,
> +		     const oav2_identifier_t *known_identifier,
> +		     arg_t *val_out)

As to the comment: What does "given symbol" refer to? I'm inclined to guess
it's "known_identifier", but then the comment may better say "... matches
the given one"?

> +#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,

Nit: One too many padding blanks.

> +/* In the context of object attributes, an identifier is defined with the
> +   following lexical constraint: [a-zA-z_][a-zA-Z0-9_].  An identifier can

Maybe better add a * or + at the end of the pattern, to make clear identifiers
can consist of other than exactly two characters?

> +/* 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 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.  */
> +  if (is_identifier_beginner (*input_line_pointer))
> +    {
> +      bool status = extract_identifier (is_part_of_identifier, arg_out);
> +      /* match_identifier() confirmed that it was the beginning of an
> +	 identifier, so we don't expect the extraction to fail.  */

What is match_identifier()? Is this stale and means to say is_identifier_beginner()?

> +      gas_assert (status);
> +      gas_assert (arg_out->vtype == VALUE_STRING);
> +
> +      if (! (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;

With this, ...

> +      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;

... why this?

> +      arg_out->vtype = VALUE_UNDEFINED;
> +      resolved = false;
> +
> + free_identifier:
> +      free ((char *) identifier);
> +      return resolved;
> +    }
> +
> +  /* If it is neither a string nor an identifier, it must be an expression.  */
> +  bool signedness_issue = false;
> +  bool success = extract_integer_literal (arg_out,
> +					  (low_ttype & UNSIGNED_INTEGER),
> +					  &signedness_issue);
> +  if (success && (low_ttype & (UNSIGNED_INTEGER | SIGNED_INTEGER)))
> +    return true;
> +
> +  char sbuffer[100];
> +  if (success)
> +    as_bad (_("unexpected integer '%lu', expected %s instead"),
> +	    arg_out->val.u64,

As indicated before, %lu and alike aren't suitable to print uint64_t values.

> +/* Can parse a list of arguments with variable length.  */
> +static bool
> +obj_attr_parse_args (arg_token_t expected_ttype,
> +		     bool resolve_identifier,
> +		     arg_t *arg_out)
> +{
> +  if ((expected_ttype & LIST) == 0)
> +    return obj_attr_parse_arg (expected_ttype, resolve_identifier, false,
> +			       arg_out);
> +
> +  static const size_t LIST_MAX_SIZE = 2;
> +  arg_t *arg_list = xcalloc (LIST_MAX_SIZE, sizeof (*arg_list));
> +
> +  /* We don't want to support recursive lists.  */
> +  expected_ttype &= ~LIST;
> +
> +  size_t n = 0;
> +  do {
> +    if (! trim_whitespace_before_param ())
> +      goto bad;
> +
> +    if (! obj_attr_parse_arg (expected_ttype, resolve_identifier, false,
> +			      &arg_list[n]))
> +      goto bad;
> +
> +    ++n;
> +    skip_whitespace (input_line_pointer);
> +    if (is_end_of_stmt (*input_line_pointer))
> +      break;
> +
> +    if (! skip_whitespace_past_comma ())
> +      goto bad;
> +
> +    if (n >= LIST_MAX_SIZE)
> +      {
> +	as_bad ("too many arguments for a list (max: %lu)", LIST_MAX_SIZE);

%zu as LIST_MAX_SIZE is size_t.

> +/* Parse the arguments of [vendor]_attribute directive.  */
> +static arg_t *
> +vendor_attribute_parse_args (obj_attr_vendor_t vendor ATTRIBUTE_UNUSED,
> +			     const obj_attr_subsection_v2_t *subsec ATTRIBUTE_UNUSED,
> +			     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_whitespace_before_param ())
> +	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 == ATTRIBUTE_KEY
> +	       || high_ttype == ATTRIBUTE_VALUE);
> +
> +      if (high_ttype == ATTRIBUTE_VALUE)
> +	{
> +	  arg_token_t type_attr_value
> +#if (TC_OBJ_ATTR_v1 && TC_OBJ_ATTR_v2)
> +	    = (subsec != NULL)
> +	      ? obj_attr_v2_get_arg_type (subsec->encoding)
> +	      : obj_attr_v1_get_arg_type (stdoutput, vendor,
> +					  args_out[n-1].val.u64);

Nit: [n - 1] please.

> +#elif (TC_OBJ_ATTR_v1)
> +	    = obj_attr_v1_get_arg_type (stdoutput, vendor,
> +					args_out[n-1].val.u64);

Again.

As I'd like to save another round trip: Okay with all remarks suitably
addressed (either verbally or by making respective adjustments). And again
at least an Arm64 will be needed here as well (if you haven't got one
already).

Jan