Re: terminate-on-error patch

[Michael Snyder <msnyder@redhat.com>]

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