[1/3] gdb, gdbserver, zpoint: report z_point support

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

Commit Message

Bouhaouel, Mohamed March 28, 2023, 11:54 a.m. UTC
  Report if the target supports Z-points when exchanging features
between GDB and gdbserver.

Signed-off-by: Mohamed Bouhaouel <mohamed.bouhaouel@intel.com>
---
 gdb/remote.c           | 15 +++++++++++++++
 gdbserver/mem-break.cc |  2 +-
 gdbserver/mem-break.h  |  4 ++++
 gdbserver/server.cc    |  7 +++++++
 4 files changed, 27 insertions(+), 1 deletion(-)
  

Comments

Guinevere Larsen Nov. 7, 2023, 10:41 a.m. UTC | #1
On 28/03/2023 13:54, Mohamed Bouhaouel via Gdb-patches wrote:
> Report if the target supports Z-points when exchanging features
> between GDB and gdbserver.
>
> Signed-off-by: Mohamed Bouhaouel <mohamed.bouhaouel@intel.com>

Hi!

Thanks for working on this, and sorry for the long time to get a review.

The code changes look good, but the commit message could use some work. 
Calling them "Z-points" threw me for a loop that google couldn't help. 
Maybe rewording to something like:

Make GDB and 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)

Also, I think this would warrant a NEWS entry and an update to the 
remote protocol documentation.
  

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index 526df313ea7..1139ce3ddf7 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -755,6 +755,9 @@  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 is supported, FALSE otherwise.  */
+  bool supports_z_point_type (int);
+
   int remove_watchpoint (CORE_ADDR, int, enum target_hw_bp_type,
 			 struct expression *) override;
 
@@ -5604,6 +5607,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;
@@ -10843,6 +10851,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 c669842228d..20434f7c9b4 100644
--- a/gdbserver/mem-break.cc
+++ b/gdbserver/mem-break.cc
@@ -987,7 +987,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..458ba0e6be2 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 949849b63a2..e758cd0d509 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -2480,6 +2480,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 ();