[binutils-gdb] remote+docs: software/hardware breakpoint traps

[Pedro Alves <palves at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f7e6eed5283bb5c8a3598dd986dc922b9a794f58

commit f7e6eed5283bb5c8a3598dd986dc922b9a794f58
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Mar 4 20:41:16 2015 +0000

    remote+docs: software/hardware breakpoint traps
    
    This adjusts target remote to tell the core whether a trap was caused
    by a breakpoint.
    
    To that end, the patch teaches GDB about new RSP stop reasons "T05
    swbreak" and "T05 hwbreak", that remote targets report back to GDB,
    similarly to how "T05 watch" indicates a stop caused by a watchpoint.
    
    Because targets that can report these events are expected to
    themselves adjust the PC after a software breakpoint, these new stop
    reasons must only be reported if the stub is talking to a GDB that
    understands them.  Because of that, the use of the new stop reasons
    needs to be handshaked on initial connection, using the qSupported
    mechanism.  GDB simply sends "swbreak+" in its qSupports query, and
    the stub reports back "swbreak+" too.
    
    Because these new stop reasons are required to fix a fundamental
    non-stop mode problem, this commit extends the remote non-stop intro
    section in the manual, documenting the events as required.
    
    To be clear, GDB will still cope with remote targets that don't
    support these new stop reasons; it will behave just like today.
    
    Tested on x86-64 Fedora 20, native and gdbserver.
    
    gdb/ChangeLog:
    2015-03-04  Pedro Alves  <palves@redhat.com>
    
    	* NEWS: Mention the new "swbreak" and "hwbreak" stop reasons.
    	* remote.c (struct remote_state) <remote_stopped_by_watchpoint_p>:
    	Delete field.
    	<stop_reason>: New field.
    	(PACKET_swbreak_feature, PACKET_hwbreak_feature): New enum values.
    	(packet_set_cmd_state): New function.
    	(remote_protocol_features): Register the "swbreak" and "hwbreak"
    	features.
    	(remote_query_supported): If not disabled with the corresponding
    	"set remote foo-packet" command, report support for the swbreak
    	and hwbreak features.
    	(struct stop_reply) <remote_stopped_by_watchpoint_p>: Delete
    	field.
    	<stop_reason>: New field.
    	(remote_parse_stop_reply): Handle "swbreak" and "hwbreak".
    	(remote_wait_as): Adjust.
    	(remote_stopped_by_sw_breakpoint)
    	(remote_supports_stopped_by_sw_breakpoint)
    	(remote_stopped_by_hw_breakpoint)
    	(remote_supports_stopped_by_hw_breakpoint): New functions.
    	(remote_stopped_by_watchpoint): New function.
    	(init_remote_ops): Install them.
    	(_initialize_remote): Register new "set/show remote
    	swbreak-feature-packet" and "set/show remote
    	swbreak-feature-packet" commands.
    
    gdb/doc/ChangeLog:
    2015-03-04  Pedro Alves  <palves@redhat.com>
    
    	* gdb.texinfo (Remote Configuration): Document the "set/show
    	remote swbreak-feature-packet" and "set/show remote
    	hwbreak-feature-packet" commands.
    	(Packets) <Z0>: Add cross link to the "swbreak" stop reason's
    	decription.
    	(Stop Reply Packets): Document the swbreak and hwbreak stop
    	reasons.
    	(General Query Packets): Document the swbreak and hwbreak
    	qSupported features.
    	(Remote Non-Stop): Explain that swbreak and hwbreak are required.

Diff:
---
 gdb/ChangeLog       |  28 +++++++++++++
 gdb/NEWS            |  10 +++++
 gdb/doc/ChangeLog   |  13 ++++++
 gdb/doc/gdb.texinfo |  80 +++++++++++++++++++++++++++++++++++
 gdb/remote.c        | 119 ++++++++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 241 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 11f6caa..22bda6a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,33 @@
 2015-03-04  Pedro Alves  <palves@redhat.com>
 
+	* NEWS: Mention the new "swbreak" and "hwbreak" stop reasons.
+	* remote.c (struct remote_state) <remote_stopped_by_watchpoint_p>:
+	Delete field.
+	<stop_reason>: New field.
+	(PACKET_swbreak_feature, PACKET_hwbreak_feature): New enum values.
+	(packet_set_cmd_state): New function.
+	(remote_protocol_features): Register the "swbreak" and "hwbreak"
+	features.
+	(remote_query_supported): If not disabled with the corresponding
+	"set remote foo-packet" command, report support for the swbreak
+	and hwbreak features.
+	(struct stop_reply) <remote_stopped_by_watchpoint_p>: Delete
+	field.
+	<stop_reason>: New field.
+	(remote_parse_stop_reply): Handle "swbreak" and "hwbreak".
+	(remote_wait_as): Adjust.
+	(remote_stopped_by_sw_breakpoint)
+	(remote_supports_stopped_by_sw_breakpoint)
+	(remote_stopped_by_hw_breakpoint)
+	(remote_supports_stopped_by_hw_breakpoint): New functions.
+	(remote_stopped_by_watchpoint): New function.
+	(init_remote_ops): Install them.
+	(_initialize_remote): Register new "set/show remote
+	swbreak-feature-packet" and "set/show remote
+	swbreak-feature-packet" commands.
+
+2015-03-04  Pedro Alves  <palves@redhat.com>
+
 	* btrace.h: Include target/waitstatus.h.
 	(struct btrace_thread_info) <stop_reason>: New field.
 	* record-btrace.c (record_btrace_step_thread): Use
diff --git a/gdb/NEWS b/gdb/NEWS
index 1c0f079..49dc0e6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -63,6 +63,16 @@ qXfer:btrace-conf:read
 Qbtrace-conf:bts:size
   Set the requested ring buffer size for branch tracing in BTS format.
 
+swbreak stop reason
+  Indicates a memory breakpoint instruction was executed, irrespective
+  of whether it was GDB that planted the breakpoint or the breakpoint
+  is hardcoded in the program.  This is required for correct non-stop
+  mode operation.
+
+hwbreak stop reason
+  Indicates the target stopped for a hardware breakpoint.  This is
+  required for correct non-stop mode operation.
+
 * The info record command now shows the recording format and the
   branch tracing configuration for the current thread when using
   the btrace record target.
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 2b2ad5a..f62b4ac6 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,16 @@
+2015-03-04  Pedro Alves  <palves@redhat.com>
+
+	* gdb.texinfo (Remote Configuration): Document the "set/show
+	remote swbreak-feature-packet" and "set/show remote
+	hwbreak-feature-packet" commands.
+	(Packets) <Z0>: Add cross link to the "swbreak" stop reason's
+	decription.
+	(Stop Reply Packets): Document the swbreak and hwbreak stop
+	reasons.
+	(General Query Packets): Document the swbreak and hwbreak
+	qSupported features.
+	(Remote Non-Stop): Explain that swbreak and hwbreak are required.
+
 2015-03-03  Philippe Proulx  eeppeliteloop@gmail.com
 
 	* gdb.texinfo (gdb/mi Async Records): Fix duplicate syscall-entry
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c0a8244..4b76ce9 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -19783,6 +19783,15 @@ are:
 @item @code{conditional-breakpoints-packet}
 @tab @code{Z0 and Z1}
 @tab @code{Support for target-side breakpoint condition evaluation}
+
+@item @code{swbreak-feature}
+@tab @code{swbreak stop reason}
+@tab @code{break}
+
+@item @code{hwbreak-feature}
+@tab @code{hwbreak stop reason}
+@tab @code{hbreak}
+
 @end multitable
 
 @node Remote Stub
@@ -34985,6 +34994,9 @@ form that should be evaluated on the target's side.  These are the
 conditions that should be taken into consideration when deciding if
 the breakpoint trigger should be reported back to @var{GDBN}.
 
+See also the @samp{swbreak} stop reason (@pxref{swbreak stop reason})
+for how to best report a memory breakpoint event to @value{GDBN}.
+
 The @var{cond_list} parameter is comprised of a series of expressions,
 concatenated without separators. Each expression has the following form:
 
@@ -35188,6 +35200,34 @@ logged execution events, because it has reached the end (or the
 beginning when executing backward) of the log.  The value of @var{r}
 will be either @samp{begin} or @samp{end}.  @xref{Reverse Execution}, 
 for more information.
+
+@item swbreak
+@anchor{swbreak stop reason}
+The packet indicates a memory breakpoint instruction was executed,
+irrespective of whether it was @value{GDBN} that planted the
+breakpoint or the breakpoint is hardcoded in the program.  The @var{r}
+part must be left empty.
+
+On some architectures, such as x86, at the architecture level, when a
+breakpoint instruction executes the program counter points at the
+breakpoint address plus an offset.  On such targets, the stub is
+responsible for adjusting the PC to point back at the breakpoint
+address.
+
+This packet should not be sent by default; older @value{GDBN} versions
+did not support it.  @value{GDBN} requests it, by supplying an
+appropriate @samp{qSupported} feature (@pxref{qSupported}).  The
+remote stub must also supply the appropriate @samp{qSupported} feature
+indicating support.
+
+This packet is required for correct non-stop mode operation.
+
+@item hwbreak
+The packet indicates the target stopped for a hardware breakpoint.
+The @var{r} part must be left empty.
+
+The same remarks about @samp{qSupported} and non-stop mode above
+apply.
 @end table
 
 @item W @var{AA}
@@ -35772,6 +35812,14 @@ description.
 This feature indicates whether @value{GDBN} supports the
 @samp{qRelocInsn} packet (@pxref{Tracepoint Packets,,Relocate
 instruction reply packet}).
+
+@item swbreak
+This feature indicates whether @value{GDBN} supports the swbreak stop
+reason in stop replies.  @xref{swbreak stop reason}, for details.
+
+@item hwbreak
+This feature indicates whether @value{GDBN} supports the hwbreak stop
+reason in stop replies.  @xref{swbreak stop reason}, for details.
 @end table
 
 Stubs should ignore any unknown values for
@@ -36000,6 +36048,16 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{swbreak}
+@tab No
+@tab @samp{-}
+@tab No
+
+@item @samp{hwbreak}
+@tab No
+@tab @samp{-}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -36180,6 +36238,14 @@ The remote stub understands the @samp{Qbtrace:bts} packet.
 @item Qbtrace-conf:bts:size
 The remote stub understands the @samp{Qbtrace-conf:bts:size} packet.
 
+@item swbreak
+The remote stub reports the @samp{swbreak} stop reason for memory
+breakpoints.
+
+@item hwbreak
+The remote stub reports the @samp{hwbreak} stop reason for hardware
+breakpoints.
+
 @end table
 
 @item qSymbol::
@@ -37565,6 +37631,20 @@ If all threads are running when the target receives the @samp{?} packet,
 or if the target is not attached to any process, it shall respond
 @samp{OK}.
 
+If the stub supports non-stop mode, it should also support the
+@samp{swbreak} stop reason if software breakpoints are supported, and
+the @samp{hwbreak} stop reason if hardware breakpoints are supported
+(@pxref{swbreak stop reason}).  This is because given the asynchronous
+nature of non-stop mode, between the time a thread hits a breakpoint
+and the time the event is finally processed by @value{GDBN}, the
+breakpoint may have already been removed from the target.  Due to
+this, @value{GDBN} needs to be able to tell whether a trap stop was
+caused by a delayed breakpoint event, which should be ignored, as
+opposed to a random trap signal, which should be reported to the user.
+Note the @samp{swbreak} feature implies that the target is responsible
+for adjusting the PC when a software breakpoint triggers, if
+necessary, such as on the x86 architecture.
+
 @node Packet Acknowledgment
 @section Packet Acknowledgment
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 2a823eb..8f783a4 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -364,8 +364,8 @@ struct remote_state
      to stop for a watchpoint.  */
   CORE_ADDR remote_watch_data_address;
 
-  /* This is non-zero if target stopped for a watchpoint.  */
-  int remote_stopped_by_watchpoint_p;
+  /* Whether the target stopped for a breakpoint/watchpoint.  */
+  enum target_stop_reason stop_reason;
 
   threadref echo_nextthread;
   threadref nextthread;
@@ -1338,11 +1338,26 @@ enum {
   /* Support for the Qbtrace-conf:bts:size packet.  */
   PACKET_Qbtrace_conf_bts_size,
 
+  /* Support for swbreak+ feature.  */
+  PACKET_swbreak_feature,
+
+  /* Support for hwbreak+ feature.  */
+  PACKET_hwbreak_feature,
+
   PACKET_MAX
 };
 
 static struct packet_config remote_protocol_packets[PACKET_MAX];
 
+/* Returns the packet's corresponding "set remote foo-packet" command
+   state.  See struct packet_config for more details.  */
+
+static enum auto_boolean
+packet_set_cmd_state (int packet)
+{
+  return remote_protocol_packets[packet].detect;
+}
+
 /* Returns whether a given packet or feature is supported.  This takes
    into account the state of the corresponding "set remote foo-packet"
    command, which may be used to bypass auto-detection.  */
@@ -4025,7 +4040,9 @@ static const struct protocol_feature remote_protocol_features[] = {
   { "qXfer:btrace-conf:read", PACKET_DISABLE, remote_supported_packet,
     PACKET_qXfer_btrace_conf },
   { "Qbtrace-conf:bts:size", PACKET_DISABLE, remote_supported_packet,
-    PACKET_Qbtrace_conf_bts_size }
+    PACKET_Qbtrace_conf_bts_size },
+  { "swbreak", PACKET_DISABLE, remote_supported_packet, PACKET_swbreak_feature },
+  { "hwbreak", PACKET_DISABLE, remote_supported_packet, PACKET_hwbreak_feature }
 };
 
 static char *remote_support_xml;
