This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Target can step over breakpoints itself
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Cc: Daniel Jacobowitz <drow at false dot org>
- Date: Mon, 3 Nov 2008 15:44:58 +0000
- Subject: Re: Target can step over breakpoints itself
- References: <200810271419.00304.pedro@codesourcery.com> <20081028141737.GB21659@caradoc.them.org>
On Tuesday 28 October 2008 14:17:37, Daniel Jacobowitz wrote:
> On Mon, Oct 27, 2008 at 02:19:00PM +0000, Pedro Alves wrote:
> > 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?
>
> Does DICOS hit breakpoints at the current PC if you continue, rather
> than step? I'd hope so - and if so, let's document that expectation.
> It's details like that which confuse people trying to implement the
> remote protocol.
Yeah, thanks for the hint. It doesn't hit the breakpoint if proceeding
at the PC where the thread stopped (an equivalent to stop_pc on the
GDB side), but will hit it if telling it to resume at some other PC
with a breakpoint there (e.g., jump *PC). Since GDB normally single-steps
to move past a breakpoint at stop_pc, this hardly matters for DICOS
(other cases where GDB expects to hit a breakpoint at PC happen around
unix signals, but DICOS doesn't have a notion of unix signals).
What I think we should do, is document that the feature only
applies to single-stepping, as attached.
>
> Does this need to be conditional on the type of breakpoint (hardware
> vs software, Z0 vs memory) or on the actual breakpoint? For instance,
> platforms with per-thread hardware breakpoints can 'step over'
> hardware breakpoints by temporarily removing them from just one
> thread; and I believe setting IA64_PSR_DD lets you step or continue
> over a breakpoint.
>
This feature is meant to apply to all breakpoints the stub side
supports. Although it can be seen as an optimization in
all-stop mode, this is an alternative to displaced stepping
on the GDB side, for non-stop mode --- where we need to make sure
running threads wouldn't miss a breakpoint.
Memory breakpoints are off the chart. Having a remote stub
support this feature with memory breakpoints on the GDB side
would be too much of a hack.
How about something like this?
@item StepOverBreakpoints
The remote stub knows how to step over breakpoints itself. If this
feature is supported by the target, then @value{GDBN} does not need to
lift breakpoints off of the inferior to step over them. This feature
only applies to single-step requests. @value{GDBN} assumes that the
target hits breakpoints at the current PC if told to continue, rather
than single-step. This feature is only defined when the remote stub
supports managing breakpoints itself (see @ref{insert breakpoint or
watchpoint packet}). @value{GDBN} assumes that this feature is
applicable to all breakpoints types supported by the stub.
--
Pedro Alves
gdb/
2008-11-03 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-11-03 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 | 16 +++++++++++++
gdb/doc/gdbint.texinfo | 9 +++++++
gdb/infrun.c | 59 ++++++++++++++++++++++++++++++-------------------
gdb/remote.c | 24 +++++++++++++++++++
gdb/target.c | 4 +++
gdb/target.h | 11 +++++++++
6 files changed, 101 insertions(+), 22 deletions(-)
Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo 2008-11-03 13:24:04.000000000 +0000
+++ src/gdb/doc/gdb.texinfo 2008-11-03 15:09:52.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,17 @@ 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 itself. If this
+feature is supported by the target, then @value{GDBN} does not need to
+lift breakpoints off of the inferior to step over them. This feature
+only applies to single-step requests. @value{GDBN} assumes that the
+target hits breakpoints at the current PC if told to continue, rather
+than single-step. This feature is only defined when the remote stub
+supports managing breakpoints itself (see @ref{insert breakpoint or
+watchpoint packet}). @value{GDBN} assumes that this feature is
+applicable to all breakpoints types supported by the stub.
+
@end table
@item qSymbol::
Index: src/gdb/doc/gdbint.texinfo
===================================================================
--- src.orig/gdb/doc/gdbint.texinfo 2008-11-03 13:24:04.000000000 +0000
+++ src/gdb/doc/gdbint.texinfo 2008-11-03 14:12:57.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-11-03 13:24:04.000000000 +0000
+++ src/gdb/target.h 2008-11-03 14:12:57.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-11-03 14:12:48.000000000 +0000
+++ src/gdb/target.c 2008-11-03 14:12:57.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-11-03 13:27:20.000000000 +0000
+++ src/gdb/infrun.c 2008-11-03 14:12:57.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)
{
@@ -1123,16 +1124,23 @@ a command like `return' or `jump' to con
resume_ptid = inferior_ptid;
}
- if (gdbarch_cannot_step_breakpoint (gdbarch))
+ /* Most targets can step a breakpoint instruction, thus
+ executing it normally (hitting the breakpoint). But if this
+ one cannot, just continue and we will hit it anyway. */
+ /* A target that executes the instruction under a breakpoint
+ automatically when told to single-step, by definition also
+ must be told to continue, otherwise the breakpoint won't be
+ hit. */
+ if (gdbarch_cannot_step_breakpoint (gdbarch)
+ || (target_can_step_over_breakpoints ()
+ && !tp->trap_expected))
{
- /* Most targets can step a breakpoint instruction, thus
- executing it normally. But if this one cannot, just
- continue and we will hit it anyway. */
if (step && breakpoint_inserted_here_p (pc))
step = 0;
}
- if (debug_displaced
+ if (!target_can_step_over_breakpoints ()
+ && debug_displaced
&& use_displaced_stepping (gdbarch)
&& tp->trap_expected)
{
@@ -1329,19 +1337,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 +2626,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 +3913,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-11-03 14:12:48.000000000 +0000
+++ src/gdb/remote.c 2008-11-03 14:12:57.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. */
@@ -2932,6 +2936,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,
@@ -2952,6 +2965,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
@@ -3185,6 +3200,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;
@@ -8566,6 +8582,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)
{
@@ -8629,6 +8652,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