From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2075 invoked by alias); 5 Dec 2008 00:30:07 -0000 Received: (qmail 2064 invoked by uid 22791); 5 Dec 2008 00:30:05 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate6.de.ibm.com (HELO mtagate6.de.ibm.com) (195.212.29.155) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 05 Dec 2008 00:29:22 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate6.de.ibm.com (8.13.8/8.13.8) with ESMTP id mB50TJuD389052 for ; Fri, 5 Dec 2008 00:29:19 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id mB50TJt53731504 for ; Fri, 5 Dec 2008 01:29:19 +0100 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id mB50TJUH003861 for ; Fri, 5 Dec 2008 01:29:19 +0100 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id mB50TJdW003858; Fri, 5 Dec 2008 01:29:19 +0100 Message-Id: <200812050029.mB50TJdW003858@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Fri, 5 Dec 2008 01:29:18 +0100 Subject: Re: [RFA] dummy frame handling cleanup, plus inferior fun call signal handling improvement To: dje@google.com (Doug Evans) Date: Fri, 05 Dec 2008 00:30:00 -0000 From: "Ulrich Weigand" Cc: pedro@codesourcery.com (Pedro Alves), gdb-patches@sourceware.org In-Reply-To: from "Doug Evans" at Dec 04, 2008 02:32:12 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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-12/txt/msg00084.txt.bz2 Doug Evans wrote: > Another issue is that if proceed() does error out, users won't see the > evaluation-has-been-abandoned message. That could be fixed by > invoking proceed in a TRY_CATCH. [For reference sake, that's not the > only reason I'm wondering about using TRY_CATCH here.] I think using a TRY_CATCH here would really make a lot of sense ... > > Hmmm, that's not quite what what the code actually does. The state is also > > restored if some error is thrown during the proceed call, for example. > > I'm not sure whether this is really the right thing to do ... > > Right, thanks. > Though it's not clear to me whether "this" in your last sentence is > intending to suggest that the comment is correct or the code is. > > For education's sake, is there an instance where one would want to > restore inferior_status when the inferior function call prematurely > stops (for whatever reason)? [assuming unwindonsignal == off.] I wasn't completely clear on this myself, so this triggered me to think again about the whole question of what to restore when ... Let's look at the various bits of information that are saved and restored, and why they need to be. Expanding a bit on the split your patch already introduced, it seems to me that we have: Inferior thread state (what you call inferior_program_state): current regcache stop_pc (redundant with regcache) tp->stop_signal "proceed" state (mostly what you call inferior_status ...): "Output arguments" of proceed: tp->stop_bpstat tp->stop_step stop_stack_dummy stopped_by_random_signal breakpoint_proceeded "Input arguments" to proceed: stop_after_trap inf->stop_soon tp->proceed_to_finish tp->step_over_calls tp->step_range_start tp->step_range_end tp->step_frame_id Internal proceed control flow: tp->trap_expected (probably does not need to be saved at all) User-selected state (... except for this, which seems somewhat distinct): selected frame implicitly: current thread Now amongst those, the inferior thread state absolutely must be restored at some point, or else we cannot continue running that thread. The proceed state needs to be restored whenever call_function_by_hand returns successfully. This is particularly clear for the "output" arguments: the caller of call_function_by_hand may still be interested in why the last prior call to proceed returned, and should not be distracted by the intervening call to proceed in call_function_by_hand (e.g. if as part of the execution of a breakpoint command list an inferior function happens to be called). The case for saving and restoring the "input" arguments is less compelling; I don't really see where an inferior call could intervene between the setting of those flags and the proceed call for which they are intended. On the other hand, it cannot hurt to save/restore them either. In any case, there is no need (and potential harm if the current thread has changed!) to restore any of this state in the case where call_function_by_hand throws an error; we get thrown back up to the user interface, where any proceed input/output state is no longer interesting. As to the user-selected state, this also needs to be preserved whenever call_function_by_hand returns successfully. If it throws an error, however, the selected frame and thread should *not* be restored but set to the frame/thread in which the error occured. Now let's look at the various places where we might want to restore state: 1. An error can occur during stack frame preparation 2. An error can occur during proceed 3. After proceed finishes normally, we are not where we expect to be (inferior terminated, wrong thread, got signaled, hit breakpoint) 4. After proceed finishes normally, we are at the stack dummy where we expected to be 5. At some point after a prior call_function_by_hand returned an error, we run into the old stack dummy In the case of 1. the registers of the current thread may have been changed, and we need to restore them. At this point, really nothing else could have changed anyway. In the case of 2. or 3., it seems to me we should not restore *anything*. We should leave the current thread and selected frame to indicate where the error has happened, and throw control back up to the user interface layer. Likewise, the inferior thread state should be left unmodified to allow the user to inspect the location where the problem occured (of course, the dummy frame needs to remain pushed to allow a potential restore at some later point). The "proceed" state is irrelevant at this point, and should not be touched. In the case of 4., we need to restore *everything*. The inferior thread state was already restored by the implicit pop of the dummy frame. This leaves the "proceed" state as well as the selected frame to be restored (the current thread must be correct at this point). In the case of 5., only the inferior state needs to be restored; this is what popping the dummy frame already does. To accomplish that, it seems that the simplest way to do so would be to install a cleanup to restore the inferior thread state while preparing the frame; once this is done, the cleanup is removed and responsibility for restoring the inferior state is moved into the (at this point freshly pushed) dummy frame. (This is just what the code already does.) The "proceed" state (and the selected frame) on the other hand should not be installed via a cleanup at all, I think. They should simply be explicitly restored only in the case 4. as above. Does this make sense? I hope I haven't overlooked anything here ... Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com