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

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Fri, 26 Aug 2011 13:45:32 +0200, Pedro Alves wrote:
> We're not terribly consistent in the printing stuff --- in some cases
> we end up showing that "<error reading variable: %s>", while in
> other cases we let the error escape, like in "p *0".
+
> I'd find it wrong that a command sequence continues
> blindly ignoring previous errors by default.

The problem is this "inconsistence" affects when the command sequences break
vs. when they continue.

What about save_gdb_index_command TRY_CATCH there?
TRY_CATCH + exception_fprintf
	Error while writing index for `objfile->name': except.message
This means the command succeeds even if some .gdb_index files could not be
produced.  I guess in this case there should have been rather final
throw_error so that save_gdb_index_command as whole throws one exception.


But it is a nice direction that any command with uncaught exception should
stop execution + clear all bpstats.  And any command that has all the
exceptions caught should continue execution + must not clear any bpstats.

Whatever violates these rules is a bug in the command implementation and not
in the GDB scripts execution control.


> I think the neatest fix would be to add some try/catch/finaly
> syntax to the cli.  There was a patch for that posted eons ago:
> 
> http://sourceware.org/ml/gdb-patches/2001-12/msg00449.html

I find this more as obsolete now, for advanced scripting there is already
Python or a control via MI by Perl, it does not make sense to further extend
the canned sequences of commands.


> I think most if not all those changes are actually bug fixes.
+
> I agree it's a bug.  The backtrace stopped gracefully, and completed,
> it didn't throw any error back to the interpreter.

I was considered any change to be a regression; great if it is the opposite.


> But we're so close to having this fixed!  :-(  Did you find some
> legitimate use case the patch breaks?

For example it changes the tracepoint example but it is great if it is
considered a bugfix.  It will also execute more bpstats now but therefore it
is not a bug but a feature.

In fact it also unifies the conditions under which bpstats gets clear and
canned commands sequence gets aborted.


> > (b) bpstat_clear_actions should also abort script_from_file.
> 
> Hmm?

Currently a caught exception:
Calls bpstat_clear_actions but it continues execution of script_from_file.
and uncaught exception:
Calls bpstat_clear_actions and also abort execution of script_from_file.

I was proposing that any exception - even the caught one should:
Call bpstat_clear_actions and also abort execution of script_from_file.

You are proposing and I am going to follow that a caught exception:
Does not call bpstat_clear_actions, it continues execution of script_from_file.
while an uncaught exception:
Call bpstat_clear_actions and also abort execution of script_from_file.

My goal was to unify bpstat_clear_actions and script_from_file abortion which
gets accomplished also with your proposal.


> > (c) There should be a new setting "set error-stops-script" with default "on"
> >     (backward compatibility) where "off" would disable bpstat_clear_actions
> >     completely.  And I will happily use "set error-stops-script off" in my
> >     ~/.gdbinit so that I can finally run `gdb -nx -x ./transcript.1'.
> 
> Some patch for something like that has been posted before too:
> 
>  <http://sourceware.org/ml/gdb-patches/2005-08/msg00120.html>

This simple patch looks cooked almost in acceptable state, I will hopefully
finish / check it in to make the testsuite more debuggable.


Thanks,
Jan