From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2238 invoked by alias); 25 Oct 2004 20:18:35 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 2214 invoked from network); 25 Oct 2004 20:18:33 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 25 Oct 2004 20:18:33 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id i9PKIS3u009556 for ; Mon, 25 Oct 2004 16:18:33 -0400 Received: from localhost.redhat.com (to-dhcp51.toronto.redhat.com [172.16.14.151]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i9PKISr30120; Mon, 25 Oct 2004 16:18:28 -0400 Received: from [127.0.0.1] (localhost.localdomain [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 8964F1294B5; Mon, 25 Oct 2004 16:17:21 -0400 (EDT) Message-ID: <417D5F4E.5010403@gnu.org> Date: Mon, 25 Oct 2004 20:18:00 -0000 From: Andrew Cagney User-Agent: Mozilla Thunderbird 0.8 (X11/20041020) MIME-Version: 1.0 To: Orjan Friberg Cc: gdb-patches@sources.redhat.com Subject: Re: STEP_SKIPS_DELAY question, sort of References: <40AE38D0.7010204@axis.com> <40AE659A.90207@gnu.org> <40B1BD1B.4090300@axis.com> <40B23BB2.6070001@gnu.org> <40B33399.3090803@axis.com> <40B3B742.50007@gnu.org> <40B465E7.7050702@axis.com> <40C45B95.9050309@axis.com> <40C46290.9000402@axis.com> <40C46919.2060802@axis.com> <40C484FE.5080702@gnu.org> <40C6DCF9.2060700@axis.com> <40C73411.9060708@gnu.org> <415D3EC2.80804@axis.com> In-Reply-To: <415D3EC2.80804@axis.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2004-10/txt/msg00414.txt.bz2 Sorry, picking up old threads. Orjan Friberg wrote: > Andrew Cagney wrote: > >> >> Can a simple, separate, more explicit logic like: >> if (we just did a step and STEP_SKIPS_DELAY (pc)) >> set up for another step >> return; >> work? The [handle_inferior_event patch snipped] was nested within >> other logic and that's not good from a readability / maintainability >> point of view. > > > Now that I've done a lot more testing, I'm picking this up again. > (Below is just the infrun.c part; gdbarch.sh obviously needs a patch, > and mips-tdep.c needs to have its current STEP_SKIPS_DELAY > implementation converted - I'll post a complete patch if this part is > approved of.) > > A few things: > * I'm not sure how to reliably detect the situation "stepping off a > breakpoint" in handle_inferior_event. I used stop_signal == > TARGET_SIGNAL_TRAP && trap_expected && currently_stepping (ecs)); could > that be too inclusive?. I think this is sufficient. Can you add something mentioning "stepping off a breakpoint" to the comment - that makes the intent clear. > * Distinguishing between "step" and "continue" (using step_range_end) is > not necessary for it to work, but explicitly returning in the "continue" > case is making things a bit clearer. Always a good idea. Andrew > * As the comment suggests, in the "step" case we don't want to preclude > the stop_after_trap check - I assume that whatever is in the delay slot > could potentially correspond to a single line of code (if nothing else, > then at least an asm("...") construct.) > > Index: infrun.c > =================================================================== > RCS file: /cvs/src/src/gdb/infrun.c,v > retrieving revision 1.177 > diff -u -r1.177 infrun.c > --- infrun.c 13 Sep 2004 18:26:28 -0000 1.177 > +++ infrun.c 1 Oct 2004 11:22:42 -0000 > @@ -721,17 +721,12 @@ > > if (read_pc () == stop_pc && breakpoint_here_p (read_pc ())) > oneproc = 1; > - > -#ifndef STEP_SKIPS_DELAY > -#define STEP_SKIPS_DELAY(pc) (0) > -#define STEP_SKIPS_DELAY_P (0) > -#endif > - /* Check breakpoint_here_p first, because breakpoint_here_p is fast > - (it just checks internal GDB data structures) and > STEP_SKIPS_DELAY > - is slow (it needs to read memory from the target). */ > - if (STEP_SKIPS_DELAY_P > - && breakpoint_here_p (read_pc () + 4) > - && STEP_SKIPS_DELAY (read_pc ())) > + > + /* If we stepped into something that needs to be stepped again > before > + before re-inserting the breakpoint, then do so. */ > + else if (gdbarch_single_step_through_delay_p (current_gdbarch) > + && gdbarch_single_step_through_delay (current_gdbarch, > + get_current_frame ())) > oneproc = 1; > } > else > @@ -1793,6 +1788,35 @@ > stopped_by_random_signal = 0; > breakpoints_failed = 0; > > + if (gdbarch_single_step_through_delay_p (current_gdbarch) > + && stop_signal == TARGET_SIGNAL_TRAP && trap_expected > + && currently_stepping (ecs)) > + { > + /* We are in the process of stepping off a breakpoint. If we > stepped > + into something that needs to be stepped again before re-inserting > + the breakpoint, then do so. */ > + int step_through_delay > + = gdbarch_single_step_through_delay (current_gdbarch, > + get_current_frame ()); > + if (step_range_end == 0 && step_through_delay) > + { > + /* The user issued a continue when stopped at a breakpoint. > + Set up for another trap and get out of here. */ > + ecs->another_trap = 1; > + keep_going (ecs); > + return; > + } > + else if (step_through_delay) > + { > + /* The user issued a step when stopped at a breakpoint. > + Maybe we should stop, maybe we should not - the delay slot > + *might* correspond to a line of source. In any case, don't > decide > + that here, just set ecs->another_trap, making sure we > + single-step again before breakpoints are re-inserted. */ > + ecs->another_trap = 1; > + } > + } > + > /* Look at the cause of the stop, and decide what to do. > The alternatives are: > 1) break; to really stop and return to the debugger, >