[2/2] gdbserver: Add linux_get_hwcap

Message ID 20190325120542.92123-2-alan.hayward@arm.com
State New
Headers show
Series
  • [1/2] Add linux_get_hwcap
Related show

Commit Message

Alan Hayward March 25, 2019, 12:05 p.m.
In gdbserver, Tidy up calls to read HWCAP (and HWCAP2) by adding common
functions, removing the Arm, AArch64, PPC and S390 specific versions.

No functionality differences.

[ I wasn't sure in gdbserver when to use CORE_ADDR and when to use int/long.
  I'm assuming CORE_ADDR is fairly recent to gdbserver? ]

Tested using AArch64, making sure PAUTH is detected correctly.

gdb/gdbserver/ChangeLog:

2019-03-25  Alan Hayward  <alan.hayward@arm.com>

	* linux-aarch64-low.c (aarch64_get_hwcap): Remove function.
	(aarch64_arch_setup): Call linux_get_hwcap.
	* linux-arm-low.c (arm_get_hwcap): Remove function.
	(arm_read_description): Call linux_get_hwcap.
	* linux-low.c (linux_get_auxv): New function.
	(linux_get_hwcap): Likewise.
	(linux_get_hwcap2): Likewise.
	* linux-low.h (linux_get_hwcap): New declaration.
	(linux_get_hwcap2): Likewise.
	* linux-ppc-low.c (ppc_get_auxv): Remove function.
	(ppc_arch_setup): Call linux_get_hwcap.
	* linux-s390-low.c (s390_get_hwcap): Remove function.
	(s390_arch_setup): Call linux_get_hwcap.
---
 gdb/gdbserver/linux-aarch64-low.c | 28 ++-------------
 gdb/gdbserver/linux-arm-low.c     | 27 +-------------
 gdb/gdbserver/linux-low.c         | 58 +++++++++++++++++++++++++++++++
 gdb/gdbserver/linux-low.h         |  8 +++++
 gdb/gdbserver/linux-ppc-low.c     | 41 ++--------------------
 gdb/gdbserver/linux-s390-low.c    | 32 +----------------
 6 files changed, 72 insertions(+), 122 deletions(-)

-- 
2.17.2 (Apple Git-113)

Comments

Simon Marchi March 25, 2019, 3:41 p.m. | #1
On 2019-03-25 8:05 a.m., Alan Hayward wrote:
> In gdbserver, Tidy up calls to read HWCAP (and HWCAP2) by adding common

> functions, removing the Arm, AArch64, PPC and S390 specific versions.

> 

> No functionality differences.

> 

> [ I wasn't sure in gdbserver when to use CORE_ADDR and when to use int/long.

>    I'm assuming CORE_ADDR is fairly recent to gdbserver? ]


I don't know if CORE_ADDR is a recent addition to gdbserver.  But I 
suppose CORE_ADDR was chosen as the return type for functions reading 
arbitrary AUXV values, since some of them may be pointers.  With 
CORE_ADDR, we know those values will fit in the data type.  When we 
return the HWCAP value, we know it won't be a pointer though, so 
returning a CORE_ADDR is a bit confusing, IMO.  Those functions 
returning the HWCAP value could return something else, an uint64_t 
maybe.  But then I would change it in the gdb version as well to match.

>   /* Implementation of linux_target_ops method "arch_setup".  */

>   

>   static void

> @@ -545,8 +521,8 @@ aarch64_arch_setup (void)

>     if (is_elf64)

>       {

>         uint64_t vq = aarch64_sve_get_vq (tid);

> -      unsigned long hwcap = 0;

> -      bool pauth_p = aarch64_get_hwcap (&hwcap) && (hwcap & AARCH64_HWCAP_PACA);

> +      unsigned long hwcap = linux_get_hwcap (8);

> +      bool pauth_p = hwcap & AARCH64_HWCAP_PACA;


Just wondering, can the linux-aarch64-low.c code be used to debug a process

> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c

> index 6f703f589f..481919c205 100644

> --- a/gdb/gdbserver/linux-low.c

> +++ b/gdb/gdbserver/linux-low.c

> @@ -7423,6 +7423,64 @@ linux_get_pc_64bit (struct regcache *regcache)

>     return pc;

>   }

>   

> +/* Extract the auxiliary vector entry with a_type matching MATCH, storing the

> +   value in VALP and returning true.  If no entry was found, return false.  */

> +

> +static bool

> +linux_get_auxv (int wordsize, CORE_ADDR match, CORE_ADDR *valp)


I think this function could return the result (CORE_ADDR) directly,
returning 0 on failure.

If 4 and 8 are the only supported wordsize values, I would suggest 
adding an assert to verify it.

> +{

> +  gdb_byte *data = (gdb_byte *) alloca (2 * wordsize);

> +  int offset = 0;

> +

> +  while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize)

> +    {

> +      if (wordsize == 4)

> +	{

> +	  unsigned int *data_p = (unsigned int *)data;

> +	  if (data_p[0] == match)

> +	    {

> +	      *valp = data_p[1];

> +	      return true;

> +	    }

> +	}

> +      else

> +	{

> +	  unsigned long *data_p = (unsigned long *)data;

> +	  if (data_p[0] == match)

> +	    {

> +	      *valp = data_p[1];

> +	      return true;

> +	    }

> +	}


I am a bit worried about relying on the size of the "int" and "long" 
types in architecture-independent code.  Could we use uint32_t and 
uint64_t instead?

Simon
Alan Hayward March 26, 2019, 1:17 p.m. | #2
> On 25 Mar 2019, at 15:41, Simon Marchi <simark@simark.ca> wrote:

> 

> On 2019-03-25 8:05 a.m., Alan Hayward wrote:

>> In gdbserver, Tidy up calls to read HWCAP (and HWCAP2) by adding common

>> functions, removing the Arm, AArch64, PPC and S390 specific versions.

>> No functionality differences.

>> [ I wasn't sure in gdbserver when to use CORE_ADDR and when to use int/long.

>>   I'm assuming CORE_ADDR is fairly recent to gdbserver? ]

> 

> I don't know if CORE_ADDR is a recent addition to gdbserver.  But I suppose CORE_ADDR was chosen as the return type for functions reading arbitrary AUXV values, since some of them may be pointers.  With CORE_ADDR, we know those values will fit in the data type.  When we return the HWCAP value, we know it won't be a pointer though, so returning a CORE_ADDR is a bit confusing, IMO.  Those functions returning the HWCAP value could return something else, an uint64_t maybe.  But then I would change it in the gdb version as well to match.


I’ve been flipping back and too in my head to change CORE_ADDR to uint64_t.

On a 32bit build, CORE_ADDR is 32bits. So it would be making a difference. Not sure if that really matters or not?

Left this change out of the update below for now.


> 

>>  /* Implementation of linux_target_ops method "arch_setup".  */

>>    static void

>> @@ -545,8 +521,8 @@ aarch64_arch_setup (void)

>>    if (is_elf64)

>>      {

>>        uint64_t vq = aarch64_sve_get_vq (tid);

>> -      unsigned long hwcap = 0;

>> -      bool pauth_p = aarch64_get_hwcap (&hwcap) && (hwcap & AARCH64_HWCAP_PACA);

>> +      unsigned long hwcap = linux_get_hwcap (8);

>> +      bool pauth_p = hwcap & AARCH64_HWCAP_PACA;

> 

> Just wondering, can the linux-aarch64-low.c code be used to debug a process


"gdbserver :2345 a.out” works on an AArch64 box if that’s what you're asking?
The code above is used to detect point auth.

> 

>> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c

>> index 6f703f589f..481919c205 100644

>> --- a/gdb/gdbserver/linux-low.c

>> +++ b/gdb/gdbserver/linux-low.c

>> @@ -7423,6 +7423,64 @@ linux_get_pc_64bit (struct regcache *regcache)

>>    return pc;

>>  }

>>  +/* Extract the auxiliary vector entry with a_type matching MATCH, storing the

>> +   value in VALP and returning true.  If no entry was found, return false.  */

>> +

>> +static bool

>> +linux_get_auxv (int wordsize, CORE_ADDR match, CORE_ADDR *valp)

> 

> I think this function could return the result (CORE_ADDR) directly,

> returning 0 on failure.


Done. Agreed that given that this function is now static, there is really no need to
keep the unused error.

With this changed, I almost changed linux_get_hwcap and linux_get_hwcap2 into inline
functions in the header. But that means adding extra includes into the header (for
the AT_HWCAP defines) and reliving the static from linux_get_auxv.

> 

> If 4 and 8 are the only supported wordsize values, I would suggest adding an assert to verify it.


Done.

> 

>> +{

>> +  gdb_byte *data = (gdb_byte *) alloca (2 * wordsize);

>> +  int offset = 0;

>> +

>> +  while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize)

>> +    {

>> +      if (wordsize == 4)

>> +	{

>> +	  unsigned int *data_p = (unsigned int *)data;

>> +	  if (data_p[0] == match)

>> +	    {

>> +	      *valp = data_p[1];

>> +	      return true;

>> +	    }

>> +	}

>> +      else

>> +	{

>> +	  unsigned long *data_p = (unsigned long *)data;

>> +	  if (data_p[0] == match)

>> +	    {

>> +	      *valp = data_p[1];

>> +	      return true;

>> +	    }

>> +	}

> 

> I am a bit worried about relying on the size of the "int" and "long" types in architecture-independent code.  Could we use uint32_t and uint64_t instead?

> 


Done.

> Simon




Updated patch below. Will push if you have no more comments.

Alan.



diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 20c75493b0..dc4ee81d2a 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -505,30 +505,6 @@ aarch64_linux_new_fork (struct process_info *parent,
 /* Matches HWCAP_PACA in kernel header arch/arm64/include/uapi/asm/hwcap.h.  */
 #define AARCH64_HWCAP_PACA (1 << 30)

-/* Fetch the AT_HWCAP entry from the auxv vector.  */
-
-static bool
-aarch64_get_hwcap (unsigned long *valp)
-{
-  unsigned char *data = (unsigned char *) alloca (16);
-  int offset = 0;
-
-  while ((*the_target->read_auxv) (offset, data, 16) == 16)
-    {
-      unsigned long *data_p = (unsigned long *)data;
-      if (data_p[0] == AT_HWCAP)
-       {
-         *valp = data_p[1];
-         return true;
-       }
-
-      offset += 16;
-    }
-
-  *valp = 0;
-  return false;
-}
-
 /* Implementation of linux_target_ops method "arch_setup".  */

 static void
@@ -545,8 +521,8 @@ aarch64_arch_setup (void)
   if (is_elf64)
     {
       uint64_t vq = aarch64_sve_get_vq (tid);
-      unsigned long hwcap = 0;
-      bool pauth_p = aarch64_get_hwcap (&hwcap) && (hwcap & AARCH64_HWCAP_PACA);
+      unsigned long hwcap = linux_get_hwcap (8);
+      bool pauth_p = hwcap & AARCH64_HWCAP_PACA;

       current_process ()->tdesc = aarch64_linux_read_description (vq, pauth_p);
     }
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 8cad5c5fd4..ff72a489cb 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -847,40 +847,15 @@ get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self)
   return next_pc;
 }

-static int
-arm_get_hwcap (unsigned long *valp)
-{
-  unsigned char *data = (unsigned char *) alloca (8);
-  int offset = 0;
-
-  while ((*the_target->read_auxv) (offset, data, 8) == 8)
-    {
-      unsigned int *data_p = (unsigned int *)data;
-      if (data_p[0] == AT_HWCAP)
-       {
-         *valp = data_p[1];
-         return 1;
-       }
-
-      offset += 8;
-    }
-
-  *valp = 0;
-  return 0;
-}
-
 static const struct target_desc *
 arm_read_description (void)
 {
   int pid = lwpid_of (current_thread);
-  unsigned long arm_hwcap = 0;
+  unsigned long arm_hwcap = linux_get_hwcap (4);

   /* Query hardware watchpoint/breakpoint capabilities.  */
   arm_linux_init_hwbp_cap (pid);

-  if (arm_get_hwcap (&arm_hwcap) == 0)
-    return tdesc_arm;
-
   if (arm_hwcap & HWCAP_IWMMXT)
     return tdesc_arm_with_iwmmxt;

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 6f703f589f..7158a6798c 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -7423,6 +7423,53 @@ linux_get_pc_64bit (struct regcache *regcache)
   return pc;
 }

+/* Fetch the entry MATCH from the auxv vector, where entries are length
+   WORDSIZE.  If no entry was found, return zero.  */
+
+static CORE_ADDR
+linux_get_auxv (int wordsize, CORE_ADDR match)
+{
+  gdb_byte *data = (gdb_byte *) alloca (2 * wordsize);
+  int offset = 0;
@@ -7423,6 +7423,53 @@ linux_get_pc_64bit (struct regcache *regcache)
   return pc;
 }

+/* Fetch the entry MATCH from the auxv vector, where entries are length
+   WORDSIZE.  If no entry was found, return zero.  */
+
+static CORE_ADDR
+linux_get_auxv (int wordsize, CORE_ADDR match)
+{
+  gdb_byte *data = (gdb_byte *) alloca (2 * wordsize);
+  int offset = 0;
+
+  gdb_assert (wordsize == 4 || wordsize == 8);
+
+  while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize)
+    {
+      if (wordsize == 4)
+       {
+         uint32_t *data_p = (uint32_t *)data;
+         if (data_p[0] == match)
+           return data_p[1];
+       }
+      else
+       {
+         uint64_t *data_p = (uint64_t *)data;
+         if (data_p[0] == match)
+           return data_p[1];
+       }
+
+      offset += 2 * wordsize;
+    }
+
+  return 0;
+}
+
+/* See linux-low.h.  */
+
+CORE_ADDR
+linux_get_hwcap (int wordsize)
+{
+  return linux_get_auxv (wordsize, AT_HWCAP);
+}
+
+/* See linux-low.h.  */
+
+CORE_ADDR
+linux_get_hwcap2 (int wordsize)
+{
+  return linux_get_auxv (wordsize, AT_HWCAP2);
+}

 static struct target_ops linux_target_ops = {
   linux_create_inferior,
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 1ade35d648..d825184835 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -435,4 +435,14 @@ bool thread_db_thread_handle (ptid_t ptid, gdb_byte **handle, int *handle_len);

 extern int have_ptrace_getregset;

+/* Fetch the AT_HWCAP entry from the auxv vector, where entries are length
+   WORDSIZE.  If no entry was found, return zero.  */
+
+CORE_ADDR linux_get_hwcap (int wordsize);
+
+/* Fetch the AT_HWCAP2 entry from the auxv vector, where entries are length
+   WORDSIZE.  If no entry was found, return zero.  */
+
+CORE_ADDR linux_get_hwcap2 (int wordsize);
+
 #endif /* GDBSERVER_LINUX_LOW_H */
diff --git a/gdb/gdbserver/linux-ppc-low.c b/gdb/gdbserver/linux-ppc-low.c
index 1b695e53fe..8deb0ce068 100644
--- a/gdb/gdbserver/linux-ppc-low.c
+++ b/gdb/gdbserver/linux-ppc-low.c
@@ -323,43 +323,6 @@ ppc_set_pc (struct regcache *regcache, CORE_ADDR pc)
     }
 }

-
-static int
-ppc_get_auxv (unsigned long type, unsigned long *valp)
-{
-  const struct target_desc *tdesc = current_process ()->tdesc;
-  int wordsize = register_size (tdesc, 0);
-  unsigned char *data = (unsigned char *) alloca (2 * wordsize);
-  int offset = 0;
-
-  while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize)
-    {
-      if (wordsize == 4)
-       {
-         unsigned int *data_p = (unsigned int *)data;
-         if (data_p[0] == type)
-           {
-             *valp = data_p[1];
-             return 1;
-           }
-       }
-      else
-       {
-         unsigned long *data_p = (unsigned long *)data;
-         if (data_p[0] == type)
-           {
-             *valp = data_p[1];
-             return 1;
-           }
-       }
-
-      offset += 2 * wordsize;
-    }
-
-  *valp = 0;
-  return 0;
-}
-
 #ifndef __powerpc64__
 static int ppc_regmap_adjusted;
 #endif
@@ -944,8 +907,8 @@ ppc_arch_setup (void)

   /* The value of current_process ()->tdesc needs to be set for this
      call.  */
-  ppc_get_auxv (AT_HWCAP, &ppc_hwcap);
-  ppc_get_auxv (AT_HWCAP2, &ppc_hwcap2);
+  ppc_hwcap = linux_get_hwcap (features.wordsize);
+  ppc_hwcap2 = linux_get_hwcap2 (features.wordsize);

   features.isa205 = ppc_linux_has_isa205 (ppc_hwcap);

diff --git a/gdb/gdbserver/linux-s390-low.c b/gdb/gdbserver/linux-s390-low.c
index edbef77fe9..f65a1ec38e 100644
--- a/gdb/gdbserver/linux-s390-low.c
+++ b/gdb/gdbserver/linux-s390-low.c
@@ -467,36 +467,6 @@ s390_set_pc (struct regcache *regcache, CORE_ADDR newpc)
     }
 }

