[v7,4/6] riscv: Enable multi-arg ifunc resolvers

RISC-V is apparently the first architecture to pass more than one
argument to ifunc resolvers. The helper macros in libc-symbols.h,
__ifunc_resolver(), __ifunc(), and __ifunc_hidden(), are incompatible
with this. These macros have an "arg" (non-final) parameter that
represents the parameter signature of the ifunc resolver. The result is
an inability to pass the required comma through in a single preprocessor
argument.

Rearrange the __ifunc_resolver() macro to be variadic, and pass the
types as those variable parameters. Move the guts of __ifunc() and
__ifunc_hidden() into new macros, __ifunc_args(), and
__ifunc_args_hidden(), that pass the variable arguments down through to
__ifunc_resolver(). Then redefine __ifunc() and __ifunc_hidden(), which
are used in a bunch of places, to simply shuffle the arguments down into
__ifunc_args[_hidden]. Finally, define a riscv-ifunc.h header, which
provides convenience macros to those looking to write ifunc selectors
that use both arguments.

Signed-off-by: Evan Green <evan@rivosinc.com>

---

(no changes since v6)

Changes in v6:
 - Introduced riscv-ifunc.h for multi-arg ifunc selectors.

Note: I opted to create another layer of macros (__ifunc_args()) rather
than doing the treewide change to rearrange the signature of __ifunc()
and __ifunc_hidden(). If folks like the overall approach but would
prefer the treewide change, I can do that too.

---
 include/libc-symbols.h      | 28 +++++++++++++++++-----------
 sysdeps/riscv/riscv-ifunc.h | 27 +++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 11 deletions(-)
 create mode 100644 sysdeps/riscv/riscv-ifunc.h

On Wed, Aug 23, 2023, 15:32 Evan Green <evan@rivosinc.com> wrote:

> RISC-V is apparently the first architecture to pass more than one
> argument to ifunc resolvers.


how does arm64 work? that passes hwcap and a pointer to a struct containing
hwcap2, so presumably they had to solve this already?

