From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5484 invoked by alias); 8 Aug 2011 15:44:27 -0000 Received: (qmail 5473 invoked by uid 22791); 8 Aug 2011 15:44:26 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 08 Aug 2011 15:44:11 +0000 Received: (qmail 11967 invoked from network); 8 Aug 2011 15:44:10 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 8 Aug 2011 15:44:10 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [patch] Do not bpstat_clear_actions on throw_exception Date: Mon, 08 Aug 2011 15:44:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-8-generic; KDE/4.6.2; x86_64; ; ) Cc: Jan Kratochvil References: <20110805201417.GA23405@host1.jankratochvil.net> In-Reply-To: <20110805201417.GA23405@host1.jankratochvil.net> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201108081644.08465.pedro@codesourcery.com> 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: 2011-08/txt/msg00148.txt.bz2 On Friday 05 August 2011 21:14:17, Jan Kratochvil wrote: > Hi, > > as discussed here recently throw_exception does various unwised side effects. > This patch is a pre-requisite for the entryval patchset as unavailable entry > values is a normal caught silent exception there but it now stops any commands > execution. > > This function was there already during initial import of GDB to CVS. I do not > see its reason as any uncaught exception throws through bpstat_do_actions_1 > and thus terminates the bpstat evaluation anyway. > > And it the exception is caught there it is wrong to stop the commands > execution. This isn't enough. If you have a bpstat with only one breakpoint with commands, and the error you tried was while running the breakpoint's commands, you won't see a problem, because bpstat_do_actions_1 does bs->commands_left = NULL before executing the commands. But, e.g., try something like: (gdb) b foo (gdb) commands > p 1 > p *0 > p 2 end (gdb) b foo (gdb) commands > p 3 > p *0 > p 4 end and run to foo. After seeing the error, try any other command, like "p 5". You'll see the breakpoint commands of the other breakpoint running... Things get more complicated with any error that is thrown from between bpstat_stop_status, and actually running the breakpoint commands. You'll also leave bs->commands_left set then. Maybe bs->commands_left shouldn't be set by bpstat_stop_status at all. > > There was also the reset of bpstat->commands but I do not find it related: > > http://sourceware.org/ml/gdb-patches/2003-12/msg00350.html > RFA: protect breakpoint commands from being freed > http://sourceware.org/ml/gdb-cvs/2003-12/msg00136.html > commit 79dbd81d5190cb413c7c44fdf00a858576f14e7e > Author: Jim Blandy > Date: Mon Dec 22 03:43:19 2003 +0000 > > * breakpoint.c (bpstat_do_actions): To ensure that > clear_proceed_status doesn't free the command tree we're > evaluating out from under us, zero the bpstat's pointer to it, and > take care of freeing it ourselves. > * cli/cli-script.c (make_cleanup_free_command_lines): Make this > function externally visible. > * cli/cli-script.h (make_cleanup_free_command_lines): New > declaration. > > No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu. > > > Thanks, > Jan > > > gdb/ > 2011-08-05 Jan Kratochvil > > * breakpoint.c (bpstat_clear_actions): Remove. > * breakpoint.h (bpstat_clear_actions): Remove. > * exceptions.c (throw_exception): Remove variable tp, its > initialization and the call of bpstat_clear_actions. > > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -3186,23 +3186,6 @@ bpstat_num (bpstat *bsp, int *num) > return 1; > } > > -/* Modify BS so that the actions will not be performed. */ > - > -void > -bpstat_clear_actions (bpstat bs) > -{ > - for (; bs != NULL; bs = bs->next) > - { > - decref_counted_command_line (&bs->commands); > - bs->commands_left = NULL; > - if (bs->old_val != NULL) > - { > - value_free (bs->old_val); > - bs->old_val = NULL; > - } > - } > -} > - > /* Called when a command is about to proceed the inferior. */ > > static void > --- a/gdb/breakpoint.h > +++ b/gdb/breakpoint.h > @@ -895,9 +895,6 @@ extern int bpstat_num (bpstat *, int *); > command loop). */ > extern void bpstat_do_actions (void); > > -/* Modify BS so that the actions will not be performed. */ > -extern void bpstat_clear_actions (bpstat); > - > /* Implementation: */ > > /* Values used to tell the printing routine how to behave for this > --- a/gdb/exceptions.c > +++ b/gdb/exceptions.c > @@ -207,22 +207,9 @@ exceptions_state_mc_action_iter_1 (void) > void > throw_exception (struct gdb_exception exception) > { > - struct thread_info *tp = NULL; > - > quit_flag = 0; > immediate_quit = 0; > > - if (!ptid_equal (inferior_ptid, null_ptid)) > - tp = find_thread_ptid (inferior_ptid); > - > - /* Perhaps it would be cleaner to do this via the cleanup chain (not sure > - I can think of a reason why that is vital, though). */ > - if (tp != NULL) > - { > - /* Clear queued breakpoint commands. */ > - bpstat_clear_actions (tp->control.stop_bpstat); > - } > - > do_cleanups (ALL_CLEANUPS); > > /* Jump to the containing catch_errors() call, communicating REASON > -- Pedro Alves