Re: terminate-on-error patch

Michael Snyder
2005-08-22T18:32:14Z

Sean,

I think this is a great idea, and it seems like a good implementation.
Kudos on including both documentation and tests!

Anybody see any objection to this going in?

Michael

Sean Callanan wrote:
> At Apple, we have a set of scripts that peek around kernel data
> structures. One script does a stack dump for each thread activation in
> the kernel. Sometimes, the stack dumps run into invalid memory; right
> now, that terminates the script, which is bad, because we could safely
> continue on to the next activation.
>
> In general, it is desirable to have at least optional handling of
> errors by scripts rather than having the scripts simply terminate in
> all cases. I have a simple patch that adds this functionality to the
> GDB command line.
>
> When the test-and-set variable "terminate-on-error" is on (default),
> the behavior is the same as unmodified GDB. When it is off, a failed
> command does not terminate a script; rather, the $_error convenience
> variable is set to 1. Upon a successful command execution, the variable
> is set to 0.
>
> The way one would use this functionality in a script is as follows:
> --
> set terminate-on-error off
> [command]
> set $err = $_error
> set terminate-on-error on
>
> if($err)
> [handle errors]
> end
> --
> I recommend that this patch be added to mainline, because it enables
> new script functionality, making scripts that use it more robust,
> without changing the behavior of any existing scripts.
>
> Sincerely,
> Sean Callanan
>
> Index: gdb/wrapper.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/wrapper.h,v
> retrieving revision 1.14
> diff -c -r1.14 wrapper.h
> *** gdb/wrapper.h 16 Feb 2005 13:21:48 -0000 1.14
> --- gdb/wrapper.h 11 Aug 2005 19:42:17 -0000
> ***************
> *** 50,53 ****
> --- 50,56 ----
>
> extern int gdb_parse_and_eval_type (char *, int, struct type **);
>
> + /* Longjmp-safe wrapper for "execute_command". */
> + extern struct gdb_exception safe_execute_command (struct ui_out *,
> char *, int);
> +
> #endif /* wrapper.h */
> Index: gdb/cli/cli-interp.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/cli/cli-interp.c,v
> retrieving revision 1.10
> diff -c -r1.10 cli-interp.c
> *** gdb/cli/cli-interp.c 26 Apr 2005 05:03:39 -0000 1.10
> --- gdb/cli/cli-interp.c 11 Aug 2005 19:42:17 -0000
> ***************
> *** 33,41 ****
>
> /* These are the ui_out and the interpreter for the console
> interpreter. */
>
> - /* Longjmp-safe wrapper for "execute_command". */
> - static struct gdb_exception safe_execute_command (struct ui_out *uiout,
> - char *command, int from_tty);
> struct captured_execute_command_args
> {
> char *command;
> --- 33,38 ----
> ***************
> *** 122,128 ****
> execute_command (args->command, args->from_tty);
> }
>
> ! static struct gdb_exception
> safe_execute_command (struct ui_out *uiout, char *command, int from_tty)
> {
> struct gdb_exception e;
> --- 119,125 ----
> execute_command (args->command, args->from_tty);
> }
>
> ! struct gdb_exception
> safe_execute_command (struct ui_out *uiout, char *command, int from_tty)
> {
> struct gdb_exception e;
> Index: gdb/cli/cli-script.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
> retrieving revision 1.30
> diff -c -r1.30 cli-script.c
> *** gdb/cli/cli-script.c 26 Apr 2005 14:57:22 -0000 1.30
> --- gdb/cli/cli-script.c 11 Aug 2005 19:42:17 -0000
> ***************
> *** 33,38 ****
> --- 33,39 ----
> #include "cli/cli-cmds.h"
> #include "cli/cli-decode.h"
> #include "cli/cli-script.h"
> + #include "wrapper.h"
>
> /* Prototypes for local functions */
>
> ***************
> *** 48,53 ****
> --- 49,57 ----
> /* Level of control structure. */
> static int control_level;
>
> + /* Terminate scripts on errors, rather than setting _error = 1 */
> + static int terminate_on_error = 1;
> +
> /* Structure for arguments to user defined functions. */
> #define MAXUSERARGS 10
> struct user_args
> ***************
> *** 310,316 ****
> if (!new_line)
> break;
> make_cleanup (free_current_contents, &new_line);
> ! execute_command (new_line, 0);
> ret = cmd->control_type;
> break;
>
> --- 314,334 ----
> if (!new_line)
> break;
> make_cleanup (free_current_contents, &new_line);
> !
> ! if(terminate_on_error)
> ! {
> ! execute_command (new_line, 0);
> ! }
> ! else
> ! {
> ! struct gdb_exception rv;
> ! struct value* rv_val;
> !
> ! rv = safe_execute_command (uiout, new_line, 0);
> ! rv_val = value_from_longest (builtin_type_int, (rv.error ! =
> NO_ERROR));
> ! set_internalvar (lookup_internalvar ("_error"), rv_val);
> ! }
> !
> ret = cmd->control_type;
> break;
>
> ***************
> *** 1313,1315 ****
> --- 1331,1342 ----
> fputs_filtered ("\n", stream);
> }
>
> + void
> + _initialize_cli_script (void)
> + {
> + add_setshow_boolean_cmd("terminate-on-error", class_support,
> &terminate_on_error,
> + "Set termination of scripts on command
> errors.",
> + "Show script termination on command errors.",
> + "Controls how scripts respond to command
> errors: by terminating or setting $_error to 1.",
> + NULL, NULL, &setlist, &showlist);
> + }
> Index: gdb/doc/gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.276
> diff -c -r1.276 gdb.texinfo
> *** gdb/doc/gdb.texinfo 1 Aug 2005 18:31:28 -0000 1.276
> --- gdb/doc/gdb.texinfo 11 Aug 2005 19:42:18 -0000
> ***************
> *** 1330,1335 ****
> --- 1330,1336 ----
>
> @menu
> * Command Syntax:: How to give commands to @value{GDBN}
> + * Error Handling:: How to detect errors in commands
> * Completion:: Command completion
> * Help:: How to ask @value{GDBN} for help
> @end menu
> ***************
> *** 1385,1390 ****
> --- 1386,1423 ----
> then fetches the next line relative to the current line from the history
> for editing.
>
> + @node Error Handling
> + @section Error handling
> +
> + @cindex errors
> + @cindex error handling
> + @kindex @r{(terminate-on-error)}
> + Some @value{GDBN} commands cannot complete successfully, and must
> terminate.
> + A command prints an error string when it encounters an error, and the
> + @value{GDBN} script (if any) that invoked the command is terminated
> as well.
> +
> + In some scripts, this behavior is undesirable. You can tell GDB to
> set a
> + condition variable rather than terminate the script.
> +
> + @table @code
> + @item set terminate-on-error on
> + Tell @value{GDBN} to terminate a script when a command it invokes
> + encounters an error. This is the default.
> +
> + @item set terminate-on-error off
> + Do not terminate a script when a command it invokes encounters an error;
> + rather, set @code{$_error} to 1 and continue script execution. Commands
> + that terminate successfully set @code{$_error} to 0.
> +
> + @item show terminate-on-error
> + Show whether or not @value{GDBN} will terminate a script if it invokes
> + a command that encounters an error.
> + @end table
> +
> + Any command invocation will overwrite @code{$_error}; hence, you should
> + copy @code{$_error} into a temporary variable after invoking any command
> + whose output you want to check.
> +
> @node Completion
> @section Command completion
>
> Index: gdb/testsuite/gdb.base/Makefile.in
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/Makefile.in,v
> retrieving revision 1.4
> diff -c -r1.4 Makefile.in
> *** gdb/testsuite/gdb.base/Makefile.in 29 Jan 2004 11:15:44 -0000
> 1.4
> --- gdb/testsuite/gdb.base/Makefile.in 11 Aug 2005 19:42:22 -0000
> ***************
> *** 12,18 ****
> scope section_command setshow setvar shmain sigall signals \
> solib solib_sl so-impl-ld so-indr-cl \
> step-line step-test structs structs2 \
> ! twice-tmp varargs vforked-prog watchpoint whatis
>
> MISCELLANEOUS = coremmap.data ../foobar.baz \
> shr1.sl shr2.sl solib_sl.sl solib1.sl solib2.sl
> --- 12,18 ----
> scope section_command setshow setvar shmain sigall signals \
> solib solib_sl so-impl-ld so-indr-cl \
> step-line step-test structs structs2 \
> ! terminate-on-error twice-tmp varargs vforked-prog watchpoint whatis
>
> MISCELLANEOUS = coremmap.data ../foobar.baz \
> shr1.sl shr2.sl solib_sl.sl solib1.sl solib2.sl
> Index: gdb/testsuite/gdb.base/terminate-on-error.c
> ===================================================================
> RCS file: gdb/testsuite/gdb.base/terminate-on-error.c
> diff -N gdb/testsuite/gdb.base/terminate-on-error.c
> *** /dev/null 1 Jan 1970 00:00:00 -0000
> --- gdb/testsuite/gdb.base/terminate-on-error.c 11 Aug 2005 19:42:24
> -0000
> ***************
> *** 0 ****
> --- 1,6 ----
> + #include <stdio.h>
> +
> + int main(int argc, char** argv)
> + {
> + printf("Hello world!\n");
> + }
> Index: gdb/testsuite/gdb.base/terminate-on-error.exp
> ===================================================================
> RCS file: gdb/testsuite/gdb.base/terminate-on-error.exp
> diff -N gdb/testsuite/gdb.base/terminate-on-error.exp
> *** /dev/null 1 Jan 1970 00:00:00 -0000
> --- gdb/testsuite/gdb.base/terminate-on-error.exp 11 Aug 2005
> 19:42:24 -0000
> ***************
> *** 0 ****
> --- 1,72 ----
> + # Copyright 2001, 2003 Free Software Foundation, Inc.
> +
> + # This program is free software; you can redistribute it and/or modify
> + # it under the terms of the GNU General Public License as published by
> + # the Free Software Foundation; either version 2 of the License, or
> + # (at your option) any later version.
> + #
> + # This program is distributed in the hope that it will be useful,
> + # but WITHOUT ANY WARRANTY; without even the implied warranty of
> + # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + # GNU General Public License for more details.
> + #
> + # You should have received a copy of the GNU General Public License
> + # along with this program; if not, write to the Free Software
> + # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
> 02111-1307, USA. */
> +
> + # Please email any bugs, comments, and/or additions to this file to:
> + # bug-gdb@prep.ai.mit.edu
> + # use this to debug:
> + #
> + # log_user 1
> +
> + # terminate-on-error.exp -- Expect script to test proper error
> handling when
> + # terminate-on-error is disabled.
> +
> + if $tracelevel then {
> + strace $tracelevel
> + }
> +
> + set testfile terminate-on-error
> + set srcfile ${testfile}.c
> + set binfile ${objdir}/${subdir}/${testfile}
> +
> + remote_exec build "rm -f ${binfile}"
> + if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
> executable {debug}] != "" } {
> + gdb_suppress_entire_file "Testcase compile failed, so all tests
> in this file will automatically fail."
> + }
> +
> + gdb_exit
> + gdb_start
> + gdb_reinitialize_dir $srcdir/$subdir
> + gdb_load ${binfile}
> +
> + gdb_test "source ${srcdir}/${subdir}/terminate-on-error.gdb" \
> + "" \
> + "load test macro"
> +
> + gdb_test "show terminate-on-error" \
> + "Script termination on command errors is on\..*" \
> + "check default value of terminate-on-error"
> +
> + gdb_test "try" \
> + "Before.*evaluation of this expression requires the target
> program to be active" \
> + "verify script terminates when terminate-on-error is on"
> +
> + gdb_test "set terminate-on-error off" \
> + "" \
> + "turn terminate-on-error off"
> +
> + gdb_test "try" \
> + "Before.*evaluation of this expression requires the target
> program to be active.*After" \
> + "verify script does not terminate when terminate-on-error is
> off"
> +
> + gdb_test "print \$_error" \
> + ".*= 0" \
> + "verify that the last printf succeeded"
> +
> + gdb_test "print \$stored_error" \
> + ".*= 1" \
> + "verify that the print failed"
> +
> + return 0
> Index: gdb/testsuite/gdb.base/terminate-on-error.gdb
> ===================================================================
> RCS file: gdb/testsuite/gdb.base/terminate-on-error.gdb
> diff -N gdb/testsuite/gdb.base/terminate-on-error.gdb
> *** /dev/null 1 Jan 1970 00:00:00 -0000
> --- gdb/testsuite/gdb.base/terminate-on-error.gdb 11 Aug 2005
> 19:42:24 -0000
> ***************
> *** 0 ****
> --- 1,8 ----
> + set $stored_error = 0
> +
> + define try
> + printf "Before\n"
> + print "test\n"
> + set $stored_error = $_error
> + printf "After\n"
> + end
>
>