@@ -4094,6 +4111,11 @@ remote_query_supported (void)
 
       q = remote_query_supported_append (q, "multiprocess+");
 
+      if (packet_set_cmd_state (PACKET_swbreak_feature) != AUTO_BOOLEAN_FALSE)
+	q = remote_query_supported_append (q, "swbreak+");
+      if (packet_set_cmd_state (PACKET_hwbreak_feature) != AUTO_BOOLEAN_FALSE)
+	q = remote_query_supported_append (q, "hwbreak+");
+
       if (remote_support_xml)
 	q = remote_query_supported_append (q, remote_support_xml);
 
@@ -5202,7 +5224,8 @@ typedef struct stop_reply
      fetch them is avoided).  */
   VEC(cached_reg_t) *regcache;
 
-  int stopped_by_watchpoint_p;
+  enum target_stop_reason stop_reason;
+
   CORE_ADDR watch_data_address;
 
   int core;
@@ -5513,7 +5536,7 @@ remote_parse_stop_reply (char *buf, struct stop_reply *event)
   event->rs = get_remote_state ();
   event->ws.kind = TARGET_WAITKIND_IGNORE;
   event->ws.value.integer = 0;
-  event->stopped_by_watchpoint_p = 0;
+  event->stop_reason = TARGET_STOPPED_BY_NO_REASON;
   event->regcache = NULL;
   event->core = -1;
 
