This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch 2/2] Do not bpstat_clear_actions on throw_exception #3


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]