-/* Get HWCAP from AUXV, using the given WORDSIZE.  Return the HWCAP, or
-   zero if not found.  */
-
-static unsigned long
-s390_get_hwcap (int wordsize)
-{
-  gdb_byte *data = (gdb_byte *) alloca (2 * wordsize);
-  int offset = 0;
-
-  while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize)
-    {
-      if (wordsize == 4)
-        {
-          unsigned int *data_p = (unsigned int *)data;
-          if (data_p[0] == AT_HWCAP)
-           return data_p[1];
-        }
-      else
-        {
-          unsigned long *data_p = (unsigned long *)data;
-          if (data_p[0] == AT_HWCAP)
-           return data_p[1];
-        }
-
-      offset += 2 * wordsize;
-    }
-
-  return 0;
-}
-
 /* Determine the word size for the given PID, in bytes.  */

 #ifdef __s390x__
@@ -548,7 +518,7 @@ s390_arch_setup (void)
   /* Determine word size and HWCAP.  */
   int pid = pid_of (current_thread);
   int wordsize = s390_get_wordsize (pid);
-  unsigned long hwcap = s390_get_hwcap (wordsize);
+  unsigned long hwcap = linux_get_hwcap (wordsize);

   /* Check whether the kernel supports extra register sets.  */
   int have_regset_last_break
