From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16139 invoked by alias); 27 Oct 2008 14:19:23 -0000 Received: (qmail 16126 invoked by uid 22791); 27 Oct 2008 14:19:21 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 27 Oct 2008 14:18:46 +0000 Received: (qmail 30195 invoked from network); 27 Oct 2008 14:18:43 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 27 Oct 2008 14:18:43 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Target can step over breakpoints itself Date: Mon, 27 Oct 2008 14:19:00 -0000 User-Agent: KMail/1.9.9 MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_U3cBJl6yrO12G4D" Message-Id: <200810271419.00304.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-10/txt/msg00661.txt.bz2 --Boundary-00=_U3cBJl6yrO12G4D Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 1141 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 --Boundary-00=_U3cBJl6yrO12G4D Content-Type: text/x-diff; charset="utf-8"; name="target_can_step_over_breakpoints.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="target_can_step_over_breakpoints.diff" Content-length: 11088 gdb/ 2008-10-27 Pedro Alves * 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 * 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 --Boundary-00=_U3cBJl6yrO12G4D--