This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 2/2] Do not bpstat_clear_actions on throw_exception #3
On Wed, 24 Aug 2011 12:19:03 +0200, Pedro Alves wrote:
> Almost there, but not yet, sorry. :-( The cleanup is installed before
> fetch_inferior_event (non-stop) switches inferior_ptid to the event thread
> with a cleanup that restores back to the previous selected thread, which
> means, this happens:
>
> inferior_event_handler
> -> install cleanup to clear the bpstat of the current thread
> -> fetch_inferior_event
> -> install cleanup to restore the selected thread
> -> switch to the event thread
> -> handle_inferior_event
> -> bpstat_stop_status on the now current event thread
> -> some error is thrown, e.g., while unwinding for step/next.
> -> the cleanup to restore the previous thread is ran
> -> the cleanup to clear the bpstat of the current
> thread is ran, but the current thread is not the
> event thread. The bpstat of the event thread is left
> handling.
>
> Installing the cleanup in fetch_inferior_event _after_
> make_cleanup_restore_current_thread would fix this.
OK this way? It fixed the reproducer (unsuitable for the testsuite) for me.
Thanks,
Jan
gdb/
2011-08-26 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.
* infrun.c (fetch_inferior_event): Call
make_bpstat_clear_actions_cleanup.
* 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-26 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/infrun.c
+++ b/gdb/infrun.c
@@ -2779,6 +2779,10 @@ fetch_inferior_event (void *client_data)
else
ts_old_chain = make_cleanup (finish_thread_state_cleanup, &ecs->ptid);
+ /* Get executed before make_cleanup_restore_current_thread above to apply
+ still for the thread which has thrown the exception. */
+ make_bpstat_clear_actions_cleanup ();
+
/* Now figure out what to do with the result of the result. */
handle_inferior_event (ecs);
--- 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