Simon Marchi March 26, 2019, 2:06 p.m. | #3
On 2019-03-26 9:17 a.m., Alan Hayward wrote:
>>> @@ -545,8 +521,8 @@ aarch64_arch_setup (void)

>>>     if (is_elf64)

>>>       {

>>>         uint64_t vq = aarch64_sve_get_vq (tid);

>>> -      unsigned long hwcap = 0;

>>> -      bool pauth_p = aarch64_get_hwcap (&hwcap) && (hwcap & AARCH64_HWCAP_PACA);

>>> +      unsigned long hwcap = linux_get_hwcap (8);

>>> +      bool pauth_p = hwcap & AARCH64_HWCAP_PACA;

>>

>> Just wondering, can the linux-aarch64-low.c code be used to debug a process

> 

> "gdbserver :2345 a.out” works on an AArch64 box if that’s what you're asking?

> The code above is used to detect point auth.


Oops no, half of my sentence is missing!  I meant, can this code be used 
to debug 32 bit processes, and if so, is linux_get_hwcap (8) right.

> Updated patch below. Will push if you have no more comments.


For some reason, I am not able to apply "updated" patches you send.  I 
presume that you paste it and your email client changes the formatting. 
Could you maybe send it as an attached file?  It's not ideal either, 
because it makes it difficult to reply/comment on the patch, but at 
least the email client won't change the formatting.

Simon
Alan Hayward March 26, 2019, 2:34 p.m. | #4
(resending because mac mail forces to html format when attaching a file....)
    
    > On 26 Mar 2019, at 14:06, Simon Marchi <simark@simark.ca> wrote:

    > 

    > On 2019-03-26 9:17 a.m., Alan Hayward wrote:

    >>>> @@ -545,8 +521,8 @@ aarch64_arch_setup (void)

    >>>>    if (is_elf64)

    >>>>      {

    >>>>        uint64_t vq = aarch64_sve_get_vq (tid);

    >>>> -      unsigned long hwcap = 0;

    >>>> -      bool pauth_p = aarch64_get_hwcap (&hwcap) && (hwcap & AARCH64_HWCAP_PACA);

    >>>> +      unsigned long hwcap = linux_get_hwcap (8);

    >>>> +      bool pauth_p = hwcap & AARCH64_HWCAP_PACA;

    >>> 

    >>> Just wondering, can the linux-aarch64-low.c code be used to debug a process

    >> "gdbserver :2345 a.out” works on an AArch64 box if that’s what you're asking?

    >> The code above is used to detect point auth.

    > 

    > Oops no, half of my sentence is missing!  I meant, can this code be used to debug 32 bit processes, and if so, is linux_get_hwcap (8) right.

    > 

    
    It’s within a is_elf64 check, so should always be 8.
    
    
    >> Updated patch below. Will push if you have no more comments.

    > 

    > For some reason, I am not able to apply "updated" patches you send.  I presume that you paste it and your email client changes the formatting. Could you maybe send it as an attached file?  It's not ideal either, because it makes it difficult to reply/comment on the patch, but at least the email client won't change the formatting.

    > 

    
    Attached.
Simon Marchi March 26, 2019, 2:56 p.m. | #5
On 2019-03-26 10:33 a.m., Alan Hayward wrote:
> It’s within a is_elf64 check, so should always be 8.


Ah, good point.

>>> Updated patch below. Will push if you have no more comments.


It LGTM, thanks for doing this!

Simon
Peter Bergner April 2, 2019, 10 p.m. | #6
On 3/25/19 7:05 AM, Alan Hayward wrote:
> 	* linux-ppc-low.c (ppc_get_auxv): Remove function.

[snip]
> diff --git a/gdb/gdbserver/linux-ppc-low.c b/gdb/gdbserver/linux-ppc-low.c

> index 1b695e53fe..8deb0ce068 100644

> --- a/gdb/gdbserver/linux-ppc-low.c

> +++ b/gdb/gdbserver/linux-ppc-low.c

> @@ -323,43 +323,6 @@ ppc_set_pc (struct regcache *regcache, CORE_ADDR pc)

>      }

>  }

