[patch] Do not bpstat_clear_actions on throw_exception

[patch] Do not bpstat_clear_actions on throw_exception

[Jan Kratochvil]

Hi,

as discussed here recently throw_exception does various unwised side effects.
This patch is a pre-requisite for the entryval patchset as unavailable entry
values is a normal caught silent exception there but it now stops any commands
execution.

This function was there already during initial import of GDB to CVS.  I do not
see its reason as any uncaught exception throws through bpstat_do_actions_1
and thus terminates the bpstat evaluation anyway.

And it the exception is caught there it is wrong to stop the commands
execution.

There was also the reset of bpstat->commands but I do not find it related:

http://sourceware.org/ml/gdb-patches/2003-12/msg00350.html
RFA: protect breakpoint commands from being freed
http://sourceware.org/ml/gdb-cvs/2003-12/msg00136.html
commit 79dbd81d5190cb413c7c44fdf00a858576f14e7e
Author: Jim Blandy <jimb@codesourcery.com>
Date:   Mon Dec 22 03:43:19 2003 +0000

    * breakpoint.c (bpstat_do_actions): To ensure that
    clear_proceed_status doesn't free the command tree we're
    evaluating out from under us, zero the bpstat's pointer to it, and
    take care of freeing it ourselves.
    * cli/cli-script.c (make_cleanup_free_command_lines): Make this
    function externally visible.
    * cli/cli-script.h (make_cleanup_free_command_lines): New
    declaration.

No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu.


Thanks,
Jan


gdb/
2011-08-05  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* breakpoint.c (bpstat_clear_actions): Remove.
	* breakpoint.h (bpstat_clear_actions): Remove.
	* exceptions.c (throw_exception): Remove variable tp, its
	initialization and the call of bpstat_clear_actions.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3186,23 +3186,6 @@ bpstat_num (bpstat *bsp, int *num)
   return 1;
 }
 
-/* Modify BS so that the actions will not be performed.  */
-
-void
-bpstat_clear_actions (bpstat bs)
-{
-  for (; bs != NULL; bs = bs->next)
-    {
-      decref_counted_command_line (&bs->commands);
-      bs->commands_left = NULL;
-      if (bs->old_val != NULL)
-	{
-	  value_free (bs->old_val);
-	  bs->old_val = NULL;
-	}
-    }
-}
-
 /* Called when a command is about to proceed the inferior.  */
 
 static void
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -895,9 +895,6 @@ extern int bpstat_num (bpstat *, int *);
    command loop).  */
 extern void bpstat_do_actions (void);
 
