[PATCH v4 17/22] gnu directives: add support for gnu_attribute and gnu_subsection in OAv2 context

[Jan Beulich]

On 03.07.2025 18:27, Matthieu Longo wrote:
> --- a/gas/attr.h
> +++ b/gas/attr.h
> @@ -24,6 +24,30 @@
>  #include "as.h"
>  #include "bfd/elf-bfd.h"
>  
> +/* The target supports Object Attributes v1.  */
> +#if (defined(TC_ARC) \
> + || defined(TC_ARM) \
> + || defined(TC_CSKY) \
> + || defined(TC_M68K) \
> + || defined(TC_MIPS) \
> + || defined(TC_MSP430) \
> + || defined(TC_PPC) \
> + || defined(TC_RISCV) \
> + || defined(TC_S390) \
> + || defined(TC_SPARC) \
> + || defined(TC_TIC6X))
> +#define HAVE_OBJ_ATTR_v1 1
> +#else
> +#define HAVE_OBJ_ATTR_v1 0
> +#endif
> +
> +/* The target supports Object Attributes v2.  */
> +#if defined(TC_AARCH64)
> +#define HAVE_OBJ_ATTR_v2 1
> +#else
> +#define HAVE_OBJ_ATTR_v2 0
> +#endif

Why is this done here, rather than in each target's tc-<arch>.h? The new identifiers
likely also better would have TC_ prefixes, and probably would want documenting in
internals.texi.

Further, why (having looked at the later patch) overall three #define-s when one
(TC_OBJ_ATTR) could do (expanding to 0 or the desired version)? Unless of course
you expect targets might want support for multiple versions.

Finally in case the above wants / needs to stay here, a style nit: In GNU style it's
"defined (...)", i.e. with a blank.

> @@ -2045,12 +2055,30 @@ obj_elf_vtable_entry (int ignore ATTRIBUTE_UNUSED)
>  }
>  
>  /* Parse a .gnu_attribute directive.  */
> -
> +#if (HAVE_OBJ_ATTR_v1 || HAVE_OBJ_ATTR_v2)
>  static void

Please retain the blank line and instead move the #if ahead of the comment.

>  obj_elf_gnu_attribute (int ignored ATTRIBUTE_UNUSED)
>  {
>    obj_attr_process_attribute (OBJ_ATTR_GNU);
>  }
> +#endif
> +
> +/* Parse a .gnu_subsection directive.  */
> +#if (HAVE_OBJ_ATTR_v2)
> +static void
> +obj_elf_gnu_subsection (int ignored ATTRIBUTE_UNUSED)

Style-wise this wants arranging similarly then.

Jan