[PATCH] Support targets that know how to step over breakpoints

Luis Machado
2012-10-04T12:48:19Z

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

Hi,

Most of the targets we deal with need to be told to lift off breakpoints
from the inferior prior to single-stepping. A few targets are able to
just step over those breakpoints without being told so.

In the latter case, GDB assumes the target knows how to manage
breakpoints on its own.

gdbserver does not know how to do this unless we force this mechanism
into it, but, honestly, i don't see the point. Usually targets that know
how to step over breakpoints do so via a more low level interface like
the kernel. Let me know if you think otherwise.

[-- Attachment #2: 0001-remote_step_over_breaks.diff --]
[-- Type: text/x-patch, Size: 12729 bytes --]

2012-10-03 Luis Machado <lgustavo@codesourcery.com>
Pedro Alves <pedro@codesourcery.com>

gdb/
* 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/
* gdb.texinfo (General Query Packets): Document
StepOverBreakpoints under qSupported.

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

Index: gdb_head/gdb/doc/gdb.texinfo
===================================================================
--- gdb_head.orig/gdb/doc/gdb.texinfo 2012-09-26 16:12:17.000000000 -0300
+++ gdb_head/gdb/doc/gdb.texinfo 2012-09-26 16:17:07.801656045 -0300
@@ -36760,6 +36760,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:
@@ -36914,6 +36919,17 @@ See @ref{Bytecode Descriptions} for deta
The remote stub supports running a breakpoint's command list itself,
rather than reporting the hit to @value{GDBN}.

+@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: gdb_head/gdb/doc/gdbint.texinfo
===================================================================
--- gdb_head.orig/gdb/doc/gdbint.texinfo 2012-09-26 16:12:17.000000000 -0300
+++ gdb_head/gdb/doc/gdbint.texinfo 2012-09-26 16:17:07.813656042 -0300
@@ -593,6 +593,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: gdb_head/gdb/infrun.c
===================================================================
--- gdb_head.orig/gdb/infrun.c 2012-09-26 16:12:17.000000000 -0300
+++ gdb_head/gdb/infrun.c 2012-09-26 16:17:07.813656042 -0300
@@ -1756,7 +1756,8 @@ a command like `return' or `jump' to con
We can't use displaced stepping when we are waiting for vfork_done
event, displaced stepping breaks the vfork child similarly as single
step software breakpoint. */
- if (use_displaced_stepping (gdbarch)
+ if (!target_can_step_over_breakpoints ()
+ && use_displaced_stepping (gdbarch)
&& (tp->control.trap_expected
|| (step && gdbarch_software_single_step_p (gdbarch)))
&& sig == GDB_SIGNAL_0
@@ -1882,16 +1883,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->control.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 (aspace, pc))
step = 0;
}

- if (debug_displaced
+ if (!target_can_step_over_breakpoints ()
+ && debug_displaced
&& use_displaced_stepping (gdbarch)
&& tp->control.trap_expected)
{
@@ -2189,19 +2197,22 @@ proceed (CORE_ADDR addr, enum gdb_signal
if (oneproc)
{
tp->control.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->control.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->control.trap_expected
+ || target_can_step_over_breakpoints ()
+ || use_displaced_stepping (gdbarch))
insert_breakpoints ();

if (!non_stop)
@@ -3914,10 +3925,12 @@ handle_inferior_event (struct execution_
singlestep_breakpoints_inserted_p = 0;
}

- /* If the arch can displace step, don't remove the
- breakpoints. */
+ /* If the target can step over breakpoints transparently, or
+ we can use displace stepping with this arch, don't remove
+ the breakpoints. */
thread_regcache = get_thread_regcache (ecs->ptid);
- if (!use_displaced_stepping (get_regcache_arch (thread_regcache)))
+ if (!target_can_step_over_breakpoints ()
+ && !use_displaced_stepping (get_regcache_arch (thread_regcache)))
remove_status = remove_breakpoints ();

/* Did we fail to remove breakpoints? If so, try
@@ -5688,10 +5701,12 @@ keep_going (struct execution_control_sta
{
struct regcache *thread_regcache = get_thread_regcache (ecs->ptid);

- if (!use_displaced_stepping (get_regcache_arch (thread_regcache)))
- /* 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 (get_regcache_arch (thread_regcache)))
+ /* 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: gdb_head/gdb/remote.c
===================================================================
--- gdb_head.orig/gdb/remote.c 2012-09-26 16:12:17.000000000 -0300
+++ gdb_head/gdb/remote.c 2012-09-26 16:17:07.817656039 -0300
@@ -319,6 +319,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;
+
/* True if the stub reports support for conditional tracepoints. */
int cond_tracepoints;

@@ -3879,6 +3883,15 @@ remote_string_tracing_feature (const str
rs->string_tracing = (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,
@@ -3909,6 +3922,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 },
{ "qXfer:siginfo:read", PACKET_DISABLE, remote_supported_packet,
PACKET_qXfer_siginfo_read },
{ "qXfer:siginfo:write", PACKET_DISABLE, remote_supported_packet,
@@ -4236,6 +4251,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;
rs->ctrlc_pending_p = 0;

general_thread = not_sent_ptid;
@@ -11000,6 +11016,13 @@ remote_can_use_agent (void)
return (remote_protocol_packets[PACKET_QAgent].support != PACKET_DISABLE);
}

+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)
{
@@ -11069,6 +11092,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;
remote_ops.to_supports_disable_randomization
= remote_supports_disable_randomization;
remote_ops.to_fileio_open = remote_hostio_open;
Index: gdb_head/gdb/target.c
===================================================================
--- gdb_head.orig/gdb/target.c 2012-09-26 16:12:17.000000000 -0300
+++ gdb_head/gdb/target.c 2012-09-26 16:17:07.821656038 -0300
@@ -673,6 +673,7 @@ update_current_target (void)
INHERIT (to_supports_multi_process, t);
INHERIT (to_supports_enable_disable_tracepoint, t);
INHERIT (to_supports_string_tracing, t);
+ INHERIT (to_can_step_over_breakpoints, t);
INHERIT (to_trace_init, t);
INHERIT (to_download_tracepoint, t);
INHERIT (to_can_download_tracepoint, t);
@@ -930,6 +931,9 @@ update_current_target (void)
de_fault (to_traceframe_info,
(struct traceframe_info * (*) (void))
tcomplain);
+ de_fault (to_can_step_over_breakpoints,
+ (int (*) (void))
+ return_zero);
de_fault (to_supports_evaluation_of_breakpoint_conditions,
(int (*) (void))
return_zero);
Index: gdb_head/gdb/target.h
===================================================================
--- gdb_head.orig/gdb/target.h 2012-09-26 16:12:17.000000000 -0300
+++ gdb_head/gdb/target.h 2012-09-26 16:17:07.825656040 -0300
@@ -857,6 +857,11 @@ struct target_ops
/* Is the target able to use agent in current state? */
int (*to_can_use_agent) (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?
*/
@@ -1011,6 +1016,12 @@ int target_supports_disable_randomizatio
#define target_can_run_breakpoint_commands() \
(*current_target.to_can_run_breakpoint_commands) ()

+/* Return true if GDB does not need to lift breakpoints from 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) ()
+
/* Invalidate all target dcaches. */
extern void target_dcache_invalidate (void);