From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24838 invoked by alias); 18 Aug 2011 16:42:40 -0000 Received: (qmail 24632 invoked by uid 22791); 18 Aug 2011 16:42:37 -0000 X-SWARE-Spam-Status: No, hits=-2.5 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; Thu, 18 Aug 2011 16:42:23 +0000 Received: (qmail 11695 invoked from network); 18 Aug 2011 16:42:22 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 18 Aug 2011 16:42:22 -0000 From: Pedro Alves To: Jan Kratochvil Subject: Re: [patch] Do not bpstat_clear_actions on throw_exception Date: Thu, 18 Aug 2011 16:42:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-10-generic; KDE/4.7.0; x86_64; ; ) Cc: gdb-patches@sourceware.org References: <20110805201417.GA23405@host1.jankratochvil.net> <201108081644.08465.pedro@codesourcery.com> <20110818153413.GA23189@host1.jankratochvil.net> In-Reply-To: <20110818153413.GA23189@host1.jankratochvil.net> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201108181742.19980.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/msg00361.txt.bz2 On Thursday 18 August 2011 16:34:13, Jan Kratochvil wrote: > > 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. > > The code path execute_command...bpstat_do_actions is multiple times in GDB > (CLI, MI, bpstat_do_actions from continuations etc.). The path > execute_command...bpstat_do_actions is currently unprotected by any TRY_CATCH, > therefore I see the only possibility to clear commands_left _before_ > execute_command. One could protect execute_command...bpstat_do_actions but > that is present at multiple places and I find the patch below also OK. > > I rather cleared commands/commands_left before starting a new command as it > seems easily unified in GDB. > > There is also normal_stop but that is before the commands get executed at all. Unfortunately, the hook-stop handling is in normal_stop. Your patch clears the breakpoint commands before get get a chance to run if the user installs a hook-stop. E.g., before: (top-gdb) define hook-stop Type commands for definition of "hook-stop". End with a line saying just "end". >echo hook-stop\n >end (top-gdb) b Breakpoint 4 at 0x4513b0: file ../../src/gdb/gdb.c, line 27. (top-gdb) commands Type commands for breakpoint(s) 4, one per line. End with a line saying just "end". >echo break commands\n >end (top-gdb) jump 27 Continuing at 0x4513b0. hook-stop ^^^^^^^^^ Breakpoint 4, main (argc=1, argv=0x7fffffffe068) at ../../src/gdb/gdb.c:27 27 { break commands ^^^^^^^^^^^^^^ after: (top-gdb) define hook-stop Type commands for definition of "hook-stop". End with a line saying just "end". >echo hook-stop\n >end (top-gdb) b 27 Breakpoint 4 at 0x4513b0: file ../../src/gdb/gdb.c, line 27. (top-gdb) commands Type commands for breakpoint(s) 4, one per line. End with a line saying just "end". >echo break commands\n >end (top-gdb) jump 27 Continuing at 0x4513b0. hook-stop ^^^^^^^^^ Breakpoint 4, main (argc=1, argv=0x7fffffffe068) at ../../src/gdb/gdb.c:27 27 { (top-gdb) This looks tricky to get right without changing gdb's behavior :-( We could try pushing bpstat_do_actions to the end of an execution command's run, but e.g., that would change the behavior of breakpoint commands in command lists, and things like "step N". OTOH, maybe that'd be the right thing to do (the current behavior could be seen as buggy --- more thought is needed). > > > Maybe bs->commands_left shouldn't be set by > > bpstat_stop_status at all. > > It already needs to check for the "silent" statement. Since bs->commands is refcounted, I don't understand why we need bs->commands_left at all. bpstat_do_actions_1 could instead itself skip the first command if it is "silent". > Besides it if > "commands" are fetched in bpstat_do_actions_1 the unwanted execution you have > shown to me can also happen. Yeah, I got confused thinking that it'd make a difference to make bpstat_do_actions_1 iterate on bs->commands directly rather than through bs->commands_left. -- Pedro Alves