>  

> -

> -static int

> -ppc_get_auxv (unsigned long type, unsigned long *valp)

> -{

> -  const struct target_desc *tdesc = current_process ()->tdesc;

> -  int wordsize = register_size (tdesc, 0);

> -  unsigned char *data = (unsigned char *) alloca (2 * wordsize);

> -  int offset = 0;

> -

> -  while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize)

> -    {

> -      if (wordsize == 4)

> -	{

> -	  unsigned int *data_p = (unsigned int *)data;

> -	  if (data_p[0] == type)

> -	    {

> -	      *valp = data_p[1];

> -	      return 1;

> -	    }

> -	}

> -      else

> -	{

> -	  unsigned long *data_p = (unsigned long *)data;

> -	  if (data_p[0] == type)

> -	    {

> -	      *valp = data_p[1];

> -	      return 1;

> -	    }

> -	}

> -

> -      offset += 2 * wordsize;

> -    }

> -

> -  *valp = 0;

> -  return 0;

> -}

> -

>  #ifndef __powerpc64__

>  static int ppc_regmap_adjusted;

>  #endif

> @@ -944,8 +907,8 @@ ppc_arch_setup (void)

>  

>    /* The value of current_process ()->tdesc needs to be set for this

>       call.  */

> -  ppc_get_auxv (AT_HWCAP, &ppc_hwcap);

