> On 25 Mar 2019, at 15:18, Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-03-25 8:05 a.m., Alan Hayward wrote:
>> Tidy up calls to read HWCAP (and HWCAP2) by adding common functions,
>> removing the PPC and AArch64 specific versions.
>> The only function difference is in aarch64_linux_core_read_description - if
>> the hwcap read fails it now return a valid description instead of nullptr.
>
> ARM also seems to have changed behavior, both the native target and core target. It's not necessarily, a problem, as long as it's a conscious change.
I should have mentioned it really. In the original it returned immediately on error.
In the new version it continues, but all the “if”s will fail and it’ll return at the
end of the function with the same result as the original early return.
>
>> diff --git a/gdb/linux-tdep.h b/gdb/linux-tdep.h
>> index 824ba3afaf..ee9c2bcc90 100644
>> --- a/gdb/linux-tdep.h
>> +++ b/gdb/linux-tdep.h
>> @@ -61,4 +61,10 @@ extern void linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch);
>> extern int linux_is_uclinux (void);
>> +/* Fetch the AT_HWCAP entry from the auxv vector for the given TARGET. */
>> +extern CORE_ADDR linux_get_hwcap (struct target_ops *target);
>> +
>> +/* Fetch the AT_HWCAP2 entry from the auxv vector for the given TARGET. */
>> +extern CORE_ADDR linux_get_hwcap2 (struct target_ops *target);
>
> For these two functions, can you mention that 0 is returned if the search in the AUXV vector fails?
>
Done.
> I was a bit surprised you didn't keep your version returning a bool to indicate whether the search succeeded or failed, but if this version is good enough, I am fine with it. If we ever have a target that really needs to differentiate between a lookup failure and a lookup success that returns the value 0, we can change it back or add another overload.
>
Once I had changed the AArch64 code, there was nothing left that was checking the error.
So it seemed better to remove it.
> This patch LGTM with the comment above updated.
>
> Simon
Thanks. Pushed code included below.
Alan.
@@ -42,6 +42,7 @@
#include <asm/ptrace.h>
#include "gregset.h"
+#include "linux-tdep.h"
/* Defines ps_err_e, struct ps_prochandle. */
#include "gdb_proc_service.h"
@@ -641,11 +642,10 @@ aarch64_linux_nat_target::read_description ()
if (ret == 0)
return tdesc_arm_with_neon;
- CORE_ADDR hwcap = 0;
- bool pauth_p = aarch64_linux_get_hwcap (this, &hwcap)
- && (hwcap & AARCH64_HWCAP_PACA);
+ CORE_ADDR hwcap = linux_get_hwcap (this);
- return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p);
+ return aarch64_read_description (aarch64_sve_get_vq (tid),
+ hwcap & AARCH64_HWCAP_PACA);
}
/* Convert a native/host siginfo object, into/from the siginfo in the
@@ -655,13 +655,10 @@ static const struct target_desc *
aarch64_linux_core_read_description (struct gdbarch *gdbarch,
struct target_ops *target, bfd *abfd)
{
- CORE_ADDR aarch64_hwcap = 0;
-
- if (!aarch64_linux_get_hwcap (target, &aarch64_hwcap))
- return nullptr;
+ CORE_ADDR hwcap = linux_get_hwcap (target);
return aarch64_read_description (aarch64_linux_core_read_vq (gdbarch, abfd),
- aarch64_hwcap & AARCH64_HWCAP_PACA);
+ hwcap & AARCH64_HWCAP_PACA);
}
/* Implementation of `gdbarch_stap_is_single_operand', as defined in
@@ -1439,15 +1436,6 @@ aarch64_linux_gcc_target_options (struct gdbarch *gdbarch)
return NULL;
}
-/* See aarch64-linux-tdep.h. */
-
-bool
-aarch64_linux_get_hwcap (struct target_ops *target, CORE_ADDR *hwcap)
-{
- *hwcap = 0;
- return target_auxv_search (target, AT_HWCAP, hwcap) == 1;
-}
-
static void
aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
{
@@ -42,7 +42,4 @@ extern const struct regset aarch64_linux_fpregset;
/* 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 for the given TARGET. */
-bool aarch64_linux_get_hwcap (struct target_ops *target, CORE_ADDR *hwcap);
-
#endif /* AARCH64_LINUX_TDEP_H */
@@ -533,7 +533,7 @@ ps_get_thread_area (struct ps_prochandle *ph,
const struct target_desc *
arm_linux_nat_target::read_description ()
{
- CORE_ADDR arm_hwcap = 0;
+ CORE_ADDR arm_hwcap = linux_get_hwcap (this);
if (have_ptrace_getregset == TRIBOOL_UNKNOWN)
{
@@ -551,11 +551,6 @@ arm_linux_nat_target::read_description ()
have_ptrace_getregset = TRIBOOL_TRUE;
}
- if (target_auxv_search (this, AT_HWCAP, &arm_hwcap) != 1)
- {
- return this->beneath ()->read_description ();
- }
-
if (arm_hwcap & HWCAP_IWMMXT)
return tdesc_arm_with_iwmmxt;
@@ -730,10 +730,7 @@ arm_linux_core_read_description (struct gdbarch *gdbarch,
struct target_ops *target,
bfd *abfd)
{
- CORE_ADDR arm_hwcap = 0;
-
- if (target_auxv_search (target, AT_HWCAP, &arm_hwcap) != 1)
- return NULL;
+ CORE_ADDR arm_hwcap = linux_get_hwcap (target);
if (arm_hwcap & HWCAP_VFP)
{
@@ -2462,6 +2462,28 @@ linux_displaced_step_location (struct gdbarch *gdbarch)
return addr;
}
+/* See linux-tdep.h. */
+
+CORE_ADDR
+linux_get_hwcap (struct target_ops *target)
+{
+ CORE_ADDR field;
+ if (target_auxv_search (target, AT_HWCAP, &field) != 1)
+ return 0;
+ return field;
+}
+
+/* See linux-tdep.h. */
+
+CORE_ADDR
+linux_get_hwcap2 (struct target_ops *target)
+{
+ CORE_ADDR field;
+ if (target_auxv_search (target, AT_HWCAP2, &field) != 1)
+ return 0;
+ return field;
+}
+
/* Display whether the gcore command is using the
/proc/PID/coredump_filter file. */
@@ -61,4 +61,12 @@ extern void linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch);
extern int linux_is_uclinux (void);
+/* Fetch the AT_HWCAP entry from the auxv vector for the given TARGET. On
+ error, 0 is returned. */
+extern CORE_ADDR linux_get_hwcap (struct target_ops *target);
+
+/* Fetch the AT_HWCAP2 entry from the auxv vector for the given TARGET. On
+ error, 0 is returned. */
+extern CORE_ADDR linux_get_hwcap2 (struct target_ops *target);
+
#endif /* linux-tdep.h */
@@ -1554,31 +1554,6 @@ store_ppc_registers (const struct regcache *regcache, int tid)
function to fail most of the time, so we ignore them. */
}
-/* Fetch the AT_HWCAP entry from the aux vector. */
-static CORE_ADDR
-ppc_linux_get_hwcap (void)
-{
- CORE_ADDR field;
-
- if (target_auxv_search (current_top_target (), AT_HWCAP, &field) != 1)
- return 0;
-
- return field;
-}
-
-/* Fetch the AT_HWCAP2 entry from the aux vector. */
-
-static CORE_ADDR
-ppc_linux_get_hwcap2 (void)
-{
- CORE_ADDR field;
-
- if (target_auxv_search (current_top_target (), AT_HWCAP2, &field) != 1)
- return 0;
-
- return field;
-}
-
/* The cached DABR value, to install in new threads.
This variable is used when the PowerPC HWDEBUG ptrace
interface is not available. */
@@ -1735,7 +1710,7 @@ ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
takes two hardware watchpoints though. */
if (len > 1
&& hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_RANGE
- && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+ && linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
return 2;
/* Check if the processor provides DAWR interface. */
if (hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_DAWR)
@@ -1755,7 +1730,7 @@ ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
ptrace interface, DAC-based processors (i.e., embedded processors) will
use addresses aligned to 4-bytes due to the way the read/write flags are
passed in the old ptrace interface. */
- else if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+ else if (((linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
&& (addr + len) > (addr & ~3) + 4)
|| (addr + len) > (addr & ~7) + 8)
return 0;
@@ -2294,7 +2269,7 @@ ppc_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len,
long dabr_value;
long read_mode, write_mode;
- if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+ if (linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
{
/* PowerPC 440 requires only the read/write flags to be passed
to the kernel. */
@@ -2497,9 +2472,9 @@ ppc_linux_nat_target::watchpoint_addr_within_range (CORE_ADDR addr,
int mask;
if (have_ptrace_hwdebug_interface ()
- && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+ && linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
return start <= addr && start + length >= addr;
- else if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
+ else if (linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
mask = 3;
else
mask = 7;
@@ -2637,8 +2612,8 @@ ppc_linux_nat_target::read_description ()
features.wordsize = ppc_linux_target_wordsize (tid);
- CORE_ADDR hwcap = ppc_linux_get_hwcap ();
- CORE_ADDR hwcap2 = ppc_linux_get_hwcap2 ();
+ CORE_ADDR hwcap = linux_get_hwcap (current_top_target ());
+ CORE_ADDR hwcap2 = linux_get_hwcap2 (current_top_target ());
if (have_ptrace_getsetvsxregs
&& (hwcap & PPC_FEATURE_HAS_VSX))
@@ -1601,10 +1601,7 @@ ppc_linux_core_read_description (struct gdbarch *gdbarch,
if (vsx)
features.vsx = true;
- CORE_ADDR hwcap;
-
- if (target_auxv_search (target, AT_HWCAP, &hwcap) != 1)
- hwcap = 0;
+ CORE_ADDR hwcap = linux_get_hwcap (target);
features.isa205 = ppc_linux_has_isa205 (hwcap);
@@ -1016,9 +1016,8 @@ s390_linux_nat_target::read_description ()
that mode, report s390 architecture with 64-bit GPRs. */
#ifdef __s390x__
{
- CORE_ADDR hwcap = 0;
+ CORE_ADDR hwcap = linux_get_hwcap (current_top_target ());
- target_auxv_search (current_top_target (), AT_HWCAP, &hwcap);
have_regset_tdb = (hwcap & HWCAP_S390_TE)
&& check_regset (tid, NT_S390_TDB, s390_sizeof_tdbregset);
@@ -332,10 +332,9 @@ s390_core_read_description (struct gdbarch *gdbarch,
struct target_ops *target, bfd *abfd)
{
asection *section = bfd_get_section_by_name (abfd, ".reg");
- CORE_ADDR hwcap = 0;
+ CORE_ADDR hwcap = linux_get_hwcap (target);
bool high_gprs, v1, v2, te, vx, gs;
- target_auxv_search (target, AT_HWCAP, &hwcap);
if (!section)
return NULL;