From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25124 invoked by alias); 22 Aug 2011 14:52:27 -0000 Received: (qmail 25093 invoked by uid 22791); 22 Aug 2011 14:52:24 -0000 X-SWARE-Spam-Status: No, hits=-6.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 22 Aug 2011 14:52:02 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p7MEprLE007453 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 22 Aug 2011 10:51:53 -0400 Received: from host1.jankratochvil.net (ovpn-116-42.ams2.redhat.com [10.36.116.42]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p7MEpp2v013675 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 22 Aug 2011 10:51:53 -0400 Received: from host1.jankratochvil.net (localhost [127.0.0.1]) by host1.jankratochvil.net (8.14.4/8.14.4) with ESMTP id p7MEpoCR027414; Mon, 22 Aug 2011 16:51:50 +0200 Received: (from jkratoch@localhost) by host1.jankratochvil.net (8.14.4/8.14.4/Submit) id p7MEpoGs027413; Mon, 22 Aug 2011 16:51:50 +0200 Date: Mon, 22 Aug 2011 14:52:00 -0000 From: Jan Kratochvil To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: [patch 2/2] Do not bpstat_clear_actions on throw_exception #3 Message-ID: <20110822145150.GB11817@host1.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) 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/msg00410.txt.bz2 Hi Pedro, On Thu, 18 Aug 2011 18:42:19 +0200, Pedro Alves wrote: > 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: OK, I agree, I have made a new testcase. > This looks tricky to get right without changing gdb's behavior :-( The question is how big changed you were thinking about. One problem I find one cannot use "step" and other such commands in the breakpoints commands lists. This may be due to GDB trying not to overflow its stack. I gues with async mode it could be implementable as some stack-in-data-structure. But that seems to be out of scope of this patch. > 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). I was playing with various changes but it had various side-effects. Do you have anything against this patch? I hope I have caught all the cases where exceptions can be thrown. Otherwise IMO everything is caught by execute_command anyway. Thanks, Jan gdb/ 2011-08-21 Jan Kratochvil * breakpoint.c (bpstat_do_actions): Wrap it by TRY_CATCH, new variable except, possibly call bpstat_clear_actions. * top.c (bpstat_clear_actions_cleanup): New function. (execute_command): New variable cleanup_if_error, register there bpstat_clear_actions_cleanup, discard it. gdb/testsuite/ 2011-08-21 Jan Kratochvil * gdb.base/commands.exp (error_clears_commands_left): New function. (): Call it. --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -3352,17 +3352,27 @@ bpstat_do_actions_1 (bpstat *bsp) void bpstat_do_actions (void) { - /* Do any commands attached to breakpoint we are stopped at. */ - while (!ptid_equal (inferior_ptid, null_ptid) - && target_has_execution - && !is_exited (inferior_ptid) - && !is_executing (inferior_ptid)) - /* Since in sync mode, bpstat_do_actions may resume the inferior, - and only return when it is stopped at the next breakpoint, we - keep doing breakpoint actions until it returns false to - indicate the inferior was not resumed. */ - if (!bpstat_do_actions_1 (&inferior_thread ()->control.stop_bpstat)) - break; + volatile struct gdb_exception except; + + TRY_CATCH (except, RETURN_MASK_ALL) + { + /* Do any commands attached to breakpoint we are stopped at. */ + while (!ptid_equal (inferior_ptid, null_ptid) + && target_has_execution + && !is_exited (inferior_ptid) + && !is_executing (inferior_ptid)) + /* Since in sync mode, bpstat_do_actions may resume the inferior, + and only return when it is stopped at the next breakpoint, we + keep doing breakpoint actions until it returns false to + indicate the inferior was not resumed. */ + if (!bpstat_do_actions_1 (&inferior_thread ()->control.stop_bpstat)) + break; + } + if (except.reason < 0) + { + bpstat_clear_actions (); + throw_exception (except); + } } /* Print out the (old or new) value associated with a watchpoint. */ --- a/gdb/top.c +++ b/gdb/top.c @@ -362,18 +362,27 @@ prepare_execute_command (void) return cleanup; } +/* Call bpstat_clear_actions with make_cleanup compatible prototype. */ + +static void +bpstat_clear_actions_cleanup (void *unused) +{ + bpstat_clear_actions (); +} + /* Execute the line P as a command, in the current user context. Pass FROM_TTY as second argument to the defining function. */ void execute_command (char *p, int from_tty) { - struct cleanup *cleanup; + struct cleanup *cleanup_if_error, *cleanup; struct cmd_list_element *c; enum language flang; static int warned = 0; char *line; + cleanup_if_error = make_cleanup (bpstat_clear_actions_cleanup, NULL); cleanup = prepare_execute_command (); /* Force cleanup of any alloca areas if using C alloca instead of @@ -477,7 +486,8 @@ execute_command (char *p, int from_tty) } } - do_cleanups (cleanup); + do_cleanups (cleanup); + discard_cleanups (cleanup_if_error); } /* Run execute_command for P and FROM_TTY. Capture its output into the --- a/gdb/testsuite/gdb.base/commands.exp +++ b/gdb/testsuite/gdb.base/commands.exp @@ -678,6 +678,74 @@ proc if_commands_test {} { } } +# Verify an error during "commands" commands execution will prevent any other +# "commands" from other breakpoints at the same location to be executed. + +proc error_clears_commands_left {} { + set test "hook-stop 1" + gdb_test_multiple {define hook-stop} $test { + -re "End with a line saying just \"end\"\\.\r\n>$" { + pass $test + } + } + set test "hook-stop 1a" + gdb_test_multiple {echo hook-stop1\n} $test { + -re "\r\n>$" { + pass $test + } + } + gdb_test_no_output "end" "hook-stop 1b" + + delete_breakpoints + gdb_breakpoint "main" + + set test "main commands 1" + gdb_test_multiple {commands $bpnum} $test { + -re "End with a line saying just \"end\"\\.\r\n>$" { + pass $test + } + } + set test "main commands 1a" + gdb_test_multiple {echo cmd1\n} $test { + -re "\r\n>$" { + pass $test + } + } + set test "main commands 1b" + gdb_test_multiple {errorcommandxy\n} $test { + -re "\r\n>$" { + pass $test + } + } + gdb_test_no_output "end" "main commands 1c" + + gdb_breakpoint "main" + set test "main commands 2" + gdb_test_multiple {commands $bpnum} $test { + -re "End with a line saying just \"end\"\\.\r\n>$" { + pass $test + } + } + set test "main commands 2a" + gdb_test_multiple {echo cmd2\n} $test { + -re "\r\n>$" { + pass $test + } + } + set test "main commands 2b" + gdb_test_multiple {errorcommandyz\n} $test { + -re "\r\n>$" { + pass $test + } + } + gdb_test_no_output "end" "main commands 2c" + + gdb_run_cmd + gdb_test "" "\r\nhook-stop1\r\n.*\r\ncmd1\r\nUndefined command: \"errorcommandxy\"\\. Try \"help\"\\." "cmd1 error" + + gdb_test {echo idle\n} "\r\nidle" "no cmd2" +} + proc redefine_hook_test {} { global gdb_prompt @@ -758,6 +826,7 @@ stray_arg0_test source_file_with_indented_comment recursive_source_test if_commands_test +error_clears_commands_left redefine_hook_test # This one should come last, as it redefines "backtrace". redefine_backtrace_test