From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16700 invoked by alias); 6 Oct 2008 23:46:40 -0000 Received: (qmail 16690 invoked by uid 22791); 6 Oct 2008 23:46:39 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-1.vmware.com (HELO smtp-outbound-1.vmware.com) (65.113.40.141) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 06 Oct 2008 23:45:59 +0000 Received: from mailhost4.vmware.com (mailhost4.vmware.com [10.16.67.124]) by smtp-outbound-1.vmware.com (Postfix) with ESMTP id C2D6568EF; Mon, 6 Oct 2008 16:45:56 -0700 (PDT) Received: from [10.20.92.59] (promb-2s-dhcp59.eng.vmware.com [10.20.92.59]) by mailhost4.vmware.com (Postfix) with ESMTP id 2F400C9A3A; Mon, 6 Oct 2008 16:45:56 -0700 (PDT) Message-ID: <48EAA2C6.2020502@vmware.com> Date: Mon, 06 Oct 2008 23:46:00 -0000 From: Michael Snyder User-Agent: Thunderbird 1.5.0.12 (X11/20080411) MIME-Version: 1.0 To: Joel Brobecker CC: "gdb-patches@sourceware.org" , Daniel Jacobowitz , Pedro Alves , teawater Subject: Re: [RFA] Reverse Debugging, 3/5 References: <48E3CD0B.8020003@vmware.com> <20081006212132.GB21853@adacore.com> <48EA83AD.9040004@vmware.com> <20081006214317.GD21853@adacore.com> In-Reply-To: <20081006214317.GD21853@adacore.com> Content-Type: multipart/mixed; boundary="------------020002080904040705000304" 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/msg00190.txt.bz2 This is a multi-part message in MIME format. --------------020002080904040705000304 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1063 Joel Brobecker wrote: >> But on the other hand, this is exactly what we are doing here. >> We are stepping into a function. Only we're doing it in >> reverse, so we're coming in thru a return, not thru a call. > > I think part of the issue is that, to me, "step_into_function" is > a misleading name for that function, as it implies that we haven't > stepped into the function yet. So, what the function does is, > now that we've stepped into the function, see if we need to continue > somewhere a little farther or not. So, to me, doing the reverse of > "step_into_function" meant going back to the calling site... > >> You still think I should split them up? > > At the very least, I think that a comment explaining what the context > and what we need to do would be very useful. But I also think that > putting the reverse part in its own function would be even clearer. > Your choice, though. All right, how do you like this change (as a diff from the previous change)? Don't worry, I'll post a new revised patch when we're at or near convergence. --------------020002080904040705000304 Content-Type: text/plain; name="stepped.txt" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="stepped.txt" Content-length: 4899 2008-10-06 Michael Snyder * infrun.c (step_into_function): Rename to stepped_into_function. Split into two versions (normal (forward), and reverse). (handle_inferior_event): Call stepped_into_function or stepped_into_function_backward, depending on exec_direction. Index: infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.322.2.7 diff -u -p -r1.322.2.7 infrun.c --- infrun.c 6 Oct 2008 23:24:00 -0000 1.322.2.7 +++ infrun.c 6 Oct 2008 23:44:06 -0000 @@ -1474,7 +1474,8 @@ void init_execution_control_state (struc void handle_inferior_event (struct execution_control_state *ecs); -static void step_into_function (struct execution_control_state *ecs); +static void stepped_into_function (struct execution_control_state *ecs); +static void stepped_into_function_backward (struct execution_control_state *ecs); static void insert_step_resume_breakpoint_at_frame (struct frame_info *step_frame); static void insert_step_resume_breakpoint_at_caller (struct frame_info *); static void insert_step_resume_breakpoint_at_sal (struct symtab_and_line sr_sal, @@ -3226,7 +3227,13 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME ( tmp_sal = find_pc_line (ecs->stop_func_start, 0); if (tmp_sal.line != 0) { - step_into_function (ecs); + /* Find start of appropriate source line (either first or + last line in callee, depending on execution + direction). */ + if (target_get_execution_direction () == EXEC_REVERSE) + stepped_into_function_backward (ecs); + else + stepped_into_function (ecs); return; } } @@ -3408,42 +3415,21 @@ currently_stepping (struct thread_info * || bpstat_should_step ()); } -/* Subroutine call with source code we should not step over. Do step - to the first line of code in it. */ +/* Inferior has stepped into a subroutine call with source code that + we should not step over. Do step to the first line of code in + it. */ static void -step_into_function (struct execution_control_state *ecs) +stepped_into_function (struct execution_control_state *ecs) { struct symtab *s; struct symtab_and_line stop_func_sal, sr_sal; s = find_pc_symtab (stop_pc); if (s && s->language != language_asm) - ecs->stop_func_start = gdbarch_skip_prologue - (current_gdbarch, ecs->stop_func_start); + ecs->stop_func_start = gdbarch_skip_prologue (current_gdbarch, + ecs->stop_func_start); - if (target_get_execution_direction () == EXEC_REVERSE) - { - stop_func_sal = find_pc_line (stop_pc, 0); - - /* OK, we're just going to keep stepping here. */ - if (stop_func_sal.pc == stop_pc) - { - /* We're there already. Just stop stepping now. */ - ecs->event_thread->stop_step = 1; - print_stop_reason (END_STEPPING_RANGE, 0); - stop_stepping (ecs); - return; - } - /* Else just reset the step range and keep going. - No step-resume breakpoint, they don't work for - epilogues, which can have multiple entry paths. */ - ecs->event_thread->step_range_start = stop_func_sal.pc; - ecs->event_thread->step_range_end = stop_func_sal.end; - keep_going (ecs); - return; - } - /* else... */ stop_func_sal = find_pc_line (ecs->stop_func_start, 0); /* Use the step_resume_break to step until the end of the prologue, even if that involves jumps (as it seems to on the vax under @@ -3505,6 +3491,43 @@ step_into_function (struct execution_con keep_going (ecs); } +/* Inferior has stepped backward into a subroutine call with source + code that we should not step over. Do step to the beginning of the + last line of code in it. */ + +static void +stepped_into_function_backward (struct execution_control_state *ecs) +{ + struct symtab *s; + struct symtab_and_line stop_func_sal, sr_sal; + + s = find_pc_symtab (stop_pc); + if (s && s->language != language_asm) + ecs->stop_func_start = gdbarch_skip_prologue (current_gdbarch, + ecs->stop_func_start); + + stop_func_sal = find_pc_line (stop_pc, 0); + + /* OK, we're just going to keep stepping here. */ + if (stop_func_sal.pc == stop_pc) + { + /* We're there already. Just stop stepping now. */ + ecs->event_thread->stop_step = 1; + print_stop_reason (END_STEPPING_RANGE, 0); + stop_stepping (ecs); + } + else + { + /* Else just reset the step range and keep going. + No step-resume breakpoint, they don't work for + epilogues, which can have multiple entry paths. */ + ecs->event_thread->step_range_start = stop_func_sal.pc; + ecs->event_thread->step_range_end = stop_func_sal.end; + keep_going (ecs); + } + return; +} + /* Insert a "step-resume breakpoint" at SR_SAL with frame ID SR_ID. This is used to both functions and to skip over code. */ --------------020002080904040705000304--