-/* Modify BS so that the actions will not be performed.  */
-extern void bpstat_clear_actions (bpstat);
-
 /* Implementation:  */
 
 /* Values used to tell the printing routine how to behave for this
--- 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

Re: [patch] Do not bpstat_clear_actions on throw_exception

[Pedro Alves]

On Friday 05 August 2011 21:14:17, Jan Kratochvil wrote:
> Hi,
> 
> as discussed here recently throw_exception does various unwised side effects.
> This patch is a pre-requisite for the entryval patchset as unavailable entry
> values is a normal caught silent exception there but it now stops any commands
> execution.
> 
> This function was there already during initial import of GDB to CVS.  I do not
> see its reason as any uncaught exception throws through bpstat_do_actions_1
> and thus terminates the bpstat evaluation anyway.
> 
> And it the exception is caught there it is wrong to stop the commands
> execution.

This isn't enough.  If you have a bpstat with only one breakpoint with
commands, and the error you tried was while running the breakpoint's
commands, you won't see a problem, because bpstat_do_actions_1
does bs->commands_left = NULL before executing the commands.

But, e.g., try something like: 

(gdb) b foo
(gdb) commands
> p 1
> p *0
> p 2
end

(gdb) b foo
(gdb) commands
> p 3
> p *0
> p 4
end

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...

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.

Maybe bs->commands_left shouldn't be set by
bpstat_stop_status at all.

> 
> There was also the reset of bpstat->commands but I do not find it related:
> 
> http://sourceware.org/ml/gdb-patches/2003-12/msg00350.html
> RFA: protect breakpoint commands from being freed
> http://sourceware.org/ml/gdb-cvs/2003-12/msg00136.html
> commit 79dbd81d5190cb413c7c44fdf00a858576f14e7e
> Author: Jim Blandy <jimb@codesourcery.com>
> Date:   Mon Dec 22 03:43:19 2003 +0000
> 
>     * breakpoint.c (bpstat_do_actions): To ensure that
>     clear_proceed_status doesn't free the command tree we're
>     evaluating out from under us, zero the bpstat's pointer to it, and
>     take care of freeing it ourselves.
>     * cli/cli-script.c (make_cleanup_free_command_lines): Make this
>     function externally visible.
>     * cli/cli-script.h (make_cleanup_free_command_lines): New
>     declaration.
> 
> No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu.
> 
> 
> Thanks,
> Jan
> 
> 
> gdb/
> 2011-08-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* breakpoint.c (bpstat_clear_actions): Remove.
> 	* breakpoint.h (bpstat_clear_actions): Remove.
> 	* exceptions.c (throw_exception): Remove variable tp, its
> 	initialization and the call of bpstat_clear_actions.
> 
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -3186,23 +3186,6 @@ bpstat_num (bpstat *bsp, int *num)
>    return 1;
>  }
>  
> -/* Modify BS so that the actions will not be performed.  */
> -
> -void
> -bpstat_clear_actions (bpstat bs)
> -{
> -  for (; bs != NULL; bs = bs->next)
> -    {
> -      decref_counted_command_line (&bs->commands);
> -      bs->commands_left = NULL;
> -      if (bs->old_val != NULL)
> -	{
> -	  value_free (bs->old_val);
> -	  bs->old_val = NULL;
> -	}
> -    }
> -}
> -
>  /* Called when a command is about to proceed the inferior.  */
>  
>  static void
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -895,9 +895,6 @@ extern int bpstat_num (bpstat *, int *);
>     command loop).  */
>  extern void bpstat_do_actions (void);
>  
> -/* Modify BS so that the actions will not be performed.  */
> -extern void bpstat_clear_actions (bpstat);
> -
>  /* Implementation:  */
>  
>  /* Values used to tell the printing routine how to behave for this
> --- 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
> 

-- 
Pedro Alves

Re: [patch] Do not bpstat_clear_actions on throw_exception

[Jan Kratochvil]

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

Re: [patch] Do not bpstat_clear_actions on throw_exception

[Pedro Alves]

On Thursday 18 August 2011 16:34:13, Jan Kratochvil wrote:

> > 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.

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:

(top-gdb) define hook-stop
Type commands for definition of "hook-stop".
End with a line saying just "end".
>echo hook-stop\n
>end
(top-gdb) b 
Breakpoint 4 at 0x4513b0: file ../../src/gdb/gdb.c, line 27.
(top-gdb) commands
Type commands for breakpoint(s) 4, one per line.
End with a line saying just "end".
>echo break commands\n
>end
(top-gdb) jump 27
Continuing at 0x4513b0.
hook-stop
^^^^^^^^^
Breakpoint 4, main (argc=1, argv=0x7fffffffe068) at ../../src/gdb/gdb.c:27
27      {
break commands
^^^^^^^^^^^^^^

after:

(top-gdb) define hook-stop
Type commands for definition of "hook-stop".
End with a line saying just "end".
>echo hook-stop\n
>end
(top-gdb) b 27
Breakpoint 4 at 0x4513b0: file ../../src/gdb/gdb.c, line 27.
(top-gdb) commands
Type commands for breakpoint(s) 4, one per line.
End with a line saying just "end".
>echo break commands\n
>end
(top-gdb) jump 27
Continuing at 0x4513b0.
hook-stop
^^^^^^^^^
Breakpoint 4, main (argc=1, argv=0x7fffffffe068) at ../../src/gdb/gdb.c:27
27      {
(top-gdb) 


This looks tricky to get right without changing gdb's behavior :-(
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).

> 
> > Maybe bs->commands_left shouldn't be set by
> > bpstat_stop_status at all.
> 
> It already needs to check for the "silent" statement. 

Since bs->commands is refcounted, I don't understand
why we need bs->commands_left at all.  bpstat_do_actions_1
could instead itself skip the first command if it is "silent".

> Besides it if
> "commands" are fetched in bpstat_do_actions_1 the unwanted execution you have
> shown to me can also happen.

Yeah, I got confused thinking that it'd make a difference
to make bpstat_do_actions_1 iterate on bs->commands
directly rather than through bs->commands_left.

-- 
Pedro Alves

[patch] Code cleanup: Remove commands_left [Do not bpstat_clear_actions on throw_exception]

[Jan Kratochvil]

On Thu, 18 Aug 2011 18:42:19 +0200, Pedro Alves wrote:
> Since bs->commands is refcounted, I don't understand
> why we need bs->commands_left at all.  bpstat_do_actions_1
> could instead itself skip the first command if it is "silent".

To make the discussion easier let's remove it first.

The patch is simple, I will check it in soon myself.
No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu.


Thanks,
Jan


gdb/
2011-08-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Code cleanup.
	* breakpoint.c (bpstat_clear_actions): Remove clearing of commands_left.
	(command_line_is_silent): New function.
	(bpstat_do_actions_1): No longer use commands_left, use
	command_line_is_silent for commands.
	(bpstat_alloc): Remove clearing of commands_left.
	(bpstat_stop_status): Remove initialization of commands_left, use
	command_line_is_silent.
	* breakpoint.h (struct bpstats): Remove commands_left.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3194,7 +3194,7 @@ bpstat_clear_actions (bpstat bs)
   for (; bs != NULL; bs = bs->next)
     {
       decref_counted_command_line (&bs->commands);
-      bs->commands_left = NULL;
+
       if (bs->old_val != NULL)
 	{
 	  value_free (bs->old_val);
@@ -3231,6 +3231,16 @@ cleanup_executing_breakpoints (void *ignore)
   executing_breakpoint_commands = 0;
 }
 
+/* Return non-zero iff CMD as the first line of a command sequence is `silent'
+   or its equivalent.  */
+
+static int
+command_line_is_silent (struct command_line *cmd)
+{
+  return cmd && (strcmp ("silent", cmd->line) == 0
+		 || (xdb_commands && strcmp ("Q", cmd->line) == 0));
+}
+
 /* Execute all the commands associated with all the breakpoints at
    this location.  Any of these commands could cause the process to
    proceed beyond this point, etc.  We look out for such changes by
@@ -3279,10 +3289,13 @@ bpstat_do_actions_1 (bpstat *bsp)
          the tree when we're done.  */
       ccmd = bs->commands;
       bs->commands = NULL;
