From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8569 invoked by alias); 1 Oct 2004 11:26:03 -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 8550 invoked from network); 1 Oct 2004 11:25:59 -0000 Received: from unknown (HELO krynn.se.axis.com) (193.13.178.10) by sourceware.org with SMTP; 1 Oct 2004 11:25:59 -0000 Received: from [10.84.130.1] (ironmaiden.se.axis.com [10.84.130.1]) by krynn.se.axis.com (8.12.9/8.12.9/Debian-5local0.1) with ESMTP id i91BPtc8017892; Fri, 1 Oct 2004 13:25:55 +0200 Message-ID: <415D3EC2.80804@axis.com> Date: Fri, 01 Oct 2004 11:26:00 -0000 From: Orjan Friberg Organization: Axis Communications User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040616 MIME-Version: 1.0 To: Andrew Cagney 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> In-Reply-To: <40C73411.9060708@gnu.org> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2004-10/txt/msg00015.txt.bz2 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?. * 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. * 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, -- Orjan Friberg Axis Communications