This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Target can step over breakpoints itself


When adding non-stop debugging support to a target/arch, one of the
most important aspects we need to take care of, is to have a means
to step over breakpoints without removing them from the inferior,
otherwise, other running threads may miss them.  We've added a mechanism
to GDB that does out-of-line single-stepping (displaced stepping), which
we use on linux x86 and ppc.  The smarts to do this are all on the GDB side.

DICOS debug API can take care of magically stepping over software
breakpoints transparently, which means that we don't have to resort to
do displaced stepping on the GDB side.

The attached patch adds a new "StepOverBreakpoints" feature to the
remote protocol to the remote stub can tell GDB that the debug api has
that feature, and exposes that knowledge to the rest of GDB by
adding a new target_ops method (target_can_step_over_breakpoints),
and teaching infrun.c to it doesn't need to do the traditional hold-and-step
dance (remove breakpoints, single-step, insert breakpoints) from the
inferior(s), if the target reports support for stepping over them
itself.

Any objections/thoughts on this?

-- 
Pedro Alves
gdb/
2008-10-27  Pedro Alves  <pedro@codesourcery.com>

	* infrun.c (resume, proceed, keep_going, handle_inferior_event):
	Check if target can step over breakpoints.

	* remote.c (struct remote_state): New member
	can_step_over_breakpoints.
	(remote_step_over_breakpoints_feature): New.
	(remote_protocol_features): Add StepOverBreakpoints feature.
	(remote_open_1): Clear can_step_over_breakpoints.
	(remote_can_step_over_breakpoints): New.
	(init_remote_ops): Set remote_ops.to_can_step_over_breakpoints to
	remote_can_step_over_breakpoints.

	* target.c (update_current_target): Inherit
	to_can_step_over_breakpoints, and default it to return 0.

	* target.h (struct target_ops): Add new
	to_can_step_over_breakpoints member.
	(target_can_step_over_breakpoints): New.

gdb/doc/
2008-10-27  Pedro Alves  <pedro@codesourcery.com>

	* gdb.texinfo (General Query Packets): Document
	StepOverBreakpoints under qSupported.

	* gdbint.texinfo (Single Stepping): Document
	target_can_step_over_breakpoints.

---
 gdb/doc/gdb.texinfo    |    9 +++++++++
 gdb/doc/gdbint.texinfo |    9 +++++++++
 gdb/infrun.c           |   45 +++++++++++++++++++++++++++------------------
 gdb/remote.c           |   24 ++++++++++++++++++++++++
 gdb/target.c           |    4 ++++
 gdb/target.h           |   11 +++++++++++
 6 files changed, 84 insertions(+), 18 deletions(-)

Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo	2008-10-27 11:52:05.000000000 +0000
+++ src/gdb/doc/gdb.texinfo	2008-10-27 13:21:21.000000000 +0000
@@ -26118,6 +26118,11 @@ These are the currently defined stub fea
 @tab @samp{-}
 @tab No
 
+@item @samp{StepOverBreakpoints}
+@tab No
+@tab @samp{-}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -26183,6 +26188,10 @@ debugging of more than one process at a 
 multiprocess extensions in packet replies unless @value{GDBN} has also
 indicated it supports them in its @samp{qSupported} request.
 
+@item StepOverBreakpoints
+The remote stub knows how to step over breakpoints, @value{GDBN} does
+not need to lift breakpoints off of the inferior to step over them.
+
 @end table
 
 @item qSymbol::
Index: src/gdb/doc/gdbint.texinfo
===================================================================
--- src.orig/gdb/doc/gdbint.texinfo	2008-10-27 12:43:17.000000000 +0000
+++ src/gdb/doc/gdbint.texinfo	2008-10-27 13:21:21.000000000 +0000
@@ -554,6 +554,15 @@ but @code{placed_size} may be.
 
 @section Single Stepping
 
+@table @code
+@cindex stepping over breakpoints
+@findex target_can_step_over_breakpoints
+@item target_can_step_over_breakpoints
+Returns true if the target handles stepping over breakpoints
+transparently, hence @value{GDBN} does not need to lift breakpoints
+off the inferior when single-stepping over a breakpoint.
+@end table
+
 @section Signal Handling
 
 @section Thread Handling