-      this_cmd_tree_chain
-	= make_cleanup_decref_counted_command_line (&ccmd);
-      cmd = bs->commands_left;
-      bs->commands_left = NULL;
+      this_cmd_tree_chain = make_cleanup_decref_counted_command_line (&ccmd);
+      cmd = ccmd ? ccmd->commands : NULL;
+      if (command_line_is_silent (cmd))
+	{
+	  /* The action has been already done by bpstat_stop_status.  */
+	  cmd = cmd->next;
+	}
 
       while (cmd != NULL)
 	{
@@ -3474,7 +3487,6 @@ bpstat_alloc (struct bp_location *bl, bpstat **bs_link_pointer)
   incref_bp_location (bl);
   /* If the condition is false, etc., don't do the commands.  */
   bs->commands = NULL;
-  bs->commands_left = NULL;
   bs->old_val = NULL;
   bs->print_it = print_it_normal;
   return bs;
@@ -4151,16 +4163,9 @@ bpstat_stop_status (struct address_space *aspace,
 		bs->print = 0;
 	      bs->commands = b->commands;
 	      incref_counted_command_line (bs->commands);
-	      bs->commands_left = bs->commands ? bs->commands->commands : NULL;
-	      if (bs->commands_left
-		  && (strcmp ("silent", bs->commands_left->line) == 0
-		      || (xdb_commands
-			  && strcmp ("Q",
-				     bs->commands_left->line) == 0)))
-		{
-		  bs->commands_left = bs->commands_left->next;
-		  bs->print = 0;
-		}
+	      if (command_line_is_silent (bs->commands
+					  ? bs->commands->commands : NULL))
+		bs->print = 0;
 	    }
 
 	  /* Print nothing for this entry if we don't stop or don't print.  */
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -950,10 +950,6 @@ struct bpstats
     /* The associated command list.  */
     struct counted_command_line *commands;
 
-    /* Commands left to be done.  This points somewhere in
-       base_command.  */
-    struct command_line *commands_left;
-
     /* Old value associated with a watchpoint.  */
     struct value *old_val;
 

Re: [patch] Code cleanup: Remove commands_left [Do not bpstat_clear_actions on throw_exception]

[Jan Kratochvil]

On Thu, 18 Aug 2011 21:04:40 +0200, Jan Kratochvil wrote:
> gdb/
> 2011-08-18  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Code cleanup.
> 	* breakpoint.c (bpstat_clear_actions): Remove clearing of commands_left.
> 	(command_line_is_silent): New function.
> 	(bpstat_do_actions_1): No longer use commands_left, use
> 	command_line_is_silent for commands.
> 	(bpstat_alloc): Remove clearing of commands_left.
> 	(bpstat_stop_status): Remove initialization of commands_left, use
> 	command_line_is_silent.
> 	* breakpoint.h (struct bpstats): Remove commands_left.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2011-08/msg00094.html


Thanks,
Jan

[patch 1/2] Code reshuffle for patch 2/2 [Re: [patch] Do not bpstat_clear_actions on throw_exception]

[Jan Kratochvil]

Hi,

just some code moving.  Not sure if bpstat_clear_actions name should stay the
same but why not.


Thanks,
Jan


gdb/
2011-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	No functionality change.
	* breakpoint.c (bpstat_clear_actions): Remove the BS parameter, make
	function comment a reference, new variables tp and bs, move here code
	from throw_exception.
	* breakpoint.h (bpstat_clear_actions): Remove the BS parameter,
	describe it in the comment.
	* exceptions.c (throw_exception): Remove variable tp, move the code for
	bpstat_clear_actions to bpstat_clear_actions.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3186,12 +3186,22 @@ bpstat_num (bpstat *bsp, int *num)
   return 1;
 }
 