> -  ppc_get_auxv (AT_HWCAP2, &ppc_hwcap2);

> +  ppc_hwcap = linux_get_hwcap (features.wordsize);

> +  ppc_hwcap2 = linux_get_hwcap2 (features.wordsize);

>  

>    features.isa205 = ppc_linux_has_isa205 (ppc_hwcap);

>  


You have removed ppc_get_auxv(), but I'm still seeing uses of it that
function which are causing build errors (powerpc64le-linux):

/home/bergner/binutils/binutils-gdb/gdb/gdbserver/linux-ppc-low.c: In function ‘int is_elfv2_inferior()’:
/home/bergner/binutils/binutils-gdb/gdb/gdbserver/linux-ppc-low.c:1113:8: error: ‘ppc_get_auxv’ was not declared in this scope
   if (!ppc_get_auxv (AT_PHDR, &phdr))

Peter
Pedro Franco de Carvalho April 4, 2019, 9:22 p.m. | #7
Hi Peter,

Peter Bergner <bergner@linux.ibm.com> writes:

> You have removed ppc_get_auxv(), but I'm still seeing uses of it that

> function which are causing build errors (powerpc64le-linux):

>

> /home/bergner/binutils/binutils-gdb/gdb/gdbserver/linux-ppc-low.c: In function ‘int is_elfv2_inferior()’:

