Re: [PATCH] Pass HWCAP to ifunc resolver

[Andrew Pinski <pinskia at gmail dot com>]

On Wed, Aug 31, 2016 at 4:49 AM, Andreas Arnez <arnez@linux.vnet.ibm.com> wrote:
> On various GNU Elf architectures, including AArch64, ARM, s390/s390x,
> ppc32/64, and sparc32/64, the dynamic loader passes HWCAP as a parameter
> to each ifunc resolver.  Currently there is an open glibc Bugzilla that
> requests this to be generalized to all architectures:
>
>   https://sourceware.org/bugzilla/show_bug.cgi?id=19766
>
> And various ifunc resolvers already rely on receiving HWCAP.  Currently
> GDB always calls an ifunc resolver without any arguments; thus the
> resolver may receive garbage, and based on that, the resolver may decide
> to return a function that is not suited for the given platform.
>
> This patch always passes HWCAP to ifunc resolvers, even on systems where
> the dynamic loader currently behaves otherwise.  The rationale is
> that (1) the dynamic loader may get adjusted on those systems as well in
> the future; (2) passing an unused argument should not cause a problem
> with existing resolvers; and (3) the logic is much simpler without such
> a distinction.

Doesn't some targets pass HWCAP2 too?

Thanks,
Andrew


>
> gdb/ChangeLog:
>
>         * elfread.c (auxv.h): New include.
>         (elf_gnu_ifunc_resolve_addr): Pass HWCAP to ifunc resolver.
>
> gdb/testsuite/ChangeLog:
>
>         * gdb.base/gnu-ifunc-lib.c (resolver_hwcap): New external
>         variable declaration.
>         (gnu_ifunc): Add parameter hwcap.  Store it in resolver_hwcap.
>         * gdb.base/gnu-ifunc.c (resolver_hwcap): New global variable.
>         * gdb.base/gnu-ifunc.exp: Add test to verify that the resolver
>         received HWCAP as its argument.
> ---
>  gdb/elfread.c                          | 13 ++++++++++---
>  gdb/testsuite/gdb.base/gnu-ifunc-lib.c |  4 +++-
>  gdb/testsuite/gdb.base/gnu-ifunc.c     |  4 ++++
>  gdb/testsuite/gdb.base/gnu-ifunc.exp   | 15 +++++++++++++++
>  4 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index e90466b..84355cf 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -46,6 +46,7 @@
>  #include "gdb_bfd.h"
>  #include "build-id.h"
>  #include "location.h"
> +#include "auxv.h"
>
>  extern void _initialize_elfread (void);
>
> @@ -860,6 +861,8 @@ elf_gnu_ifunc_resolve_addr (struct gdbarch *gdbarch, CORE_ADDR pc)
>    CORE_ADDR start_at_pc, address;
>    struct type *func_func_type = builtin_type (gdbarch)->builtin_func_func;
>    struct value *function, *address_val;
> +  CORE_ADDR hwcap = 0;
> +  struct value *hwcap_val;
>
>    /* Try first any non-intrusive methods without an inferior call.  */
>
> @@ -875,10 +878,14 @@ elf_gnu_ifunc_resolve_addr (struct gdbarch *gdbarch, CORE_ADDR pc)
>    function = allocate_value (func_func_type);
>    set_value_address (function, pc);
>
> -  /* STT_GNU_IFUNC resolver functions have no parameters.  FUNCTION is the
> -     function entry address.  ADDRESS may be a function descriptor.  */
> +  /* STT_GNU_IFUNC resolver functions usually receive the HWCAP vector as
> +     parameter.  FUNCTION is the function entry address.  ADDRESS may be a
> +     function descriptor.  */
>
> -  address_val = call_function_by_hand (function, 0, NULL);
> +  target_auxv_search (&current_target, AT_HWCAP, &hwcap);
> +  hwcap_val = value_from_longest (builtin_type (gdbarch)
> +                                 ->builtin_unsigned_long, hwcap);
> +  address_val = call_function_by_hand (function, 1, &hwcap_val);
>    address = value_as_address (address_val);
>    address = gdbarch_convert_from_func_ptr_addr (gdbarch, address,
>                                                 &current_target);
> diff --git a/gdb/testsuite/gdb.base/gnu-ifunc-lib.c b/gdb/testsuite/gdb.base/gnu-ifunc-lib.c
> index 8a55f60..0c0aeef 100644
> --- a/gdb/testsuite/gdb.base/gnu-ifunc-lib.c
> +++ b/gdb/testsuite/gdb.base/gnu-ifunc-lib.c
> @@ -16,6 +16,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>
>  extern volatile int gnu_ifunc_initialized;
> +extern volatile unsigned long resolver_hwcap;
>  extern int init_stub (int arg);
>  extern int final (int arg);
>
> @@ -24,8 +25,9 @@ typedef int (*final_t) (int arg);
>  asm (".type gnu_ifunc, %gnu_indirect_function");
>
>  final_t
> -gnu_ifunc (void)
> +gnu_ifunc (unsigned long hwcap)
>  {
> +  resolver_hwcap = hwcap;
>    if (! gnu_ifunc_initialized)
>      return init_stub;
>    else
> diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.c b/gdb/testsuite/gdb.base/gnu-ifunc.c
> index c68866e..77dea30 100644
> --- a/gdb/testsuite/gdb.base/gnu-ifunc.c
> +++ b/gdb/testsuite/gdb.base/gnu-ifunc.c
> @@ -35,6 +35,10 @@ final (int arg)
>
>  volatile int gnu_ifunc_initialized;
>
> +/* This stores the argument received by the ifunc resolver.  */
> +
> +volatile unsigned long resolver_hwcap = -1;
> +
>  static void
>  gnu_ifunc_pre (void)
>  {
> diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp
> index 097e48a9..3b2775b 100644
> --- a/gdb/testsuite/gdb.base/gnu-ifunc.exp
> +++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp
> @@ -76,6 +76,21 @@ gdb_continue_to_breakpoint "break-at-call" ".*break-at-call.*"
>
>  gdb_test "p gnu_ifunc (3)" " = 4"
>
> +# Test that the resolver received its argument.
> +
> +set actual_hwcap "0x0"
> +set test "info auxv"
> +gdb_test_multiple $test $test {
> +    -re "\r\n\\d+\\s+AT_HWCAP\[^\r\n\]+($hex)\r\n.*$gdb_prompt $" {
> +       set actual_hwcap $expect_out(1,string)
> +    }
> +    -re ".*$gdb_prompt $" {
> +       pass "$test (no HWCAP)"
> +    }
> +}
> +
> +gdb_test "p/x resolver_hwcap" "= $actual_hwcap" "resolver received HWCAP"
> +
>  # Test GDB will skip the gnu_ifunc resolver on first call.
>
>  gdb_test "step" "\r\nfinal .*"
> --
> 2.3.0
>