This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Do not bpstat_clear_actions on throw_exception
On Mon, 08 Aug 2011 17:44:08 +0200, Pedro Alves wrote:
> 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...
Thank you very much, I agree now, created a testcase for it.
> 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.
> Maybe bs->commands_left shouldn't be set by
> bpstat_stop_status at all.
It already needs to check for the "silent" statement. Besides it if
"commands" are fetched in bpstat_do_actions_1 the unwanted execution you have
shown to me can also happen.
Or could you describe more the idea you had?
No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu.
Thanks,
Jan
gdb/
2011-08-18 Jan Kratochvil <jan.kratochvil@redhat.com>
* exceptions.c (throw_exception): Move variable tp, its initialization
and the call of bpstat_clear_actions to ...
* top.c (prepare_execute_command): ... here.
gdb/testsuite/
2011-08-18 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.base/commands.exp (error_clears_commands_left): New function.
(): Call it.
--- 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
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -348,6 +348,7 @@ prepare_execute_command (void)
{
struct value *mark;
struct cleanup *cleanup;
+ struct thread_info *tp = NULL;
mark = value_mark ();
cleanup = make_cleanup_value_free_to_mark (mark);
@@ -359,6 +360,17 @@ prepare_execute_command (void)
if (non_stop)
target_dcache_invalidate ();
+ 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);
+ }
+
return cleanup;
}
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -678,6 +678,61 @@ 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 {} {
+ 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 "" "cmd1\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 +813,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