> /home/bergner/binutils/binutils-gdb/gdb/gdbserver/linux-ppc-low.c:1113:8: error: ‘ppc_get_auxv’ was not declared in this scope

>    if (!ppc_get_auxv (AT_PHDR, &phdr))

>

> Peter


I should be able to post a patch to fix this tomorrow (there is some
discussion for this elsewhere in the thread).

--
Pedo Franco de Carvalho

Patch

diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 20c75493b0..dc4ee81d2a 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -505,30 +505,6 @@  aarch64_linux_new_fork (struct process_info *parent,
 /* Matches HWCAP_PACA in kernel header arch/arm64/include/uapi/asm/hwcap.h.  */
 #define AARCH64_HWCAP_PACA (1 << 30)
 
-/* Fetch the AT_HWCAP entry from the auxv vector.  */
-
-static bool
-aarch64_get_hwcap (unsigned long *valp)
-{
-  unsigned char *data = (unsigned char *) alloca (16);
-  int offset = 0;
-
-  while ((*the_target->read_auxv) (offset, data, 16) == 16)
-    {
-      unsigned long *data_p = (unsigned long *)data;
-      if (data_p[0] == AT_HWCAP)
-	{
-	  *valp = data_p[1];
-	  return true;
-	}
-
-      offset += 16;
-    }
-
-  *valp = 0;
-  return false;
-}
-
 /* Implementation of linux_target_ops method "arch_setup".  */
 
 static void
@@ -545,8 +521,8 @@  aarch64_arch_setup (void)
   if (is_elf64)
     {
       uint64_t vq = aarch64_sve_get_vq (tid);
-      unsigned long hwcap = 0;
-      bool pauth_p = aarch64_get_hwcap (&hwcap) && (hwcap & AARCH64_HWCAP_PACA);
+      unsigned long hwcap = linux_get_hwcap (8);
+      bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
 
       current_process ()->tdesc = aarch64_linux_read_description (vq, pauth_p);
     }
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 8cad5c5fd4..ff72a489cb 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -847,40 +847,15 @@  get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self)
   return next_pc;
 }
 
