[PATCH] Pass HWCAP to ifunc resolver

[PATCH] Pass HWCAP to ifunc resolver

[Andreas Arnez]

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.

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

Re: [PATCH] Pass HWCAP to ifunc resolver

[Andrew Pinski]

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
>

Re: [PATCH] Pass HWCAP to ifunc resolver

[Andreas Arnez]

On Thu, Sep 01 2016, Andrew Pinski wrote:

> 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?

If I understand the glibc code correctly, no.  The loader maintains the
variables dl_hwcap and dl_hwcap2 and fills them from AT_HWCAP and
AT_HWCAP2, respectively, but the ifunc resolvers only receive dl_hwcap:

  ./sysdeps/aarch64/dl-irel.h:33:  return ((ElfW(Addr) (*) (unsigned long int)) (addr)) (GLRO(dl_hwcap));
  ./sysdeps/arm/dl-irel.h:33:  return ((Elf32_Addr (*) (unsigned long int)) (addr)) (GLRO(dl_hwcap));
  ./sysdeps/generic/dl-irel.h:26:  return ((DL_FIXUP_VALUE_TYPE (*) (void)) (addr)) ();
  ./sysdeps/hppa/dl-irel.h:36:  return ((struct fdesc) {0, 0});
  ./sysdeps/i386/dl-irel.h:32:  return ((Elf32_Addr (*) (void)) (addr)) ();
  ./sysdeps/powerpc/powerpc32/dl-irel.h:33:  return ((Elf32_Addr (*) (unsigned long int)) (addr)) (GLRO(dl_hwcap));
  ./sysdeps/powerpc/powerpc64/dl-irel.h:34:  return ((Elf64_Addr (*) (unsigned long int)) (addr)) (GLRO(dl_hwcap));
  ./sysdeps/s390/dl-irel.h:33:  return ((ElfW(Addr) (*) (unsigned long int)) (addr)) (GLRO(dl_hwcap));
  ./sysdeps/sparc/sparc32/dl-irel.h:34:  return ((Elf32_Addr (*) (int)) (addr)) (GLRO(dl_hwcap));
  ./sysdeps/sparc/sparc64/dl-irel.h:34:  return ((Elf64_Addr (*) (int)) (addr)) (GLRO(dl_hwcap));
  ./sysdeps/x86_64/dl-irel.h:32:  return ((ElfW(Addr) (*) (void)) (addr)) ();

--
Andreas

Re: [PATCH] Pass HWCAP to ifunc resolver

[Ulrich Weigand]

Andreas Arnez 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.

This makes sense to me.  If necessary, we can always still add platform-
specific call sequences later.

> 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.

This is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Pass HWCAP to ifunc resolver

[Andreas Arnez]

On Tue, Sep 06 2016, Ulrich Weigand wrote:

> Andreas Arnez wrote:

[...]

>
>> 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.
>
> This is OK.

Thanks, pushed.

--
Andreas