Index: src/gdb/target.h
===================================================================
--- src.orig/gdb/target.h	2008-10-27 13:16:42.000000000 +0000
+++ src/gdb/target.h	2008-10-27 13:21:21.000000000 +0000
@@ -540,6 +540,11 @@ struct target_ops
        simultaneously?  */
     int (*to_supports_multi_process) (void);
 
+    /* Return true if GDB does not need to lift breakpoints from the
+       the inferior while stepping over a breakpoint; the target
+       handles that transparently.  */
+    int (*to_can_step_over_breakpoints) (void);
+
     int to_magic;
     /* Need sub-structure for target machine related rather than comm related?
      */
@@ -657,6 +662,12 @@ extern void target_resume (ptid_t ptid, 
 #define	target_supports_multi_process()	\
      (*current_target.to_supports_multi_process) ()
 
+/* Return true if GDB does not need to lift breakpoints from the the
+   inferior while stepping over a breakpoint; the target handles that
+   transparently.  */
+#define target_can_step_over_breakpoints() \
+     (*current_target.to_can_step_over_breakpoints) ()
+
 extern DCACHE *target_dcache;
 
 extern int target_read_string (CORE_ADDR, char **, int, int *);
Index: src/gdb/target.c
===================================================================
--- src.orig/gdb/target.c	2008-10-27 13:16:42.000000000 +0000
+++ src/gdb/target.c	2008-10-27 13:22:51.000000000 +0000
@@ -472,6 +472,7 @@ update_current_target (void)
       INHERIT (to_get_ada_task_ptid, t);
       /* Do not inherit to_search_memory.  */
       INHERIT (to_supports_multi_process, t);
+      INHERIT (to_can_step_over_breakpoints, t);
       INHERIT (to_magic, t);
       /* Do not inherit to_memory_map.  */
       /* Do not inherit to_flash_erase.  */
@@ -642,6 +643,9 @@ update_current_target (void)
   de_fault (to_supports_multi_process,
 	    (int (*) (void))
 	    return_zero);
+  de_fault (to_can_step_over_breakpoints,
+	    (int (*) (void))
+	    return_zero);
 #undef de_fault
 
   /* Finally, position the target-stack beneath the squashed
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-10-27 11:52:04.000000000 +0000
+++ src/gdb/infrun.c	2008-10-27 14:05:54.000000000 +0000
@@ -1009,7 +1009,8 @@ a command like `return' or `jump' to con
      the comments for displaced_step_prepare explain why.  The
      comments in the handle_inferior event for dealing with 'random
      signals' explain what we do instead.  */
-  if (use_displaced_stepping (gdbarch)
+  if (!target_can_step_over_breakpoints ()
+      && use_displaced_stepping (gdbarch)
       && tp->trap_expected
       && sig == TARGET_SIGNAL_0)
     {
@@ -1132,7 +1133,8 @@ a command like `return' or `jump' to con
 	    step = 0;
 	}
 
-      if (debug_displaced
+      if (!target_can_step_over_breakpoints ()
+	  && debug_displaced
           && use_displaced_stepping (gdbarch)
           && tp->trap_expected)
         {
@@ -1329,19 +1331,22 @@ proceed (CORE_ADDR addr, enum target_sig
   if (oneproc)
     {
       tp->trap_expected = 1;
-      /* If displaced stepping is enabled, we can step over the
-	 breakpoint without hitting it, so leave all breakpoints
-	 inserted.  Otherwise we need to disable all breakpoints, step
-	 one instruction, and then re-add them when that step is
-	 finished.  */
-      if (!use_displaced_stepping (gdbarch))
+      /* If displaced stepping is enabled or the target can step over
+	 breakpoints, we can step over the breakpoint without hitting
+	 it, so leave all breakpoints inserted.  Otherwise we need to
+	 disable all breakpoints, step one instruction, and then
+	 re-add them when that step is finished.  */
+      if (!target_can_step_over_breakpoints ()
+	  && !use_displaced_stepping (gdbarch))
 	remove_breakpoints ();
     }
 
   /* We can insert breakpoints if we're not trying to step over one,
-     or if we are stepping over one but we're using displaced stepping
-     to do so.  */
-  if (! tp->trap_expected || use_displaced_stepping (gdbarch))
+     or if we are stepping over one but the target can step over them
+     transparently or we're using displaced stepping to do so.  */
+  if (! tp->trap_expected
+      || target_can_step_over_breakpoints ()
+      || use_displaced_stepping (gdbarch))
     insert_breakpoints ();
 
   if (!non_stop)
@@ -2615,9 +2620,11 @@ targets should add new threads to the th
 	      singlestep_breakpoints_inserted_p = 0;
 	    }
 
-	  /* If the arch can displace step, don't remove the
-	     breakpoints.  */
-	  if (!use_displaced_stepping (current_gdbarch))
+	  /* If the target can step over breakpoints transparently, or
+	     we can use displace stepping with this arch, don't remove
+	     the breakpoints.  */
+	  if (!target_can_step_over_breakpoints ()
+	      && !use_displaced_stepping (current_gdbarch))
 	    remove_status = remove_breakpoints ();
 
 	  /* Did we fail to remove breakpoints?  If so, try
@@ -3900,10 +3907,12 @@ keep_going (struct execution_control_sta
       
       if (ecs->event_thread->stepping_over_breakpoint)
 	{
-	  if (! use_displaced_stepping (current_gdbarch))
-	    /* Since we can't do a displaced step, we have to remove
-	       the breakpoint while we step it.  To keep things
-	       simple, we remove them all.  */
+	  if (!target_can_step_over_breakpoints ()
+	      && !use_displaced_stepping (current_gdbarch))
+	    /* Since neither the target can step over breakpoints
+	       transparently, neither can we do a displaced step, we
+	       have to remove the breakpoint while we step it.  To
+	       keep things simple, we remove them all.  */
 	    remove_breakpoints ();
 	}
       else
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2008-10-27 13:16:42.000000000 +0000
+++ src/gdb/remote.c	2008-10-27 13:21:49.000000000 +0000
@@ -301,6 +301,10 @@ struct remote_state
 
   /* True if the stub reports support for vCont;t.  */
   int support_vCont_t;
+
+  /* True if the stub reports support for stepping over breakpoints
+     transparently.  */
+  int can_step_over_breakpoints;
 };
 
 /* Returns true if the multi-process extensions are in effect.  */
@@ -2921,6 +2925,15 @@ remote_non_stop_feature (const struct pr
   rs->non_stop_aware = (support == PACKET_ENABLE);
 }
 
+static void
+remote_step_over_breakpoints_feature (const struct protocol_feature *feature,
+				      enum packet_support support,
+				      const char *value)
+{
+  struct remote_state *rs = get_remote_state ();
+  rs->can_step_over_breakpoints = (support == PACKET_ENABLE);
+}
+
 static struct protocol_feature remote_protocol_features[] = {
   { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
   { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
@@ -2941,6 +2954,8 @@ static struct protocol_feature remote_pr
     PACKET_QStartNoAckMode },
   { "multiprocess", PACKET_DISABLE, remote_multi_process_feature, -1 },
   { "QNonStop", PACKET_DISABLE, remote_non_stop_feature, -1 },
+  { "StepOverBreakpoints", PACKET_DISABLE,
+    remote_step_over_breakpoints_feature, -1 },
 };
 
 static void
@@ -3174,6 +3189,7 @@ remote_open_1 (char *name, int from_tty,
   rs->extended = extended_p;
   rs->non_stop_aware = 0;
   rs->waiting_for_stop_reply = 0;
+  rs->can_step_over_breakpoints = 0;
 
   general_thread = not_sent_ptid;
   continue_thread = not_sent_ptid;
@@ -8561,6 +8577,13 @@ remote_supports_multi_process (void)
   return remote_multi_process_p (rs);
 }
 
+static int
+remote_can_step_over_breakpoints (void)
+{
+  struct remote_state *rs = get_remote_state ();
+  return rs->can_step_over_breakpoints;
+}
+
 static void
 init_remote_ops (void)
 {
@@ -8624,6 +8647,7 @@ Specify the serial device it is connecte
   remote_ops.to_terminal_ours = remote_terminal_ours;
   remote_ops.to_supports_non_stop = remote_supports_non_stop;
   remote_ops.to_supports_multi_process = remote_supports_multi_process;
+  remote_ops.to_can_step_over_breakpoints = remote_can_step_over_breakpoints;
 }
 
 /* Set up the extended remote vector by making a copy of the standard

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]