-/* Modify BS so that the actions will not be performed.  */
+/* See breakpoint.h.  */
 
 void
-bpstat_clear_actions (bpstat bs)
+bpstat_clear_actions (void)
 {
-  for (; bs != NULL; bs = bs->next)
+  struct thread_info *tp;
+  bpstat bs;
+
+  if (ptid_equal (inferior_ptid, null_ptid))
+    return;
+
+  tp = find_thread_ptid (inferior_ptid);
+  if (tp == NULL)
+    return;
+
+  for (bs = tp->control.stop_bpstat; bs != NULL; bs = bs->next)
     {
       decref_counted_command_line (&bs->commands);
 
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -895,8 +895,9 @@ extern int bpstat_num (bpstat *, int *);
    command loop).  */
 extern void bpstat_do_actions (void);
 
-/* Modify BS so that the actions will not be performed.  */
-extern void bpstat_clear_actions (bpstat);
+/* Modify all entries of STOP_BPSTAT of INFERIOR_PTID so that the actions will
+   not be performed.  */
+extern void bpstat_clear_actions (void);
 
 /* Implementation:  */
 
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -207,21 +207,12 @@ 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);
-    }
+  bpstat_clear_actions ();
 
   do_cleanups (ALL_CLEANUPS);
 

Re: [patch 1/2] Code reshuffle for patch 2/2 [Re: [patch] Do not bpstat_clear_actions on throw_exception]

[Jan Kratochvil]

On Mon, 22 Aug 2011 16:51:43 +0200, Jan Kratochvil wrote:
> gdb/
> 2011-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	No functionality change.
> 	* breakpoint.c (bpstat_clear_actions): Remove the BS parameter, make
> 	function comment a reference, new variables tp and bs, move here code
> 	from throw_exception.
> 	* breakpoint.h (bpstat_clear_actions): Remove the BS parameter,
> 	describe it in the comment.
> 	* exceptions.c (throw_exception): Remove variable tp, move the code for
> 	bpstat_clear_actions to bpstat_clear_actions.

Checked in; it could be also considered as a code cleanup maybe, just to avoid
carrying it over for all the other patches being investigated.
	http://sourceware.org/ml/gdb-cvs/2011-08/msg00112.html


Thanks,
Jan

Re: [patch 1/2] Code reshuffle for patch 2/2 [Re: [patch] Do not bpstat_clear_actions on throw_exception]

[Pedro Alves]

On Friday 26 August 2011 10:29:12, Jan Kratochvil wrote:
> On Mon, 22 Aug 2011 16:51:43 +0200, Jan Kratochvil wrote:
> > gdb/
> > 2011-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	No functionality change.
> > 	* breakpoint.c (bpstat_clear_actions): Remove the BS parameter, make
> > 	function comment a reference, new variables tp and bs, move here code
> > 	from throw_exception.
> > 	* breakpoint.h (bpstat_clear_actions): Remove the BS parameter,
> > 	describe it in the comment.
> > 	* exceptions.c (throw_exception): Remove variable tp, move the code for
> > 	bpstat_clear_actions to bpstat_clear_actions.
> 
> Checked in; it could be also considered as a code cleanup maybe, just to avoid
> carrying it over for all the other patches being investigated.
> 	http://sourceware.org/ml/gdb-cvs/2011-08/msg00112.html

Thanks for doing this.

-- 
Pedro Alves