Re: [patch] Do not bpstat_clear_actions on throw_exception

[Pedro Alves <pedro@codesourcery.com>]

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