On Mon, Jul 14, 2025 at 5:32 AM Yury Khrustalev <yury.khrusta...@arm.com> wrote:
>
> This optional header is used to bring in the definition of the
> struct __ifunc_arg_t type. Since it has been added to glibc only
> recently, the previous implementation had to check whether this
> header is present and, if not, it provide its own definition.
>
> This creates dead code because either one of these two parts would
> not be tested. The ABI specification for ifunc resolvers allows to
> create own ABI-compatible definition for this type, which is the
> right way of doing it.
>
> In addition to improving consistency, the new approach also helps
> with addition of new fields to struct __ifunc_arg_t type without
> the need to work-around situations when the definition imported
> from the header lacks these new fields.
>
> ABI allows to define as many hwcap fields in this struct as needed,
> provided that at runtime we only access the fields that are permitted
> by the _size value.
>
> gcc/
> * config/aarch64/aarch64.cc (build_ifunc_arg_type):
> Add new fields _hwcap3 and _hwcap4.
>
> libatomic/
> * config/linux/aarch64/host-config.h (__ifunc_arg_t):
> Remove sys/ifunc.h and add new fields _hwcap3 and _hwcap4.
>
> libgcc/
> * config/aarch64/cpuinfo.c (__ifunc_arg_t): Likewise.
> (__init_cpu_features): obtain and assign values for the
> fields _hwcap3 and _hwcap4.
> (__init_cpu_features_constructor): check _size in the
> arg argument.
>
> ---
>
> Regression has been checked on AArch64 and no regression has been
> found. OK for trunk?
I think this is ok. I assume you tested it with both a newish and older glibc.
Thanks,
Andrew
>
> base commit: 3a1067c8b8c
>
> ---
> gcc/config/aarch64/aarch64.cc | 12 +++++
> libatomic/config/linux/aarch64/host-config.h | 12 +++--
> libgcc/config/aarch64/cpuinfo.c | 46 ++++++++++++++++----
> 3 files changed, 57 insertions(+), 13 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 6e16763f957..26c0f70f952 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -20466,6 +20466,8 @@ aarch64_compare_version_priority (tree decl1, tree
> decl2)
> unsigned long _size; // Size of the struct, so it can grow.
> unsigned long _hwcap;
> unsigned long _hwcap2;
> + unsigned long _hwcap3;
> + unsigned long _hwcap4;
> }
> */
>
> @@ -20482,14 +20484,24 @@ build_ifunc_arg_type ()
> tree field3 = build_decl (UNKNOWN_LOCATION, FIELD_DECL,
> get_identifier ("_hwcap2"),
> long_unsigned_type_node);
> + tree field4 = build_decl (UNKNOWN_LOCATION, FIELD_DECL,
> + get_identifier ("_hwcap3"),
> + long_unsigned_type_node);
> + tree field5 = build_decl (UNKNOWN_LOCATION, FIELD_DECL,
> + get_identifier ("_hwcap4"),
> + long_unsigned_type_node);
>
> DECL_FIELD_CONTEXT (field1) = ifunc_arg_type;
> DECL_FIELD_CONTEXT (field2) = ifunc_arg_type;
> DECL_FIELD_CONTEXT (field3) = ifunc_arg_type;
> + DECL_FIELD_CONTEXT (field4) = ifunc_arg_type;
> + DECL_FIELD_CONTEXT (field5) = ifunc_arg_type;
>
> TYPE_FIELDS (ifunc_arg_type) = field1;
> DECL_CHAIN (field1) = field2;
> DECL_CHAIN (field2) = field3;
> + DECL_CHAIN (field3) = field4;
> + DECL_CHAIN (field4) = field5;
>
> layout_type (ifunc_arg_type);
>
> diff --git a/libatomic/config/linux/aarch64/host-config.h
> b/libatomic/config/linux/aarch64/host-config.h
> index d0d44bf18ea..c6f8d693f2c 100644
> --- a/libatomic/config/linux/aarch64/host-config.h
> +++ b/libatomic/config/linux/aarch64/host-config.h
> @@ -40,16 +40,20 @@
> # define HWCAP2_LSE128 (1UL << 47)
> #endif
>
> -#if __has_include(<sys/ifunc.h>)
> -# include <sys/ifunc.h>
> -#else
> +/* The following struct is ABI-correct description of the 2nd argument for an
> + ifunc resolver as per SYSVABI spec (see link below). It is safe to extend
> + it with new fields. The ifunc resolver implementations must always check
> + the runtime size of the buffer using the value in the _size field.
> + https://github.com/ARM-software/abi-aa/blob/main/sysvabi64/sysvabi64.rst.
> */
> typedef struct __ifunc_arg_t {
> unsigned long _size;
> unsigned long _hwcap;
> unsigned long _hwcap2;
> + unsigned long _hwcap3;
> + unsigned long _hwcap4;
> } __ifunc_arg_t;
> +
> # define _IFUNC_ARG_HWCAP (1ULL << 62)
> -#endif
>
> /* From the file which imported `host-config.h' we can ascertain which
> architectural extension provides relevant atomic support. From this,
> diff --git a/libgcc/config/aarch64/cpuinfo.c b/libgcc/config/aarch64/cpuinfo.c
> index dda9dc69689..50f54cac26b 100644
> --- a/libgcc/config/aarch64/cpuinfo.c
> +++ b/libgcc/config/aarch64/cpuinfo.c
> @@ -27,15 +27,18 @@
> #if __has_include(<sys/auxv.h>)
> #include <sys/auxv.h>
>
> -#if __has_include(<sys/ifunc.h>)
> -#include <sys/ifunc.h>
> -#else
> +/* The following struct is ABI-correct description of the 2nd argument for an
> + ifunc resolver as per SYSVABI spec (see link below). It is safe to extend
> + it with new fields. The ifunc resolver implementations must always check
> + the runtime size of the buffer using the value in the _size field.
> + https://github.com/ARM-software/abi-aa/blob/main/sysvabi64/sysvabi64.rst.
> */
> typedef struct __ifunc_arg_t {
> unsigned long _size;
> unsigned long _hwcap;
> unsigned long _hwcap2;
> + unsigned long _hwcap3;
> + unsigned long _hwcap4;
> } __ifunc_arg_t;
> -#endif
>
> #if __has_include(<asm/hwcap.h>)
> #include <asm/hwcap.h>
> @@ -237,6 +240,18 @@ struct {
> #define HWCAP2_LRCPC3 (1UL << 46)
> #endif
>
> +#ifndef AT_HWCAP3
> +#define AT_HWCAP3 29
> +#endif
> +
> +#ifndef AT_HWCAP4
> +#define AT_HWCAP4 30
> +#endif
> +
> +#define __IFUNC_ARG_SIZE_HWCAP2 (sizeof (unsigned long) * 3)
> +#define __IFUNC_ARG_SIZE_HWCAP3 (sizeof (unsigned long) * 4)
> +#define __IFUNC_ARG_SIZE_HWCAP4 (sizeof (unsigned long) * 5)
> +
> static void
> __init_cpu_features_constructor (unsigned long hwcap,
> const __ifunc_arg_t *arg)
> @@ -247,8 +262,14 @@ __init_cpu_features_constructor (unsigned long hwcap,
> #define extractBits(val, start, number) \
> (val & ((1UL << number) - 1UL) << start) >> start
> unsigned long hwcap2 = 0;
> - if (hwcap & _IFUNC_ARG_HWCAP)
> + if ((hwcap & _IFUNC_ARG_HWCAP) && arg->_size >= __IFUNC_ARG_SIZE_HWCAP2)
> hwcap2 = arg->_hwcap2;
> + unsigned long hwcap3 __attribute__ ((unused)) = 0;
> + if ((hwcap & _IFUNC_ARG_HWCAP) && arg->_size >= __IFUNC_ARG_SIZE_HWCAP3)
> + hwcap3 = arg->_hwcap3;
> + unsigned long hwcap4 __attribute__ ((unused)) = 0;
> + if ((hwcap & _IFUNC_ARG_HWCAP) && arg->_size >= __IFUNC_ARG_SIZE_HWCAP4)
> + hwcap4 = arg->_hwcap4;
> if (hwcap & HWCAP_CRC32)
> setCPUFeature(FEAT_CRC);
> if (hwcap & HWCAP_PMULL)
> @@ -383,17 +404,24 @@ __init_cpu_features(void)
> {
> unsigned long hwcap;
> unsigned long hwcap2;
> + unsigned long hwcap3;
> + unsigned long hwcap4;
>
> /* CPU features already initialized. */
> if (__atomic_load_n (&__aarch64_cpu_features.features, __ATOMIC_RELAXED))
> return;
> - hwcap = getauxval(AT_HWCAP);
> - hwcap2 = getauxval(AT_HWCAP2);
> + hwcap = getauxval (AT_HWCAP);
> + hwcap2 = getauxval (AT_HWCAP2);
> + hwcap3 = getauxval (AT_HWCAP3);
> + hwcap4 = getauxval (AT_HWCAP4);
> +
> __ifunc_arg_t arg;
> - arg._size = sizeof(__ifunc_arg_t);
> + arg._size = sizeof (__ifunc_arg_t);
> arg._hwcap = hwcap;
> arg._hwcap2 = hwcap2;
> - __init_cpu_features_constructor(hwcap | _IFUNC_ARG_HWCAP, &arg);
> + arg._hwcap3 = hwcap3;
> + arg._hwcap4 = hwcap4;
> + __init_cpu_features_constructor (hwcap | _IFUNC_ARG_HWCAP, &arg);
> #undef extractBits
> #undef getCPUFeature
> #undef setCPUFeature
> --
> 2.39.5
>
