Message ID | 20240228031042.375726-1-thiago.bauermann@linaro.org |
---|---|
State | New |
Headers | show |
Series | [v2] gdb/arm: Remove tpidruro register from non-FreeBSD target descriptions | expand |
On 2/27/24 7:10 PM, Thiago Jung Bauermann wrote: > Commit 92d48a1e4eac ("Add an arm-tls feature which includes the tpidruro > register from CP15.") introduced the org.gnu.gdb.arm.tls feature, which > adds the tpidruro register, and unconditionally enabled it in > aarch32_create_target_description. > > In Linux, the tpidruro register isn't available via ptrace in the 32-bit > kernel but it is available for an aarch32 program running under an arm64 > kernel via the ptrace compat interface. This isn't currently implemented > however, which causes GDB on arm-linux with 64-bit kernel to list the > register but show it as unavailable, as reported by Tom de Vries: > > $ gdb -q -batch a.out -ex start -ex 'p $tpidruro' > Temporary breakpoint 1 at 0x512 > > Temporary breakpoint 1, 0xaaaaa512 in main () > $1 = <unavailable> > > Simon Marchi then clarified: > >> The only time we should be seeing some "unavailable" registers or memory >> is in the context of tracepoints, for things that are not collected. >> Seeing an unavailable register here is a sign that something is not >> right. > > Therefore, disable the TLS feature in aarch32 target descriptions for Linux > and NetBSD targets (the latter also doesn't seem to support accessing > tpidruro either, based on a quick look at arm-netbsd-nat.c). > > This patch fixes the following tests: > > Running gdb.base/inline-frame-cycle-unwind.exp ... > FAIL: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 3: backtrace when the unwind is broken at frame 3 > FAIL: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 5: backtrace when the unwind is broken at frame 5 > FAIL: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 1: backtrace when the unwind is broken at frame 1 > > Tested with Ubuntu 22.04.3 on armv8l-linux-gnueabihf in native, > native-gdbserver and native-extended-gdbserver targets with no regressions. > > PR tdep/31418 > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31418 > --- > > Hello, > > As often happens, after I sent v1 of this patch I noticed a problem with it: > It introduces two variants of aarch32 target description (one with TLS > support and one without), but only creates and returns the first one that was > requested. This version fixes the problem. > > The only change compared to v1 is in gdb/aarch32-tdep.c. > > Changes in v2: > - Cache two versions of tdesc_aarch32 in aarch32_read_description, one with > TLS support and one without.t Good catch, V2 looks ok to me as well. Approved-By: John Baldwin <jhb@FreeBSD.org>
John Baldwin <jhb@FreeBSD.org> writes: > On 2/27/24 7:10 PM, Thiago Jung Bauermann wrote: >> Hello, >> As often happens, after I sent v1 of this patch I noticed a problem with it: >> It introduces two variants of aarch32 target description (one with TLS >> support and one without), but only creates and returns the first one that was >> requested. This version fixes the problem. >> The only change compared to v1 is in gdb/aarch32-tdep.c. >> Changes in v2: >> - Cache two versions of tdesc_aarch32 in aarch32_read_description, one with >> TLS support and one without.t > > Good catch, V2 looks ok to me as well. > > Approved-By: John Baldwin <jhb@FreeBSD.org> Thank you again! Just pushed as commit bbb12eb9c84a.
diff --git a/gdb/aarch32-tdep.c b/gdb/aarch32-tdep.c index 9177b47d1481..0b7783c3e15d 100644 --- a/gdb/aarch32-tdep.c +++ b/gdb/aarch32-tdep.c @@ -22,15 +22,20 @@ #include "gdbsupport/common-regcache.h" #include "arch/aarch32.h" -static struct target_desc *tdesc_aarch32; +static struct target_desc *tdesc_aarch32_list[2]; /* See aarch32-tdep.h. */ const target_desc * -aarch32_read_description () +aarch32_read_description (bool tls) { - if (tdesc_aarch32 == nullptr) - tdesc_aarch32 = aarch32_create_target_description (); + struct target_desc *tdesc = tdesc_aarch32_list[tls]; - return tdesc_aarch32; + if (tdesc == nullptr) + { + tdesc = aarch32_create_target_description (tls); + tdesc_aarch32_list[tls] = tdesc; + } + + return tdesc; } diff --git a/gdb/aarch32-tdep.h b/gdb/aarch32-tdep.h index 654483438485..009ee61890e2 100644 --- a/gdb/aarch32-tdep.h +++ b/gdb/aarch32-tdep.h @@ -22,6 +22,6 @@ struct target_desc; /* Get the AArch32 target description. */ -const target_desc *aarch32_read_description (); +const target_desc *aarch32_read_description (bool tls); #endif /* aarch32-tdep.h. */ diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index 11a41e1afae0..9dc45e1c1d96 100644 --- a/gdb/aarch64-linux-nat.c +++ b/gdb/aarch64-linux-nat.c @@ -887,7 +887,7 @@ aarch64_linux_nat_target::read_description () ret = ptrace (PTRACE_GETREGSET, tid, NT_ARM_VFP, &iovec); if (ret == 0) - return aarch32_read_description (); + return aarch32_read_description (false); CORE_ADDR hwcap = linux_get_hwcap (); CORE_ADDR hwcap2 = linux_get_hwcap2 (); diff --git a/gdb/arch/aarch32.c b/gdb/arch/aarch32.c index c910e3b5a388..9f3e25088f41 100644 --- a/gdb/arch/aarch32.c +++ b/gdb/arch/aarch32.c @@ -25,7 +25,7 @@ /* See aarch32.h. */ target_desc * -aarch32_create_target_description () +aarch32_create_target_description (bool tls) { target_desc_up tdesc = allocate_target_description (); @@ -39,7 +39,8 @@ aarch32_create_target_description () /* Create a vfpv3 feature, then a blank NEON feature. */ regnum = create_feature_arm_arm_vfpv3 (tdesc.get (), regnum); tdesc_create_feature (tdesc.get (), "org.gnu.gdb.arm.neon"); - regnum = create_feature_arm_arm_tls (tdesc.get (), regnum); + if (tls) + regnum = create_feature_arm_arm_tls (tdesc.get (), regnum); return tdesc.release (); } diff --git a/gdb/arch/aarch32.h b/gdb/arch/aarch32.h index f7ee6faeea3c..1811b8a7a925 100644 --- a/gdb/arch/aarch32.h +++ b/gdb/arch/aarch32.h @@ -22,6 +22,6 @@ /* Create the AArch32 target description. */ -target_desc *aarch32_create_target_description (); +target_desc *aarch32_create_target_description (bool tls); #endif /* aarch32.h. */ diff --git a/gdb/arm-fbsd-tdep.c b/gdb/arm-fbsd-tdep.c index b485951c3764..7b82de2166b5 100644 --- a/gdb/arm-fbsd-tdep.c +++ b/gdb/arm-fbsd-tdep.c @@ -228,7 +228,7 @@ arm_fbsd_read_description_auxv (const std::optional<gdb::byte_vector> &auxv, if (arm_hwcap & HWCAP_VFP) { if (arm_hwcap & HWCAP_NEON) - return aarch32_read_description (); + return aarch32_read_description (tls); else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPD32)) == (HWCAP_VFPv3 | HWCAP_VFPD32)) return arm_read_description (ARM_FP_TYPE_VFPV3, tls); diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c index 02994732563a..75f498cd5b3f 100644 --- a/gdb/arm-linux-nat.c +++ b/gdb/arm-linux-nat.c @@ -568,7 +568,7 @@ arm_linux_nat_target::read_description () /* NEON implies VFPv3-D32 or no-VFP unit. Say that we only support Neon with VFPv3-D32. */ if (arm_hwcap & HWCAP_NEON) - return aarch32_read_description (); + return aarch32_read_description (false); else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPv3D16)) == HWCAP_VFPv3) return arm_read_description (ARM_FP_TYPE_VFPV3, false); diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c index cc79247aaf12..a8b27a7463a6 100644 --- a/gdb/arm-linux-tdep.c +++ b/gdb/arm-linux-tdep.c @@ -740,7 +740,7 @@ arm_linux_core_read_description (struct gdbarch *gdbarch, /* NEON implies VFPv3-D32 or no-VFP unit. Say that we only support Neon with VFPv3-D32. */ if (arm_hwcap & HWCAP_NEON) - return aarch32_read_description (); + return aarch32_read_description (false); else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPv3D16)) == HWCAP_VFPv3) return arm_read_description (ARM_FP_TYPE_VFPV3, false); diff --git a/gdb/arm-netbsd-nat.c b/gdb/arm-netbsd-nat.c index df8b5ec7b03c..4b9f92946412 100644 --- a/gdb/arm-netbsd-nat.c +++ b/gdb/arm-netbsd-nat.c @@ -350,7 +350,7 @@ arm_netbsd_nat_target::read_description () len = sizeof(flag); if (sysctlbyname("machdep.neon_present", &flag, &len, NULL, 0) == 0 && flag) - return aarch32_read_description (); + return aarch32_read_description (false); return arm_read_description (ARM_FP_TYPE_VFPV3, false); } diff --git a/gdbserver/linux-aarch32-tdesc.cc b/gdbserver/linux-aarch32-tdesc.cc index a696d8946e29..54c6f62e9965 100644 --- a/gdbserver/linux-aarch32-tdesc.cc +++ b/gdbserver/linux-aarch32-tdesc.cc @@ -32,7 +32,7 @@ aarch32_linux_read_description () { if (tdesc_aarch32 == nullptr) { - tdesc_aarch32 = aarch32_create_target_description (); + tdesc_aarch32 = aarch32_create_target_description (false); static const char *expedite_regs[] = { "r11", "sp", "pc", 0 }; init_target_desc (tdesc_aarch32, expedite_regs);