* [PATCH v3 0/3] Check for zpoint support when handling watchpoints @ 2023-11-14 13:35 Mohamed Bouhaouel 2023-11-14 13:35 ` [PATCH v3 1/3] gdb, gdbserver, zpoint: report z_point support Mohamed Bouhaouel ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Mohamed Bouhaouel @ 2023-11-14 13:35 UTC (permalink / raw) To: gdb-patches; +Cc: blarsen, mohamed.bouhaouel, eliz Hi, Thanks Eli Zaretskii for your feedback, I updated the first patch accordingly. V1 of this series can be found here: https://sourceware.org/pipermail/gdb-patches/2023-March/198371.html V2 of this series can be found here: https://sourceware.org/pipermail/gdb-patches/2023-November/204091.html Changes since V2: * Updated text format in the added documentation. 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/NEWS | 4 ++++ gdb/breakpoint.c | 20 ++++++++++++++++++++ gdb/breakpoint.h | 3 +++ gdb/doc/gdb.texinfo | 32 ++++++++++++++++++++++++++++++++ gdb/remote.c | 42 +++++++++++++++++++++++++++++++++++++++--- gdbserver/mem-break.cc | 2 +- gdbserver/mem-break.h | 4 ++++ gdbserver/server.cc | 7 +++++++ 8 files changed, 110 insertions(+), 4 deletions(-) -- 2.25.1 Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/3] gdb, gdbserver, zpoint: report z_point support 2023-11-14 13:35 [PATCH v3 0/3] Check for zpoint support when handling watchpoints Mohamed Bouhaouel @ 2023-11-14 13:35 ` Mohamed Bouhaouel 2023-11-14 13:45 ` Eli Zaretskii 2023-11-14 14:36 ` Tom Tromey 2023-11-14 13:35 ` [PATCH v3 2/3] gdb, breakpoint: add a breakpoint type converter Mohamed Bouhaouel 2023-11-14 13:35 ` [PATCH v3 3/3] gdb, zpoint: check for target hardware breakpoint support Mohamed Bouhaouel 2 siblings, 2 replies; 12+ messages in thread From: Mohamed Bouhaouel @ 2023-11-14 13:35 UTC (permalink / raw) To: gdb-patches; +Cc: blarsen, mohamed.bouhaouel, eliz Make gdbserver report which types of breakpoints and watchpoints are supported when exchanging features. This is done by replying with Z<type>+ (Supported) or Z<type>- (unsupported) Reviewed-By: Eli Zaretskii <eliz@gnu.org> --- gdb/NEWS | 4 ++++ gdb/doc/gdb.texinfo | 32 ++++++++++++++++++++++++++++++++ gdb/remote.c | 16 ++++++++++++++++ gdbserver/mem-break.cc | 2 +- gdbserver/mem-break.h | 4 ++++ gdbserver/server.cc | 7 +++++++ 6 files changed, 64 insertions(+), 1 deletion(-) diff --git a/gdb/NEWS b/gdb/NEWS index 96aba256dbb..6e9f679dd56 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -75,6 +75,10 @@ QThreadOptions in qSupported QThreadOptions packet, and the qSupported response can contain the set of thread options the remote stub supports. +qSupported + Gdbserver now reports which types of breakpoints and watchpoints + are supported when exchanging features using Z<type>+, Z<type>-. + *** Changes in GDB 14 * GDB now supports the AArch64 Scalable Matrix Extension 2 (SME2), which diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index e4c00143fd1..ccc513df0cc 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -44631,6 +44631,31 @@ These are the currently defined stub features and their properties: @tab @samp{-} @tab No +@item @samp{Z0} +@tab No +@tab @samp{-} +@tab No + +@item @samp{Z1} +@tab No +@tab @samp{-} +@tab No + +@item @samp{Z2} +@tab No +@tab @samp{-} +@tab No + +@item @samp{Z3} +@tab No +@tab @samp{-} +@tab No + +@item @samp{Z4} +@tab No +@tab @samp{-} +@tab No + @end multitable These are the currently defined stub features, in more detail: @@ -44858,6 +44883,13 @@ The remote stub supports and implements the required memory tagging functionality and understands the @samp{qMemTags} (@pxref{qMemTags}) and @samp{QMemTags} (@pxref{QMemTags}) packets. +@item Z@var{type} +The remote stub reports which types of breakpoints and watchpoints are +supported. This is done using flags, @samp{Z@var{type}+} for supported +and @samp{Z@var{type}-} for unsupported, where @var{type} is one of +@samp{0}, @samp{1}, @samp{2}, @samp{3}, @samp{4} +(@pxref{insert breakpoint or watchpoint packet}). + For AArch64 GNU/Linux systems, this feature also requires access to the @file{/proc/@var{pid}/smaps} file so memory mapping page flags can be inspected. This is done via the @samp{vFile} requests. diff --git a/gdb/remote.c b/gdb/remote.c index ce5addade6f..7aa380a5989 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -808,6 +808,10 @@ class remote_target : public process_stratum_target int insert_watchpoint (CORE_ADDR, int, enum target_hw_bp_type, struct expression *) override; + /* Returns true if GDB Z breakpoint type TYPE is supported, + false otherwise. */ + bool supports_z_point_type (int type); + int remove_watchpoint (CORE_ADDR, int, enum target_hw_bp_type, struct expression *) override; @@ -5717,6 +5721,11 @@ static const struct protocol_feature remote_protocol_features[] = { { "no-resumed", PACKET_DISABLE, remote_supported_packet, PACKET_no_resumed }, { "memory-tagging", PACKET_DISABLE, remote_supported_packet, PACKET_memory_tagging_feature }, + { "Z0", PACKET_SUPPORT_UNKNOWN, remote_supported_packet, PACKET_Z0 }, + { "Z1", PACKET_SUPPORT_UNKNOWN, remote_supported_packet, PACKET_Z1 }, + { "Z2", PACKET_SUPPORT_UNKNOWN, remote_supported_packet, PACKET_Z2 }, + { "Z3", PACKET_SUPPORT_UNKNOWN, remote_supported_packet, PACKET_Z3 }, + { "Z4", PACKET_SUPPORT_UNKNOWN, remote_supported_packet, PACKET_Z4 }, }; static char *remote_support_xml; @@ -10991,6 +11000,13 @@ watchpoint_to_Z_packet (int type) } } +bool +remote_target::supports_z_point_type (int type) +{ + Z_packet_type packet = watchpoint_to_Z_packet (type); + return (m_features.packet_support (PACKET_Z0 + packet) != PACKET_DISABLE); +} + int remote_target::insert_watchpoint (CORE_ADDR addr, int len, enum target_hw_bp_type type, struct expression *cond) diff --git a/gdbserver/mem-break.cc b/gdbserver/mem-break.cc index 3bee8bc8990..7c3a1d1d47e 100644 --- a/gdbserver/mem-break.cc +++ b/gdbserver/mem-break.cc @@ -998,7 +998,7 @@ find_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind) return NULL; } -static int +int z_type_supported (char z_type) { return (z_type >= '0' && z_type <= '4' diff --git a/gdbserver/mem-break.h b/gdbserver/mem-break.h index 9bf7aa84932..b27f0a23ac2 100644 --- a/gdbserver/mem-break.h +++ b/gdbserver/mem-break.h @@ -276,4 +276,8 @@ int remove_memory_breakpoint (struct raw_breakpoint *bp); void clone_all_breakpoints (struct thread_info *child_thread, const struct thread_info *parent_thread); + +/* Returns true if Z_TYPE is supported by the target. */ + +int z_type_supported (char z_type); #endif /* GDBSERVER_MEM_BREAK_H */ diff --git a/gdbserver/server.cc b/gdbserver/server.cc index a8e23561dcb..e727973ad45 100644 --- a/gdbserver/server.cc +++ b/gdbserver/server.cc @@ -2613,6 +2613,13 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) if (target_supports_memory_tagging ()) strcat (own_buf, ";memory-tagging+"); + /* Z points support. */ + strcat (own_buf, z_type_supported ('0') ? ";Z0+" : ";Z0-"); + strcat (own_buf, z_type_supported ('1') ? ";Z1+" : ";Z1-"); + strcat (own_buf, z_type_supported ('2') ? ";Z2+" : ";Z2-"); + strcat (own_buf, z_type_supported ('3') ? ";Z3+" : ";Z3-"); + strcat (own_buf, z_type_supported ('4') ? ";Z4+" : ";Z4-"); + /* Reinitialize components as needed for the new connection. */ hostio_handle_new_gdb_connection (); target_handle_new_gdb_connection (); -- 2.25.1 Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/3] gdb, gdbserver, zpoint: report z_point support 2023-11-14 13:35 ` [PATCH v3 1/3] gdb, gdbserver, zpoint: report z_point support Mohamed Bouhaouel @ 2023-11-14 13:45 ` Eli Zaretskii 2023-11-14 14:36 ` Tom Tromey 1 sibling, 0 replies; 12+ messages in thread From: Eli Zaretskii @ 2023-11-14 13:45 UTC (permalink / raw) To: Mohamed Bouhaouel; +Cc: gdb-patches, blarsen > From: Mohamed Bouhaouel <mohamed.bouhaouel@intel.com> > Cc: blarsen@redhat.com, > mohamed.bouhaouel@intel.com, > eliz@gnu.org > Date: Tue, 14 Nov 2023 14:35:30 +0100 > > Make gdbserver report which types of breakpoints and watchpoints > are supported when exchanging features. This is done by replying with > Z<type>+ (Supported) or Z<type>- (unsupported) > > Reviewed-By: Eli Zaretskii <eliz@gnu.org> > --- > gdb/NEWS | 4 ++++ > gdb/doc/gdb.texinfo | 32 ++++++++++++++++++++++++++++++++ > gdb/remote.c | 16 ++++++++++++++++ > gdbserver/mem-break.cc | 2 +- > gdbserver/mem-break.h | 4 ++++ > gdbserver/server.cc | 7 +++++++ > 6 files changed, 64 insertions(+), 1 deletion(-) Thanks, the documentation parts are okay. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/3] gdb, gdbserver, zpoint: report z_point support 2023-11-14 13:35 ` [PATCH v3 1/3] gdb, gdbserver, zpoint: report z_point support Mohamed Bouhaouel 2023-11-14 13:45 ` Eli Zaretskii @ 2023-11-14 14:36 ` Tom Tromey 2023-11-15 17:31 ` Bouhaouel, Mohamed 1 sibling, 1 reply; 12+ messages in thread From: Tom Tromey @ 2023-11-14 14:36 UTC (permalink / raw) To: Mohamed Bouhaouel; +Cc: gdb-patches, blarsen, eliz >>>>> Mohamed Bouhaouel <mohamed.bouhaouel@intel.com> writes: > Make gdbserver report which types of breakpoints and watchpoints > are supported when exchanging features. This is done by replying with > Z<type>+ (Supported) or Z<type>- (unsupported) Thanks for the patch. I suppose the idea is that this work can't really be done on the gdb side, because gdb creates the watchpoint and such earlier, and only then tries to communicate with the remote... so errors come too late? > +@item Z@var{type} > +The remote stub reports which types of breakpoints and watchpoints are > +supported. This is done using flags, @samp{Z@var{type}+} for supported I think this part of the docs should mention that the 'Z' response assumes that both of the corresponding 'z' and 'Z' packets are supported. > + return (m_features.packet_support (PACKET_Z0 + packet) != PACKET_DISABLE); Too many parens. > +/* Returns true if Z_TYPE is supported by the target. */ > + > +int z_type_supported (char z_type); > #endif /* GDBSERVER_MEM_BREAK_H */ Should have a newline before the #endif. Tom ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v3 1/3] gdb, gdbserver, zpoint: report z_point support 2023-11-14 14:36 ` Tom Tromey @ 2023-11-15 17:31 ` Bouhaouel, Mohamed 0 siblings, 0 replies; 12+ messages in thread From: Bouhaouel, Mohamed @ 2023-11-15 17:31 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, blarsen, eliz Thanks Tom for the feedback, > I suppose the idea is that this work can't really be done on the gdb > side, because gdb creates the watchpoint and such earlier, and only then > tries to communicate with the remote... so errors come too late? Yes, and the error messages keep popping up on each resume command (`continue`, `step`, ...) until the user manually deletes the watchpoint. > I think this part of the docs should mention that the 'Z' response > assumes that both of the corresponding 'z' and 'Z' packets are > supported. Right, will be added in the next patch update. > > + return (m_features.packet_support (PACKET_Z0 + packet) != > PACKET_DISABLE); > > Too many parens. Will be addressed in the next patch update. > > +int z_type_supported (char z_type); > > #endif /* GDBSERVER_MEM_BREAK_H */ > > Should have a newline before the #endif. Will be addressed in the next patch update. Regards, Mohamed Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/3] gdb, breakpoint: add a breakpoint type converter 2023-11-14 13:35 [PATCH v3 0/3] Check for zpoint support when handling watchpoints Mohamed Bouhaouel 2023-11-14 13:35 ` [PATCH v3 1/3] gdb, gdbserver, zpoint: report z_point support Mohamed Bouhaouel @ 2023-11-14 13:35 ` Mohamed Bouhaouel 2023-11-14 14:37 ` Tom Tromey 2023-11-14 13:35 ` [PATCH v3 3/3] gdb, zpoint: check for target hardware breakpoint support Mohamed Bouhaouel 2 siblings, 1 reply; 12+ messages in thread From: Mohamed Bouhaouel @ 2023-11-14 13:35 UTC (permalink / raw) To: gdb-patches; +Cc: blarsen, mohamed.bouhaouel, eliz Add a new function 'bptype_to_target_hw_bp_type' to perform the translation from 'bptype' to 'target_hw_bp_type'. Reviewed-By: Guinevere Larsen <blarsen@redhat.com> --- gdb/breakpoint.c | 20 ++++++++++++++++++++ gdb/breakpoint.h | 3 +++ 2 files changed, 23 insertions(+) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index fe09dbcbeee..13eaff79789 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -9827,6 +9827,26 @@ watchpoint::~watchpoint () bpt->related_breakpoint = this->related_breakpoint; } +/* See breakpoint.h. */ + +target_hw_bp_type +bptype_to_target_hw_bp_type (bptype type) +{ + switch (type) + { + case bp_hardware_watchpoint: + return hw_write; + case bp_read_watchpoint: + return hw_read; + case bp_access_watchpoint: + return hw_access; + case bp_hardware_breakpoint: + return hw_execute; + default: + error (_ ("Bad breakpoint type: bptype %d."), type); + } +} + /* Return non-zero if EXP is verified as constant. Returned zero means EXP is variable. Also the constant detection may fail for some constant expressions and in such case still falsely return diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index e75efc90495..cc9a1ebc184 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -2029,4 +2029,7 @@ extern void enable_disable_bp_location (bp_location *loc, bool enable); extern void notify_breakpoint_modified (breakpoint *b); +/* Translate BPTYPE to TARGET_HW_BP_TYPE. */ + +extern target_hw_bp_type bptype_to_target_hw_bp_type (bptype type); #endif /* !defined (BREAKPOINT_H) */ -- 2.25.1 Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/3] gdb, breakpoint: add a breakpoint type converter 2023-11-14 13:35 ` [PATCH v3 2/3] gdb, breakpoint: add a breakpoint type converter Mohamed Bouhaouel @ 2023-11-14 14:37 ` Tom Tromey 2023-11-15 17:44 ` Bouhaouel, Mohamed 0 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2023-11-14 14:37 UTC (permalink / raw) To: Mohamed Bouhaouel; +Cc: gdb-patches, blarsen, eliz >>>>> Mohamed Bouhaouel <mohamed.bouhaouel@intel.com> writes: > Add a new function 'bptype_to_target_hw_bp_type' to perform > the translation from 'bptype' to 'target_hw_bp_type'. Thank you. > +target_hw_bp_type > +bptype_to_target_hw_bp_type (bptype type) > +{ > + default: > + error (_ ("Bad breakpoint type: bptype %d."), type); Can this ever be reached? If not it should be gdb_assert_not_reached. If so then it should have a better error message -- this one won't make sense to users. > +/* Translate BPTYPE to TARGET_HW_BP_TYPE. */ > + > +extern target_hw_bp_type bptype_to_target_hw_bp_type (bptype type); > #endif /* !defined (BREAKPOINT_H) */ Newline before #endif Tom ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v3 2/3] gdb, breakpoint: add a breakpoint type converter 2023-11-14 14:37 ` Tom Tromey @ 2023-11-15 17:44 ` Bouhaouel, Mohamed 0 siblings, 0 replies; 12+ messages in thread From: Bouhaouel, Mohamed @ 2023-11-15 17:44 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, blarsen, eliz Hi, Thanks for the feedback. > > +target_hw_bp_type > > +bptype_to_target_hw_bp_type (bptype type) > > +{ > > > + default: > > + error (_ ("Bad breakpoint type: bptype %d."), type); > > Can this ever be reached? If not it should be gdb_assert_not_reached. > If so then it should have a better error message -- this one won't make > sense to users. It can be reached by mistake, e.g. if someone passes a wrong btype to the function. I find using "gdb_assert_not_reached" is more suitable in this case. I will change it in the next patch update. > > +extern target_hw_bp_type bptype_to_target_hw_bp_type (bptype type); > > #endif /* !defined (BREAKPOINT_H) */ > > Newline before #endif Will be address in the next update. Regards, Mohamed Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 3/3] gdb, zpoint: check for target hardware breakpoint support 2023-11-14 13:35 [PATCH v3 0/3] Check for zpoint support when handling watchpoints Mohamed Bouhaouel 2023-11-14 13:35 ` [PATCH v3 1/3] gdb, gdbserver, zpoint: report z_point support Mohamed Bouhaouel 2023-11-14 13:35 ` [PATCH v3 2/3] gdb, breakpoint: add a breakpoint type converter Mohamed Bouhaouel @ 2023-11-14 13:35 ` Mohamed Bouhaouel 2023-11-14 14:45 ` Tom Tromey 2 siblings, 1 reply; 12+ messages in thread From: Mohamed Bouhaouel @ 2023-11-14 13:35 UTC (permalink / raw) To: gdb-patches; +Cc: blarsen, mohamed.bouhaouel, eliz In 'can_use_hw_breakpoint', check if the target supports hardware-assisted breakpoints. This will prevent GDB from trying to insert the hardware breakpoint in case it is not supported. Reviewed-By: Guinevere Larsen <blarsen@redhat.com> --- gdb/remote.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 7aa380a5989..10504696a57 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -10982,7 +10982,7 @@ remote_target::remove_breakpoint (struct gdbarch *gdbarch, } static enum Z_packet_type -watchpoint_to_Z_packet (int type) +hw_bp_to_Z_packet (int type) { switch (type) { @@ -10995,6 +10995,9 @@ watchpoint_to_Z_packet (int type) case hw_access: return Z_PACKET_ACCESS_WP; break; + case hw_execute: + return Z_PACKET_HARDWARE_BP; + break; default: internal_error (_("hw_bp_to_z: bad watchpoint type %d"), type); } @@ -11003,7 +11006,7 @@ watchpoint_to_Z_packet (int type) bool remote_target::supports_z_point_type (int type) { - Z_packet_type packet = watchpoint_to_Z_packet (type); + Z_packet_type packet = hw_bp_to_Z_packet (type); return (m_features.packet_support (PACKET_Z0 + packet) != PACKET_DISABLE); } @@ -11014,7 +11017,7 @@ remote_target::insert_watchpoint (CORE_ADDR addr, int len, struct remote_state *rs = get_remote_state (); char *endbuf = rs->buf.data () + get_remote_packet_size (); char *p; - enum Z_packet_type packet = watchpoint_to_Z_packet (type); + enum Z_packet_type packet = hw_bp_to_Z_packet (type); if (m_features.packet_support ((to_underlying (PACKET_Z0) + to_underlying (packet))) == PACKET_DISABLE) @@ -11064,7 +11067,7 @@ remote_target::remove_watchpoint (CORE_ADDR addr, int len, struct remote_state *rs = get_remote_state (); char *endbuf = rs->buf.data () + get_remote_packet_size (); char *p; - enum Z_packet_type packet = watchpoint_to_Z_packet (type); + enum Z_packet_type packet = hw_bp_to_Z_packet (type); if (m_features.packet_support ((to_underlying (PACKET_Z0) + to_underlying (packet))) == PACKET_DISABLE) @@ -11118,6 +11121,11 @@ remote_target::can_use_hw_breakpoint (enum bptype type, int cnt, int ot) { if (type == bp_hardware_breakpoint) { + /* Check if the target supports hardware-assisted breakpoints. + Return 0 if not. */ + if (!supports_z_point_type (bptype_to_target_hw_bp_type (type))) + return 0; + if (remote_hw_breakpoint_limit == 0) return 0; else if (remote_hw_breakpoint_limit < 0) @@ -11127,6 +11135,18 @@ remote_target::can_use_hw_breakpoint (enum bptype type, int cnt, int ot) } else { + /* Check if the target supports the hardware watchpoint type. + Return 0 if not. */ + if (!supports_z_point_type (bptype_to_target_hw_bp_type (type))) + { + /* If hw read watchpoints are not supported while hw access are, + GDB will try to insert the watchpoint as hw access. */ + bool access_support = supports_z_point_type ( + bptype_to_target_hw_bp_type (bp_access_watchpoint)); + if (!(type == bp_read_watchpoint && access_support)) + return 0; + } + if (remote_hw_watchpoint_limit == 0) return 0; else if (remote_hw_watchpoint_limit < 0) -- 2.25.1 Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 3/3] gdb, zpoint: check for target hardware breakpoint support 2023-11-14 13:35 ` [PATCH v3 3/3] gdb, zpoint: check for target hardware breakpoint support Mohamed Bouhaouel @ 2023-11-14 14:45 ` Tom Tromey 2023-11-15 18:18 ` Bouhaouel, Mohamed 0 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2023-11-14 14:45 UTC (permalink / raw) To: Mohamed Bouhaouel; +Cc: gdb-patches, blarsen, eliz >>>>> Mohamed Bouhaouel <mohamed.bouhaouel@intel.com> writes: > In 'can_use_hw_breakpoint', check if the target supports > hardware-assisted breakpoints. This will prevent GDB > from trying to insert the hardware breakpoint in case > it is not supported. Thanks. One question I have is how this is tested. Another oddity is that supposedly a remote can reply '' to a z packet, meaning it isn't supported. However, insert_hw_breakpoint (at least) doesn't seem to record this response. > + /* If hw read watchpoints are not supported while hw access are, > + GDB will try to insert the watchpoint as hw access. */ > + bool access_support = supports_z_point_type ( > + bptype_to_target_hw_bp_type (bp_access_watchpoint)); Formatting looks wrong here. Tom ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v3 3/3] gdb, zpoint: check for target hardware breakpoint support 2023-11-14 14:45 ` Tom Tromey @ 2023-11-15 18:18 ` Bouhaouel, Mohamed 2023-11-17 13:42 ` Tom Tromey 0 siblings, 1 reply; 12+ messages in thread From: Bouhaouel, Mohamed @ 2023-11-15 18:18 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, blarsen, eliz Hi, Thanks Tom for the feedback, > One question I have is how this is tested. We would need a remote target that does not support hardware watchpoints. But I am not sure how can we build such a test. > Another oddity is that supposedly a remote can reply '' to a z packet, > meaning it isn't supported. However, insert_hw_breakpoint (at least) > doesn't seem to record this response. Could you please elaborate? The remote can or can't replay to z packets? > > + /* If hw read watchpoints are not supported while hw access are, > > + GDB will try to insert the watchpoint as hw access. */ > > + bool access_support = supports_z_point_type ( > > + bptype_to_target_hw_bp_type (bp_access_watchpoint)); > > Formatting looks wrong here. Hmm, how else can we format it? One suggestion is to use an additional variable to store the return of " bptype_to_target_hw_bp_type (bp_access_watchpoint)". Thanks, Mohamed Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 3/3] gdb, zpoint: check for target hardware breakpoint support 2023-11-15 18:18 ` Bouhaouel, Mohamed @ 2023-11-17 13:42 ` Tom Tromey 0 siblings, 0 replies; 12+ messages in thread From: Tom Tromey @ 2023-11-17 13:42 UTC (permalink / raw) To: Bouhaouel, Mohamed; +Cc: Tom Tromey, gdb-patches, blarsen, eliz >>>>> Bouhaouel, Mohamed <mohamed.bouhaouel@intel.com> writes: >> > + /* If hw read watchpoints are not supported while hw access are, >> > + GDB will try to insert the watchpoint as hw access. */ >> > + bool access_support = supports_z_point_type ( >> > + bptype_to_target_hw_bp_type (bp_access_watchpoint)); >> >> Formatting looks wrong here. > Hmm, how else can we format it? One suggestion is to use an additional variable > to store the return of " bptype_to_target_hw_bp_type (bp_access_watchpoint)". Yeah, that would work. A dangling open paren isn't really gdb style. Another way would be bool access_support = (supports_z_point_type (bptype_to_target_hw_bp_type (bp_access_watchpoint))); kind of uglier than a new variable but it's done elsewhere. Tom ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-11-17 13:42 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-11-14 13:35 [PATCH v3 0/3] Check for zpoint support when handling watchpoints Mohamed Bouhaouel 2023-11-14 13:35 ` [PATCH v3 1/3] gdb, gdbserver, zpoint: report z_point support Mohamed Bouhaouel 2023-11-14 13:45 ` Eli Zaretskii 2023-11-14 14:36 ` Tom Tromey 2023-11-15 17:31 ` Bouhaouel, Mohamed 2023-11-14 13:35 ` [PATCH v3 2/3] gdb, breakpoint: add a breakpoint type converter Mohamed Bouhaouel 2023-11-14 14:37 ` Tom Tromey 2023-11-15 17:44 ` Bouhaouel, Mohamed 2023-11-14 13:35 ` [PATCH v3 3/3] gdb, zpoint: check for target hardware breakpoint support Mohamed Bouhaouel 2023-11-14 14:45 ` Tom Tromey 2023-11-15 18:18 ` Bouhaouel, Mohamed 2023-11-17 13:42 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).