-static int
-arm_get_hwcap (unsigned long *valp)
-{
-  unsigned char *data = (unsigned char *) alloca (8);
-  int offset = 0;
-
-  while ((*the_target->read_auxv) (offset, data, 8) == 8)
-    {
-      unsigned int *data_p = (unsigned int *)data;
-      if (data_p[0] == AT_HWCAP)
-	{
-	  *valp = data_p[1];
-	  return 1;
-	}
-
-      offset += 8;
-    }
-
-  *valp = 0;
-  return 0;
-}
-
 static const struct target_desc *
 arm_read_description (void)
 {
   int pid = lwpid_of (current_thread);
-  unsigned long arm_hwcap = 0;
+  unsigned long arm_hwcap = linux_get_hwcap (4);
 
   /* Query hardware watchpoint/breakpoint capabilities.  */
   arm_linux_init_hwbp_cap (pid);
 
-  if (arm_get_hwcap (&arm_hwcap) == 0)
-    return tdesc_arm;
-
   if (arm_hwcap & HWCAP_IWMMXT)
     return tdesc_arm_with_iwmmxt;
 
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 6f703f589f..481919c205 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -7423,6 +7423,64 @@  linux_get_pc_64bit (struct regcache *regcache)
   return pc;
 }
 
+/* Extract the auxiliary vector entry with a_type matching MATCH, storing the
+   value in VALP and returning true.  If no entry was found, return false.  */
+
+static bool
+linux_get_auxv (int wordsize, CORE_ADDR match, CORE_ADDR *valp)
+{
+  gdb_byte *data = (gdb_byte *) alloca (2 * wordsize);
+  int offset = 0;
+
+  while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize)
+    {
+      if (wordsize == 4)
+	{
+	  unsigned int *data_p = (unsigned int *)data;
+	  if (data_p[0] == match)
+	    {
+	      *valp = data_p[1];
+	      return true;
+	    }
+	}
+      else
+	{
+	  unsigned long *data_p = (unsigned long *)data;
+	  if (data_p[0] == match)
+	    {
+	      *valp = data_p[1];
+	      return true;
+	    }
+	}
+
+      offset += 2 * wordsize;
+    }
+
+  *valp = 0;
+  return false;
+}
+
+/* See linux-low.h.  */
+
+CORE_ADDR
+linux_get_hwcap (int wordsize)
+{
+  CORE_ADDR field;
+  if (!linux_get_auxv (wordsize, AT_HWCAP, &field))
+    return 0;
+  return field;
+}
+
+/* See linux-low.h.  */
+
+CORE_ADDR
+linux_get_hwcap2 (int wordsize)
+{
+  CORE_ADDR field;
+  if (!linux_get_auxv (wordsize, AT_HWCAP2, &field))
+    return 0;
+  return field;
+}
 
 static struct target_ops linux_target_ops = {
   linux_create_inferior,
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 1ade35d648..711a44b665 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -435,4 +435,12 @@  bool thread_db_thread_handle (ptid_t ptid, gdb_byte **handle, int *handle_len);
 
 extern int have_ptrace_getregset;
 
+/* Fetch the AT_HWCAP entry from the auxv vector, where entries are length
+   WORDSIZE.  */
+CORE_ADDR linux_get_hwcap (int wordsize);
+
+/* Fetch the AT_HWCAP2 entry from the auxv vector, where entries are length
+   WORDSIZE.  */
+CORE_ADDR linux_get_hwcap2 (int wordsize);
+
 #endif /* GDBSERVER_LINUX_LOW_H */
diff --git a/gdb/gdbserver/linux-ppc-low.c b/gdb/gdbserver/linux-ppc-low.c
index 1b695e53fe..8deb0ce068 100644
--- a/gdb/gdbserver/linux-ppc-low.c
+++ b/gdb/gdbserver/linux-ppc-low.c
@@ -323,43 +323,6 @@  ppc_set_pc (struct regcache *regcache, CORE_ADDR pc)
     }
 }
 
