From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23452 invoked by alias); 27 Nov 2012 15:20:22 -0000 Received: (qmail 23435 invoked by uid 22791); 27 Nov 2012 15:20:18 -0000 X-SWARE-Spam-Status: No, hits=-4.2 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,MISSING_HEADERS,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_EG X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 27 Nov 2012 15:20:04 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1TdMx9-0004WW-7B from Luis_Gustavo@mentor.com for gdb-patches@sourceware.org; Tue, 27 Nov 2012 07:20:03 -0800 Received: from NA1-MAIL.mgc.mentorg.com ([147.34.98.181]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Tue, 27 Nov 2012 07:20:02 -0800 Received: from [0.0.0.0] ([172.16.63.104]) by NA1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 27 Nov 2012 07:20:02 -0800 Message-ID: <50B4DA35.3010206@codesourcery.com> Date: Tue, 27 Nov 2012 15:20:00 -0000 From: Luis Machado Reply-To: lgustavo@codesourcery.com User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.28) Gecko/20120313 Lightning/1.0b2 Thunderbird/3.1.20 MIME-Version: 1.0 CC: gdb-patches@sourceware.org Subject: Re: [PATCH] Support targets that know how to step over breakpoints References: <506D859E.9050600@codesourcery.com> <507E99BB.8050105@codesourcery.com> <508FB2B6.6040006@codesourcery.com> In-Reply-To: <508FB2B6.6040006@codesourcery.com> Content-Type: multipart/mixed; boundary="------------060005000907030704090303" 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: 2012-11/txt/msg00743.txt.bz2 This is a multi-part message in MIME format. --------------060005000907030704090303 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1107 On 10/30/2012 08:57 AM, Luis Machado wrote: > On 10/17/2012 08:42 AM, Luis Machado wrote: >> Ping? >> >> On 10/04/2012 09:48 AM, Luis Machado wrote: >>> 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. >> >> > > Ping? http://sourceware.org/ml/gdb-patches/2012-10/msg00067.html > Meanwhile i've updated this patch for the latest cvs head. I'm wondering if the patch is too ugly for someone to take a look at it or if it is too odd a feature to add. I suppose not. Hopefully i can get some traction with this new refreshed and shiny version! :-) Luis --------------060005000907030704090303 Content-Type: text/x-patch; name="0001-remote_step_over_breaks.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-remote_step_over_breaks.diff" Content-length: 12727 2012-11-27 Luis Machado Pedro Alves 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-11-27 09:24:30.955089370 -0200 +++ gdb_head/gdb/doc/gdb.texinfo 2012-11-27 09:33:05.411095450 -0200 @@ -37083,6 +37083,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: @@ -37237,6 +37242,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-10-10 15:35:25.384415678 -0300 +++ gdb_head/gdb/doc/gdbint.texinfo 2012-11-27 09:33:05.415095451 -0200 @@ -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-11-27 09:24:30.987089370 -0200 +++ gdb_head/gdb/infrun.c 2012-11-27 09:33:05.423095450 -0200 @@ -1786,7 +1786,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 @@ -1912,16 +1913,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) { @@ -2219,19 +2227,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) @@ -3941,10 +3952,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 @@ -5715,10 +5728,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-11-27 09:24:31.007089371 -0200 +++ gdb_head/gdb/remote.c 2012-11-27 09:33:05.423095450 -0200 @@ -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; @@ -11001,6 +11017,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) { @@ -11070,6 +11093,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-11-27 09:24:31.023089369 -0200 +++ gdb_head/gdb/target.c 2012-11-27 09:33:05.427095452 -0200 @@ -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-11-27 09:24:31.023089369 -0200 +++ gdb_head/gdb/target.h 2012-11-27 09:33:05.431095451 -0200 @@ -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); --------------060005000907030704090303--