@@ -5556,10 +5579,36 @@ Packet: '%s'\n"),
 		   || (strncmp (p, "rwatch", p1 - p) == 0)
 		   || (strncmp (p, "awatch", p1 - p) == 0))
 	    {
-	      event->stopped_by_watchpoint_p = 1;
+	      event->stop_reason = TARGET_STOPPED_BY_WATCHPOINT;
 	      p = unpack_varlen_hex (++p1, &addr);
 	      event->watch_data_address = (CORE_ADDR) addr;
 	    }
+	  else if (strncmp (p, "swbreak", p1 - p) == 0)
+	    {
+	      event->stop_reason = TARGET_STOPPED_BY_SW_BREAKPOINT;
+
+	      /* Make sure the stub doesn't forget to indicate support
+		 with qSupported.  */
+	      if (packet_support (PACKET_swbreak_feature) != PACKET_ENABLE)
+		error (_("Unexpected swbreak stop reason"));
+
+	      /* The value part is documented as "must be empty",
+		 though we ignore it, in case we ever decide to make
+		 use of it in a backward compatible way.  */
+	      p = skip_to_semicolon (p1 + 1);
+	    }
+	  else if (strncmp (p, "hwbreak", p1 - p) == 0)
+	    {
+	      event->stop_reason = TARGET_STOPPED_BY_HW_BREAKPOINT;
+
+	      /* Make sure the stub doesn't forget to indicate support
+		 with qSupported.  */
+	      if (packet_support (PACKET_hwbreak_feature) != PACKET_ENABLE)
+		error (_("Unexpected hwbreak stop reason"));
+
+	      /* See above.  */
+	      p = skip_to_semicolon (p1 + 1);
+	    }
 	  else if (strncmp (p, "library", p1 - p) == 0)
 	    {
 	      event->ws.kind = TARGET_WAITKIND_LOADED;
@@ -5817,7 +5866,7 @@ process_stop_reply (struct stop_reply *stop_reply,
 	  VEC_free (cached_reg_t, stop_reply->regcache);
 	}
 
-      rs->remote_stopped_by_watchpoint_p = stop_reply->stopped_by_watchpoint_p;
+      rs->stop_reason = stop_reply->stop_reason;
       rs->remote_watch_data_address = stop_reply->watch_data_address;
 
       remote_notice_new_inferior (ptid, 0);
@@ -5944,7 +5993,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
 
   buf = rs->buf;
 
-  rs->remote_stopped_by_watchpoint_p = 0;
+  rs->stop_reason = TARGET_STOPPED_BY_NO_REASON;
 
   /* We got something.  */
   rs->waiting_for_stop_reply = 0;
@@ -8421,12 +8470,54 @@ remote_check_watch_resources (struct target_ops *self,
   return -1;
 }
 
+/* The to_stopped_by_sw_breakpoint method of target remote.  */
+
+static int
+remote_stopped_by_sw_breakpoint (struct target_ops *ops)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  return rs->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT;
+}
+
+/* The to_supports_stopped_by_sw_breakpoint method of target
+   remote.  */
+
+static int
+remote_supports_stopped_by_sw_breakpoint (struct target_ops *ops)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  return (packet_support (PACKET_swbreak_feature) == PACKET_ENABLE);
+}
+
+/* The to_stopped_by_hw_breakpoint method of target remote.  */
+
+static int
+remote_stopped_by_hw_breakpoint (struct target_ops *ops)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  return rs->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT;
+}
+
+/* The to_supports_stopped_by_hw_breakpoint method of target
+   remote.  */
+
+static int
+remote_supports_stopped_by_hw_breakpoint (struct target_ops *ops)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  return (packet_support (PACKET_hwbreak_feature) == PACKET_ENABLE);
+}
+
 static int
 remote_stopped_by_watchpoint (struct target_ops *ops)
 {
   struct remote_state *rs = get_remote_state ();
 
-  return rs->remote_stopped_by_watchpoint_p;
+  return rs->stop_reason == TARGET_STOPPED_BY_WATCHPOINT;
 }
 
 static int