-
-static int
-ppc_get_auxv (unsigned long type, unsigned long *valp)
-{
-  const struct target_desc *tdesc = current_process ()->tdesc;
-  int wordsize = register_size (tdesc, 0);
-  unsigned char *data = (unsigned char *) alloca (2 * wordsize);
-  int offset = 0;
-
-  while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize)
-    {
-      if (wordsize == 4)
-	{
-	  unsigned int *data_p = (unsigned int *)data;
-	  if (data_p[0] == type)
-	    {
-	      *valp = data_p[1];
-	      return 1;
-	    }
-	}
-      else
-	{
-	  unsigned long *data_p = (unsigned long *)data;
-	  if (data_p[0] == type)
-	    {
-	      *valp = data_p[1];
-	      return 1;
-	    }
-	}
-
-      offset += 2 * wordsize;
-    }
-
-  *valp = 0;
-  return 0;
-}
-
 #ifndef __powerpc64__
 static int ppc_regmap_adjusted;
 #endif
@@ -944,8 +907,8 @@  ppc_arch_setup (void)
 
   /* The value of current_process ()->tdesc needs to be set for this
      call.  */
-  ppc_get_auxv (AT_HWCAP, &ppc_hwcap);
-  ppc_get_auxv (AT_HWCAP2, &ppc_hwcap2);
+  ppc_hwcap = linux_get_hwcap (features.wordsize);
+  ppc_hwcap2 = linux_get_hwcap2 (features.wordsize);
 
   features.isa205 = ppc_linux_has_isa205 (ppc_hwcap);
 
diff --git a/gdb/gdbserver/linux-s390-low.c b/gdb/gdbserver/linux-s390-low.c
index edbef77fe9..f65a1ec38e 100644
--- a/gdb/gdbserver/linux-s390-low.c
+++ b/gdb/gdbserver/linux-s390-low.c
@@ -467,36 +467,6 @@  s390_set_pc (struct regcache *regcache, CORE_ADDR newpc)
     }
 }
 
-/* Get HWCAP from AUXV, using the given WORDSIZE.  Return the HWCAP, or
-   zero if not found.  */
-
-static unsigned long
-s390_get_hwcap (int wordsize)
-{
-  gdb_byte *data = (gdb_byte *) alloca (2 * wordsize);
-  int offset = 0;
-
-  while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize)
-    {
-      if (wordsize == 4)
-        {
-          unsigned int *data_p = (unsigned int *)data;
-          if (data_p[0] == AT_HWCAP)
-	    return data_p[1];
-        }
-      else
-        {
-          unsigned long *data_p = (unsigned long *)data;
-          if (data_p[0] == AT_HWCAP)
-	    return data_p[1];
-        }
-
-      offset += 2 * wordsize;
-    }
-
-  return 0;
-}
-
 /* Determine the word size for the given PID, in bytes.  */
 
 #ifdef __s390x__
@@ -548,7 +518,7 @@  s390_arch_setup (void)
   /* Determine word size and HWCAP.  */
   int pid = pid_of (current_thread);
   int wordsize = s390_get_wordsize (pid);
-  unsigned long hwcap = s390_get_hwcap (wordsize);
+  unsigned long hwcap = linux_get_hwcap (wordsize);
 
   /* Check whether the kernel supports extra register sets.  */
   int have_regset_last_break