[patch 2/2] Do not bpstat_clear_actions on throw_exception #3
Jan Kratochvil
jan.kratochvil@redhat.com
Tue Aug 23 20:33:00 GMT 2011
On Mon, 22 Aug 2011 19:06:15 +0200, Pedro Alves wrote:
> Not all cases. In async mode, handle_inferior_event is called
> _outside_ of execute_command, directly by the event loop (well, almost
> directly). Thus any exception thrown between bpstat_stop_status is called,
> and the bpstat_do_actions call in inf-loop.c, will leave the thread
> with a dangling bpstat too. Might be good enough to wrap
> handle_inferior_event with a similar cleanup?
Done, thanks. On top of the same patch 1/2 as before.
Testing is more difficult, going to post now patch 3/2 but in fact the real
continuation is not tested there as the testcase gets caught by
execute_command. I haven't done a proper testcase for the async mode.
No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu.
Thanks,
Jan
gdb/
2011-08-21 Jan Kratochvil <jan.kratochvil@redhat.com>
* breakpoint.c (bpstat_do_actions): New variable cleanup_if_error, call
make_bpstat_clear_actions_cleanup and discard_cleanups for it.
* defs.h (make_bpstat_clear_actions_cleanup): New declaration.
* exceptions.c (throw_exception): Remove the bpstat_clear_actions call.
* inf-loop.c (inferior_event_handler): New variable cleanup_if_error,
call make_bpstat_clear_actions_cleanup and discard_cleanups for it.
Call bpstat_clear_actions for failed fetch_inferior_event_wrapper.
* top.c (execute_command): New variable cleanup_if_error, call
make_bpstat_clear_actions_cleanup and discard_cleanups for it.
* utils.c (do_bpstat_clear_actions_cleanup)
(make_bpstat_clear_actions_cleanup): New functions.
gdb/testsuite/
2011-08-21 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.base/commands.exp (error_clears_commands_left): New function.
(): Call it.
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3352,6 +3352,8 @@ bpstat_do_actions_1 (bpstat *bsp)
void
bpstat_do_actions (void)
{
+ struct cleanup *cleanup_if_error = make_bpstat_clear_actions_cleanup ();
+
/* Do any commands attached to breakpoint we are stopped at. */
while (!ptid_equal (inferior_ptid, null_ptid)
&& target_has_execution
@@ -3363,6 +3365,8 @@ bpstat_do_actions (void)
indicate the inferior was not resumed. */
if (!bpstat_do_actions_1 (&inferior_thread ()->control.stop_bpstat))
break;
+
+ discard_cleanups (cleanup_if_error);
}
/* Print out the (old or new) value associated with a watchpoint. */
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -429,6 +429,8 @@ extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
extern int parse_pid_to_attach (char *args);
+extern struct cleanup *make_bpstat_clear_actions_cleanup (void);
+
/* From demangle.c */
extern void set_demangling_style (char *);
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -210,10 +210,6 @@ throw_exception (struct gdb_exception exception)
quit_flag = 0;
immediate_quit = 0;
- /* 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). */
- bpstat_clear_actions ();
-
do_cleanups (ALL_CLEANUPS);
/* Jump to the containing catch_errors() call, communicating REASON
--- a/gdb/inf-loop.c
+++ b/gdb/inf-loop.c
@@ -42,6 +42,7 @@ inferior_event_handler (enum inferior_event_type event_type,
{
struct gdb_exception e;
int was_sync = 0;
+ struct cleanup *cleanup_if_error = make_bpstat_clear_actions_cleanup ();
switch (event_type)
{
@@ -53,6 +54,7 @@ inferior_event_handler (enum inferior_event_type event_type,
if (!catch_errors (fetch_inferior_event_wrapper,
client_data, "", RETURN_MASK_ALL))
{
+ bpstat_clear_actions ();
do_all_intermediate_continuations (1);
do_all_continuations (1);
async_enable_stdin ();
@@ -142,6 +144,8 @@ inferior_event_handler (enum inferior_event_type event_type,
printf_unfiltered (_("Event type not recognized.\n"));
break;
}
+
+ discard_cleanups (cleanup_if_error);
}
static int
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -368,12 +368,13 @@ prepare_execute_command (void)
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_bpstat_clear_actions_cleanup ();
cleanup = prepare_execute_command ();
/* Force cleanup of any alloca areas if using C alloca instead of
@@ -477,7 +478,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/utils.c
+++ b/gdb/utils.c
@@ -3674,6 +3674,23 @@ parse_pid_to_attach (char *args)
return pid;
}
+/* Helper for make_bpstat_clear_actions_cleanup. */
+
+static void
+do_bpstat_clear_actions_cleanup (void *unused)
+{
+ bpstat_clear_actions ();
+}
+
+/* Call bpstat_clear_actions for the case an exception is throw. You should
+ discard_cleanups if no exception is caught. */
+
+struct cleanup *
+make_bpstat_clear_actions_cleanup (void)
+{
+ return make_cleanup (do_bpstat_clear_actions_cleanup, NULL);
+}
+
/* Provide a prototype to silence -Wmissing-prototypes. */
extern initialize_file_ftype _initialize_utils;
--- 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
More information about the Gdb-patches
mailing list