@@ -11617,6 +11708,10 @@ Specify the serial device it is connected to\n\
   remote_ops.to_files_info = remote_files_info;
   remote_ops.to_insert_breakpoint = remote_insert_breakpoint;
   remote_ops.to_remove_breakpoint = remote_remove_breakpoint;
+  remote_ops.to_stopped_by_sw_breakpoint = remote_stopped_by_sw_breakpoint;
+  remote_ops.to_supports_stopped_by_sw_breakpoint = remote_supports_stopped_by_sw_breakpoint;
+  remote_ops.to_stopped_by_hw_breakpoint = remote_stopped_by_hw_breakpoint;
+  remote_ops.to_supports_stopped_by_hw_breakpoint = remote_supports_stopped_by_hw_breakpoint;
   remote_ops.to_stopped_by_watchpoint = remote_stopped_by_watchpoint;
   remote_ops.to_stopped_data_address = remote_stopped_data_address;
   remote_ops.to_watchpoint_addr_within_range =
@@ -12322,6 +12417,12 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_Qbtrace_conf_bts_size],
        "Qbtrace-conf:bts:size", "btrace-conf-bts-size", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_swbreak_feature],
+                         "swbreak-feature", "swbreak-feature", 0);
+
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_hwbreak_feature],
+                         "hwbreak-feature", "hwbreak-feature", 0);
+
   /* Assert that we've registered commands for all packet configs.  */
   {
     int i;