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

Message ID 20230328115449.6526-1-mohamed.bouhaouel@intel.com
Headers
Series Check for zpoint support when handling watchpoints |

Message

Bouhaouel, Mohamed March 28, 2023, 11:54 a.m. UTC
  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)

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(-)
  

Comments

Guinevere Larsen Nov. 7, 2023, 10:48 a.m. UTC | #1
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>