Re: [PATCH] Pass HWCAP to ifunc resolver

Andrew Pinski
2016-09-01T05:03:20Z

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
>