The helper macros in libc-symbols.h,
> __ifunc_resolver(), __ifunc(), and __ifunc_hidden(), are incompatible
> with this. These macros have an "arg" (non-final) parameter that
> represents the parameter signature of the ifunc resolver. The result is
> an inability to pass the required comma through in a single preprocessor
> argument.
>
> Rearrange the __ifunc_resolver() macro to be variadic, and pass the
> types as those variable parameters. Move the guts of __ifunc() and
> __ifunc_hidden() into new macros, __ifunc_args(), and
> __ifunc_args_hidden(), that pass the variable arguments down through to
> __ifunc_resolver(). Then redefine __ifunc() and __ifunc_hidden(), which
> are used in a bunch of places, to simply shuffle the arguments down into
> __ifunc_args[_hidden]. Finally, define a riscv-ifunc.h header, which
> provides convenience macros to those looking to write ifunc selectors
> that use both arguments.
>
> Signed-off-by: Evan Green <evan@rivosinc.com>
>
> ---
>
> (no changes since v6)
>
> Changes in v6:
>  - Introduced riscv-ifunc.h for multi-arg ifunc selectors.
>
> Note: I opted to create another layer of macros (__ifunc_args()) rather
> than doing the treewide change to rearrange the signature of __ifunc()
> and __ifunc_hidden(). If folks like the overall approach but would
> prefer the treewide change, I can do that too.
>
> ---
>  include/libc-symbols.h      | 28 +++++++++++++++++-----------
>  sysdeps/riscv/riscv-ifunc.h | 27 +++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+), 11 deletions(-)
>  create mode 100644 sysdeps/riscv/riscv-ifunc.h
>
> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> index 5794614488..36b92039c5 100644
> --- a/include/libc-symbols.h
> +++ b/include/libc-symbols.h
> @@ -665,9 +665,9 @@ for linking")
>  #endif
>
>  /* Helper / base  macros for indirect function symbols.  */
> -#define __ifunc_resolver(type_name, name, expr, arg, init, classifier) \
> +#define __ifunc_resolver(type_name, name, expr, init, classifier, ...) \
>    classifier inhibit_stack_protector                                   \
> -  __typeof (type_name) *name##_ifunc (arg)                             \
> +  __typeof (type_name) *name##_ifunc (__VA_ARGS__)                     \
>    {                                                                    \
>      init ();                                                           \
>      __typeof (type_name) *res = expr;                                  \
> @@ -675,13 +675,13 @@ for linking")
>    }
>
>  #ifdef HAVE_GCC_IFUNC
> -# define __ifunc(type_name, name, expr, arg, init)                     \
> +# define __ifunc_args(type_name, name, expr, init, ...)
>       \
>    extern __typeof (type_name) name __attribute__                       \
>                               ((ifunc (#name "_ifunc")));               \
> -  __ifunc_resolver (type_name, name, expr, arg, init, static)
> +  __ifunc_resolver (type_name, name, expr, init, static, __VA_ARGS__)
>
> -# define __ifunc_hidden(type_name, name, expr, arg, init)      \
> -  __ifunc (type_name, name, expr, arg, init)
> +# define __ifunc_args_hidden(type_name, name, expr, init, ...)         \
> +  __ifunc (type_name, name, expr, init, __VA_ARGS__)
>  #else
>  /* Gcc does not support __attribute__ ((ifunc (...))).  Use the old
> behaviour
>     as fallback.  But keep in mind that the debug information for the ifunc
> @@ -692,18 +692,24 @@ for linking")
>     different signatures.  (Gcc support is disabled at least on a ppc64le
>     Ubuntu 14.04 system.)  */
>
> -# define __ifunc(type_name, name, expr, arg, init)                     \
> +# define __ifunc_args(type_name, name, expr, init, ...)
>       \
>    extern __typeof (type_name) name;                                    \
> -  __typeof (type_name) *name##_ifunc (arg) __asm__ (#name);            \
> -  __ifunc_resolver (type_name, name, expr, arg, init,)                 \
> +  __typeof (type_name) *name##_ifunc (__VA_ARGS__) __asm__ (#name);    \
> +  __ifunc_resolver (type_name, name, expr, init, , __VA_ARGS__)
>       \
>   __asm__ (".type " #name ", %gnu_indirect_function");
>
> -# define __ifunc_hidden(type_name, name, expr, arg, init)              \
> +# define __ifunc_args_hidden(type_name, name, expr, init, ...)         \
>    extern __typeof (type_name) __libc_##name;                           \
> -  __ifunc (type_name, __libc_##name, expr, arg, init)                  \
> +  __ifunc (type_name, __libc_##name, expr, __VA_INIT__, init)          \
>    strong_alias (__libc_##name, name);
>  #endif /* !HAVE_GCC_IFUNC  */
>
> +#define __ifunc(type_name, name, expr, arg, init)                      \
> +  __ifunc_args (type_name, name, expr, init, arg)
> +
> +#define __ifunc_hidden(type_name, name, expr, arg, init)               \
> +  __ifunc_args_hidden (type_name, expr, init, arg)
> +
>  /* The following macros are used for indirect function symbols in libc.so.
>     First of all, you need to have the function prototyped somewhere,
>     say in foo.h:
> diff --git a/sysdeps/riscv/riscv-ifunc.h b/sysdeps/riscv/riscv-ifunc.h
> new file mode 100644
> index 0000000000..7bff591d1e
> --- /dev/null
> +++ b/sysdeps/riscv/riscv-ifunc.h
> @@ -0,0 +1,27 @@
> +/* Common definition for ifunc resolvers.  Linux/RISC-V version.
> +   This file is part of the GNU C Library.
> +   Copyright (C) 2023 Free Software Foundation, Inc.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library 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
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <sysdep.h>
> +#include <ifunc-init.h>
> +#include <sys/hwprobe.h>
> +
> +#define INIT_ARCH()
> +
> +#define riscv_libc_ifunc(name, expr)                           \
> +  __ifunc_args (name, name, expr(hwcap, hwprobe), INIT_ARCH,   \
> +                uint64_t hwcap, __riscv_hwprobe_t hwprobe)
> --
> 2.34.1
>
>

On Wed, Aug 23, 2023 at 4:20 PM enh <enh@google.com> wrote:
>
>
>
> On Wed, Aug 23, 2023, 15:32 Evan Green <evan@rivosinc.com> wrote:
>>
>> RISC-V is apparently the first architecture to pass more than one
>> argument to ifunc resolvers.
>
>
> how does arm64 work? that passes hwcap and a pointer to a struct containing hwcap2, so presumably they had to solve this already?

This macro is a glibc internal thing, so I think they only had to
solve it if they needed to use that second arg in any of the ifunc
selectors written within glibc. When I grep around for libc_ifunc in
aarch64 files, they seem to use none of the parameters passed (and can
pretend the signature is void). Instead they peer at midr or other
globals set up in INIT_ARCH() to do vendor-y things (eg return
__memcpy_thunderx if the vendor/partnum match).

I suppose I could cheat too (Florian mentioned this in the last spin),
skip this patch, and just reach for the internal __riscv_hwprobe
symbol directly in the memcpy ifunc. I kind of liked that this memcpy
change served as a nice example, but I suppose there's an argument to
be made for aiming for brevity.

-Evan