[2/2] gdbserver: Add linux_get_hwcap
Commit Message
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(-)
Comments
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
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
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
@@ -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);
}
@@ -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;
@@ -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,
@@ -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 */
@@ -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);
@@ -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