Re: [PATCH 0/3] Check for zpoint support when handling watchpoints

[Guinevere Larsen <blarsen@redhat.com>]

On 28/03/2023 13:54, Mohamed Bouhaouel via Gdb-patches wrote:
> Hi all,
>
> This set of patches improves the flow of inserting a watchpoint or a
> hardware breakpoint.  It consists of checking whether the target
> supports the z_point before doing the actual insertion.  Having such a
> flow would avoid unnecessary too many warning messages, not
> increment the breakpoint counter when not supported, and prevent
> commands from being aborted upon resuming.
>
> Below are the behaviors, with and without the patches, of trying to
> insert a hardware watchpoint when the target does not support that.
>
> * Original behavior
>
> (gdb) awatch dim0
> Hardware watchpoint 2: dim0
> Warning:
> Could not insert hardware watchpoint 2.
> Could not insert hardware breakpoints:
> You may have requested too many hardware breakpoints/watchpoint.
>
> (gdb) continue
> Hardware watchpoint 2: dim0
> Warning:
> Could not insert hardware watchpoint 2.
> Could not insert hardware breakpoints:
> You may have requested too many hardware breakpoints/watchpoint.
>
> Command aborted.
> (gdb)
>
> The warning messages keep popping up on each resume command (`continue`,
> `step`, ...) until the user manually deletes the watchpoint.
>
> * With patches applied
>
> (gdb) awatch dim0
> Target does not support this type of hardware watchpoint.
> (gdb)
Hi Mohamed!

Thanks for working on this, and sorry for the horrible delay in getting 
a review. I read through the series, and patches 2 and 3 look fine, you 
can add my review tag to them. My comments about patch 1 were sent in 
reply to that.

I hope some maintainer looks at this soon!
Reviewed-By: Guinevere Larsen <blarsen@redhat.com>

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> Regards,
> Mohamed
>
> Mohamed Bouhaouel (3):
>    gdb, gdbserver, zpoint: report z_point support
>    gdb, breakpoint: add a breakpoint type converter
>    gdb, zpoint: check for target hardware breakpoint support
>
>   gdb/breakpoint.c       | 20 ++++++++++++++++++++
>   gdb/breakpoint.h       |  3 +++
>   gdb/remote.c           | 41 ++++++++++++++++++++++++++++++++++++++---
>   gdbserver/mem-break.cc |  2 +-
>   gdbserver/mem-break.h  |  4 ++++
>   gdbserver/server.cc    |  7 +++++++
>   6 files changed, 73 insertions(+), 4 deletions(-)
>