[PATCH] [gdb/aarch64] Handle unknown debug architecture versions more gracefully

[PATCH] [gdb/aarch64] Handle unknown debug architecture versions more gracefully

[Luis Machado]

In PR 30340 it was reported that a KVM-based AArch64 system's kernel was
returning a debug architecture version of 0 when fetching both the
NT_ARM_HW_WATCH and NT_ARM_HW_BREAK register sets.

Even though the debug architecture version being reported is invalid, gdb can
still make things work by ignoring that information and relying on the counts
of hardware watchpoints and hardware breakpoints.

This patch makes gdb handle this situation a bit more gracefully by causing gdb
to warn when it sees an invalid/unknown debug architecture version, but still
allowing gdb to detect the number of hardware watchpoints and hardware
breakpoints available.

PR tdep/30340
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30340
---
 gdb/nat/aarch64-linux-hw-point.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
index ccb47cd5aa2..b014d387e1e 100644
--- a/gdb/nat/aarch64-linux-hw-point.c
+++ b/gdb/nat/aarch64-linux-hw-point.c
@@ -250,10 +250,21 @@ aarch64_linux_get_debug_reg_capacity (int tid)
   iov.iov_base = &dreg_state;
   iov.iov_len = sizeof (dreg_state);
 
+  /* It has been reported that under KVM the debug architecture version can
+     be reported as 0, which is invalid.  But this doesn't mean gdb can't use
+     hardware debugging resources.  Instead of bailing out, carry on fetching
+     the hardware breakpoints/watchpoints count so we can potentially get back
+     on track.  */
+
   /* Get hardware watchpoint register info.  */
-  if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_HW_WATCH, &iov) == 0
-      && compatible_debug_arch (AARCH64_DEBUG_ARCH (dreg_state.dbg_info)))
+  if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_HW_WATCH, &iov) == 0)
     {
+      if (!compatible_debug_arch (AARCH64_DEBUG_ARCH (dreg_state.dbg_info)))
+	warning (_("Unknown/Invalid debug architecture version %d.\n"
+		   "Attempting to fetch the number of hardware watchpoints "
+		   "available."),
+		   AARCH64_DEBUG_ARCH (dreg_state.dbg_info));
+
       aarch64_num_wp_regs = AARCH64_DEBUG_NUM_SLOTS (dreg_state.dbg_info);
       if (aarch64_num_wp_regs > AARCH64_HWP_MAX_NUM)
 	{
@@ -271,9 +282,14 @@ aarch64_linux_get_debug_reg_capacity (int tid)
     }
 
   /* Get hardware breakpoint register info.  */
-  if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_HW_BREAK, &iov) == 0
-      && compatible_debug_arch (AARCH64_DEBUG_ARCH (dreg_state.dbg_info)))
+  if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_HW_BREAK, &iov) == 0)
     {
+      if (!compatible_debug_arch (AARCH64_DEBUG_ARCH (dreg_state.dbg_info)))
+	warning (_("Unknown/Invalid debug architecture version %d.\n"
+		   "Attempting to fetch the number of hardware breakpoints "
+		   "available."),
+		   AARCH64_DEBUG_ARCH (dreg_state.dbg_info));
+
       aarch64_num_bp_regs = AARCH64_DEBUG_NUM_SLOTS (dreg_state.dbg_info);
       if (aarch64_num_bp_regs > AARCH64_HBP_MAX_NUM)
 	{
-- 
2.25.1

Re: [PATCH] [gdb/aarch64] Handle unknown debug architecture versions more gracefully

[Andrew Burgess]

Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:

> In PR 30340 it was reported that a KVM-based AArch64 system's kernel was
> returning a debug architecture version of 0 when fetching both the
> NT_ARM_HW_WATCH and NT_ARM_HW_BREAK register sets.
>
> Even though the debug architecture version being reported is invalid, gdb can
> still make things work by ignoring that information and relying on the counts
> of hardware watchpoints and hardware breakpoints.
>
> This patch makes gdb handle this situation a bit more gracefully by causing gdb
> to warn when it sees an invalid/unknown debug architecture version, but still
> allowing gdb to detect the number of hardware watchpoints and hardware
> breakpoints available.
>
> PR tdep/30340

Apparently we no longer need to include this cookie.  Just using the bug
url should be enough for this commit to be linked bugzilla.

> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30340
> ---
>  gdb/nat/aarch64-linux-hw-point.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
> index ccb47cd5aa2..b014d387e1e 100644
> --- a/gdb/nat/aarch64-linux-hw-point.c
> +++ b/gdb/nat/aarch64-linux-hw-point.c
> @@ -250,10 +250,21 @@ aarch64_linux_get_debug_reg_capacity (int tid)
>    iov.iov_base = &dreg_state;
>    iov.iov_len = sizeof (dreg_state);
>  
> +  /* It has been reported that under KVM the debug architecture version can
> +     be reported as 0, which is invalid.  But this doesn't mean gdb can't use
> +     hardware debugging resources.  Instead of bailing out, carry on fetching
> +     the hardware breakpoints/watchpoints count so we can potentially get back
> +     on track.  */

Given my recent comment on pr gdb/30340 you might want to reword the
comment a little -- we now understand why we saw 0 in this case.

> +
>    /* Get hardware watchpoint register info.  */
> -  if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_HW_WATCH, &iov) == 0
> -      && compatible_debug_arch (AARCH64_DEBUG_ARCH (dreg_state.dbg_info)))
> +  if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_HW_WATCH, &iov) == 0)
>      {
> +      if (!compatible_debug_arch (AARCH64_DEBUG_ARCH (dreg_state.dbg_info)))
> +	warning (_("Unknown/Invalid debug architecture version %d.\n"
> +		   "Attempting to fetch the number of hardware watchpoints "
> +		   "available."),
> +		   AARCH64_DEBUG_ARCH (dreg_state.dbg_info));
> +
>        aarch64_num_wp_regs = AARCH64_DEBUG_NUM_SLOTS (dreg_state.dbg_info);

I guess the risk here is that on a later architecture the format of
dreg_state might change, in which case reading the wp/bp count might not
get the correct result.

But in that case we'd print the warning, so when things go wrong later
we will have given the user a clue for what the problem might be.

So, I think this change seems a reasonable improvement.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


>        if (aarch64_num_wp_regs > AARCH64_HWP_MAX_NUM)
>  	{
> @@ -271,9 +282,14 @@ aarch64_linux_get_debug_reg_capacity (int tid)
>      }
>  
>    /* Get hardware breakpoint register info.  */
> -  if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_HW_BREAK, &iov) == 0
> -      && compatible_debug_arch (AARCH64_DEBUG_ARCH (dreg_state.dbg_info)))
> +  if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_HW_BREAK, &iov) == 0)
>      {
> +      if (!compatible_debug_arch (AARCH64_DEBUG_ARCH (dreg_state.dbg_info)))
> +	warning (_("Unknown/Invalid debug architecture version %d.\n"
> +		   "Attempting to fetch the number of hardware breakpoints "
> +		   "available."),
> +		   AARCH64_DEBUG_ARCH (dreg_state.dbg_info));
> +
>        aarch64_num_bp_regs = AARCH64_DEBUG_NUM_SLOTS (dreg_state.dbg_info);
>        if (aarch64_num_bp_regs > AARCH64_HBP_MAX_NUM)
>  	{
> -- 
> 2.25.1