From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24771 invoked by alias); 7 Oct 2008 18:46:21 -0000 Received: (qmail 24759 invoked by uid 22791); 7 Oct 2008 18:46:20 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-2.vmware.com (HELO smtp-outbound-2.vmware.com) (65.115.85.73) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 07 Oct 2008 18:45:45 +0000 Received: from mailhost5.vmware.com (mailhost5.vmware.com [10.16.68.131]) by smtp-outbound-2.vmware.com (Postfix) with ESMTP id C6E291000C; Tue, 7 Oct 2008 11:45:43 -0700 (PDT) Received: from [10.20.92.59] (promb-2s-dhcp59.eng.vmware.com [10.20.92.59]) by mailhost5.vmware.com (Postfix) with ESMTP id C004BDC061; Tue, 7 Oct 2008 11:45:43 -0700 (PDT) Message-ID: <48EBADE0.4010405@vmware.com> Date: Tue, 07 Oct 2008 18: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> <48EAA2C6.2020502@vmware.com> <20081007040703.GF28138@adacore.com> In-Reply-To: <20081007040703.GF28138@adacore.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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/msg00221.txt.bz2 Joel Brobecker wrote: >>> 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... > > When I wrote that, I wasn't asking you to change the name. I hope > you didn't feel forced to do it... > >> * infrun.c (step_into_function): Rename to stepped_into_function. > > Can I suggest: handle_stepped_into_function. This follows the > "handle_inferior_event" style while explaining that we're handling > the situation we're dealing with. > > Same for handle_stepped_into_function_backwards. > >> + /* 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; > > I think that the comment is redundant in this case. Hopefully the > function name should give a good clue of what it is supposed to do > (handle the case of just having stepped into a function, either > forward or backwards depending on the execution direction). The > actual details of how they do it do not matter at this level, > and keeping a copy here introduces some chances that it might > become out of date. OK, I'll add "handle_" to the name, and remove the redundant comment. >> +/* Inferior has stepped into a subroutine call with source code that > ^ The inferior... >> + we should not step over. Do step to the first line of code in >> + it. */ > >> +/* Inferior has stepped backward into a subroutine call with source > Same here: "The inferior..." >> + code that we should not step over. Do step to the beginning of the >> + last line of code in it. */ > > I am sorry - perhaps this is because it's getting late, but I'm getting > confused again. What does it mean to step backward into a subroutine > call? My understanding was completely wrong. Could you give maybe > an example at the user-interface level? Maybe it's a typo in the > function description, but the function implementation doesn't seem > to be doing what you say it is. "Do step to the beginning of the > current line of code"? (I'm still interested in my example) Sure -- you're right, it is confusing. I should remove the word "call" in the comment above. Here's your example: 18: i = i + 1; 19: foo (); 20: j = j + 1; Now if I am at line 18, and I step forward, I will step into function foo, thru the call and foo's prologue. I will expect to stop at the beginning of the first source line in foo, after the prologue. OTOH, if I am at line 20 and I step backward, I will also step into function foo -- thru the return instruction and foo's epilogue. In this case, I will expect to stop at the beginning of the LAST (executed) source line in foo, before the epilogue. In each case it is accomplished by single-stepping and using line information. There is a difference in that functions usually have a single entry but may have multiple returns, but the single-stepping makes this transparent. >> +static void >> +stepped_into_function_backward (struct execution_control_state *ecs) > > English question: Are "backward" and "backwards" interchangeable? > If these two words are strictly equivalent, I'd like for us to remain > consistent. But if now, can you explain to me what the difference is? My non-professional, layman's interpretation is that "backwards" is more slangy. I will switch to using "backward" consistantly. >> + if (s && s->language != language_asm) >> + ecs->stop_func_start = gdbarch_skip_prologue (current_gdbarch, >> + ecs->stop_func_start); > > Depending of your answer to my question above, I'm wondering if > we should actually do a prologue skip... Hmmm. Well, the question is, do we need to update stop_func_start. We don't use it locally, so perhaps we don't need to do it. But I don't really know if we're doing it because someone else will maybe need it later. It might be an artifact. But it's not "incorrect", in that skipping the prologue is the right thing to do IF you want the function start location.