Re: RFC: block of commands

[Doug Evans <dje at google dot com>]

Philippe Waroquiers writes:
 > Ping ^ 3 ?
 >
 > On Sat, 2015-11-21 at 19:42 +0100, Philippe Waroquiers wrote:
 > > The included patch implements the concept of a 'block of commands'.
 > > A block of commands is one or more commands, separated by ;.
 > > A block of command starts with {.
 > > Block of commands can be included (nested) in block of commands.
 > > If nested blocks are used, each block must be terminated by }.
 > > {, } and ;  can be escaped with \ if needed.
 > >
 > > A block of command can optionally start with a control flag that
 > > controls the behaviour when a command of the block fails.
 > > The /c flag indicates to output the error and continue the block.
 > > The /s flag indicates to continue the block, without outputting the  
error.
 > >
 > > The block of commands can be used in the
 > >    'thread apply' command,
 > > to execute more than one command for each thread.
 > >
 > > The backtrace command has been modified to (optionally) accept a
 > > trailing block of commands.
 > >
 > > Command qualifiers /s have been added to backtrace and thread apply.
 > > /s qualifier indicates to only output a frame (or thread) info if
 > > the given block of commands (or command for thread apply) has produced
 > > some output.
 > >
 > > A block of command can be used in a user defined command, to avoid
 > > having a command failing to stop the execution of the user defined
 > > command.
 > >
 > > And finally, such a block can be used interactively, to group some
 > > commands and e.g. retrieve and execute a group from the history.
 > >
 > > Some example of usage:
 > >    bt {/c p i
 > >      => show all frames, show var i (if it exists), otherwise show an  
error)
 > >
 > >    bt {/s p i
 > >      => same, but do not show an error
 > >
 > >    bt /s {/s p i
 > >      => only show the frames where i can be succesfully printed
 > >
 > >
 > >    thread apply all bt {/s p i
 > >      => show a backtrace for all threads, print i in each frame (if it  
exists)
 > >
 > >    thread apply all /s bt /s {/s p i
 > >      => show only the threads and thread frames with a var i.
 > >
 > >    thread apply all { set $prevsp = $sp; bt \{ p $ebp - $prevsp\; set  
$prevsp = $sp \} }
 > >      => print the size of each frame
 > >
 > >    define robustshow
 > >    {/c print i }
 > >    {/c print j }
 > >    end
 > >       => define a command that will print i and/or j.
 > >
 > >
 > > The patch has been (somewhat) manually tested.
 > > Not yet done: tests, user manual, NEWS, ChangeLog.
 > >
 > > Comments welcome on the idea and/or the patch.

Heya.

One feature I've always wanted gdb's cli to have is the
ability to catch errors and be able to continue.
There's always a tension of whether to add things to the cli
or just say "do it in python". gdb scripts are still useful
and thus sometimes I think it's ok to extend the cli, such
as in this case.
Thus instead of /c in this patch I'd prefer something
along the lines of "try ... end".
["end" is gdb's canonical "trailing }", and I'm all for
Consistency Is Good here.]
The actual details and semantics of "try" I would
leave to a separate thread. For one, I'd have to think
about it some more :-), but I would support someone
wanting to go down that path.

OTOH, trying to extend the cli's syntax with { and ; is not
something I can support, at least not without more discussion.
This example:

    thread apply all { set $prevsp = $sp; bt \{ p $ebp - $prevsp\; set  
$prevsp = $sp \} }
      => print the size of each frame

suggests things could be better; having to escape braces within
subcommands feels excessively error prone.
gdb has one line per command, and changing that
may make things too unwieldly. There are still
macros so one could still do "thread apply all print-frame-size",
and put the guts of frame size computation in a macro named
print-frame-size.

Also, I'd prefer to have generally composable commands
over extending individual commands.
E.g., one common thing to do with "thread apply" is backtrace,
as in "thread apply all bt". I like that much better than
if we had extended backtrace to work on each thread.

Following "Consistency Is Good", and given that we have
"thread apply", for your "bt {/c p i" case how about
something like:

frame apply all foo

?

 > > diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
 > > index cab2336..615e543 100644
 > > --- a/gdb/cli/cli-decode.c
 > > +++ b/gdb/cli/cli-decode.c
 > > @@ -1255,6 +1255,11 @@ find_command_name_length (const char *text)
 > >    if (*p == '!')
 > >      return 1;
 > >
 > > +  /* Similarly, recognize '{' as a single character command so that
 > > +     a command block works as expected.  */
 > > +  if (*p == '{')
 > > +    return 1;
 > > +
 > >    while (isalnum (*p) || *p == '-' || *p == '_'
 > >  	 /* Characters used by TUI specific commands.  */
 > >  	 || *p == '+' || *p == '<' || *p == '>' || *p == '$')
 > > diff --git a/gdb/stack.c b/gdb/stack.c
 > > index b825bdf..d8d9791 100644
 > > --- a/gdb/stack.c
 > > +++ b/gdb/stack.c
 > > @@ -1712,10 +1712,14 @@ frame_info (char *addr_exp, int from_tty)
 > >  }
 > >
 > >  /* Print briefly all stack frames or just the innermost COUNT_EXP
 > > -   frames.  */
 > > +   frames.  If cmd != NULL, executes cmd in the context of each
 > > +   printed stack frame.  If silent, only print stack frames
 > > +   for which cmd has produced some output.  */
 > >
 > >  static void
 > > -backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
 > > +backtrace_command_1 (char *count_exp,
 > > +		     char *cmd, int silent,
 > > +		     int show_locals, int no_filters,
 > >  		     int from_tty)
 > >  {
 > >    struct frame_info *fi;
 > > @@ -1820,8 +1824,42 @@ backtrace_command_1 (char *count_exp, int  
show_locals, int no_filters,
 > >      {
 > >        for (i = 0, fi = trailing; fi && count--; i++, fi =  
get_prev_frame (fi))
 > >  	{
 > > +	  char *cmd_result = NULL;
 > > +
 > >  	  QUIT;
 > >
 > > +	  if (cmd != NULL)
 > > +	    {
 > > +	      char *saved_cmd;
 > > +	      struct frame_id frame_id = get_frame_id (fi);
 > > +
 > > +	      /* Save a copy of the command in case it is clobbered by
 > > +		 execute_command.  */
 > > +	      saved_cmd = xstrdup (cmd);
 > > +	      make_cleanup (xfree, saved_cmd);
 > > +
 > > +	      select_frame (fi);
 > > +	      cmd_result = execute_command_to_string (cmd, from_tty);
 > > +
 > > +	      /* Restore exact command used previously.  */
 > > +	      strcpy (cmd, saved_cmd);
 > > +
 > > +	      /* execute_command (might) invalidates FI.  */
 > > +	      fi = frame_find_by_id (frame_id);
 > > +	      if (fi == NULL)
 > > +		{
 > > +		  trailing = NULL;
 > > +		  warning (_("Unable to restore previously selected frame."));
 > > +		  break;
 > > +		}
 > > +	    }
 > > +
 > > +	  if (silent && (cmd_result == NULL || *cmd_result == 0))
 > > +	    {
 > > +	      xfree (cmd_result);
 > > +	      continue;
 > > +	    }
 > > +
 > >  	  /* Don't use print_stack_frame; if an error() occurs it probably
 > >  	     means further attempts to backtrace would fail (on the other
 > >  	     hand, perhaps the code does or could be fixed to make sure
 > > @@ -1843,6 +1881,13 @@ backtrace_command_1 (char *count_exp, int  
show_locals, int no_filters,
 > >  		  break;
 > >  		}
 > >  	    }
 > > +	  if (cmd_result != NULL)
 > > +	    {
 > > +	      if (*cmd_result != 0)
 > > +		printf_filtered ("%s", cmd_result);
 > > +	      xfree (cmd_result);
 > > +	    }
 > > +
 > >
 > >  	  /* Save the last frame to check for error conditions.  */
 > >  	  trailing = fi;
 > > @@ -1871,6 +1916,9 @@ backtrace_command (char *arg, int from_tty)
 > >  {
 > >    struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
 > >    int fulltrace_arg = -1, arglen = 0, argc = 0, no_filters  = -1;
 > > +  int cmd_arg = -1;
 > > +  char *cmd = NULL;
 > > +  int silent_arg = -1;
 > >    int user_arg = 0;
 > >
 > >    if (arg)
 > > @@ -1890,29 +1938,39 @@ backtrace_command (char *arg, int from_tty)
 > >
 > >  	  if (no_filters < 0 && subset_compare (argv[i], "no-filters"))
 > >  	    no_filters = argc;
 > > +	  else if (fulltrace_arg < 0 && subset_compare (argv[i], "full"))
 > > +	    fulltrace_arg = argc;
 > > +	  else if (silent_arg < 0 && subset_compare (argv[i], "/s"))
 > > +	    silent_arg = argc;
 > > +	  else if (cmd_arg < 0 && subset_compare ("{", argv[i]))
 > > +	     {
 > > +		char *block = strchr(arg, '{');
 > > +
 > > +		cmd_arg = argc;
 > > +		cmd = xmalloc (strlen (block) + 1);
 > > +		strcpy (cmd, block);
 > > +		make_cleanup (xfree, cmd);
 > > +		break;
 > > +	     }
 > >  	  else
 > >  	    {
 > > -	      if (fulltrace_arg < 0 && subset_compare (argv[i], "full"))
 > > -		fulltrace_arg = argc;
 > > -	      else
 > > -		{
 > > -		  user_arg++;
 > > -		  arglen += strlen (argv[i]);
 > > -		}
 > > +	      user_arg++;
 > > +	      arglen += strlen (argv[i]);
 > >  	    }
 > >  	  argc++;
 > >  	}
 > >        arglen += user_arg;
 > > -      if (fulltrace_arg >= 0 || no_filters >= 0)
 > > +      if (fulltrace_arg >= 0 || no_filters >= 0
 > > +	  || cmd_arg >= 0 || silent_arg > 0)
 > >  	{
 > >  	  if (arglen > 0)
 > >  	    {
 > >  	      arg = (char *) xmalloc (arglen + 1);
 > >  	      make_cleanup (xfree, arg);
 > >  	      arg[0] = 0;
 > > -	      for (i = 0; i < argc; i++)
 > > +	      for (i = 0; i < argc && i != cmd_arg; i++)
 > >  		{
 > > -		  if (i != fulltrace_arg && i != no_filters)
 > > +		  if (i != fulltrace_arg && i != no_filters && i != silent_arg)
 > >  		    {
 > >  		      strcat (arg, argv[i]);
 > >  		      strcat (arg, " ");
 > > @@ -1924,7 +1982,8 @@ backtrace_command (char *arg, int from_tty)
 > >  	}
 > >      }
 > >
 > > -  backtrace_command_1 (arg, fulltrace_arg >= 0 /* show_locals */,
 > > +  backtrace_command_1 (arg, cmd, silent_arg >= 0, /* silent */
 > > +		       fulltrace_arg >= 0 /* show_locals */,
 > >  		       no_filters >= 0 /* no frame-filters */, from_tty);
 > >
 > >    do_cleanups (old_chain);
 > > @@ -2600,7 +2659,12 @@ Print backtrace of all stack frames, or  
innermost COUNT frames.\n\
 > >  With a negative argument, print outermost -COUNT frames.\nUse of the \
 > >  'full' qualifier also prints the values of the local variables.\n\
 > >  Use of the 'no-filters' qualifier prohibits frame filters from  
executing\n\
 > > -on this backtrace.\n"));
 > > +on this backtrace.\n\
 > > +After the backtrace arguments and qualifiers, you can specify a block  
of commands.\n\
 > > +The block of commands will be executed for each each printed stack  
frame.\n\
 > > +Example:    backtrace -5 {/s print i; print j}\n\
 > > +will print the variables i and j (if they exists) in all printed  
stack frames.\n\
 > > +   use    help {    for the syntax of a block of command."));
 > >    add_com_alias ("bt", "backtrace", class_stack, 0);
 > >
 > >    add_com_alias ("where", "backtrace", class_alias, 0);
 > > diff --git a/gdb/thread.c b/gdb/thread.c
 > > index 6d410fb..8fdcfb8 100644
 > > --- a/gdb/thread.c
 > > +++ b/gdb/thread.c
 > > @@ -1596,6 +1596,7 @@ thread_apply_all_command (char *cmd, int  
from_tty)
 > >  {
 > >    struct cleanup *old_chain;
 > >    char *saved_cmd;
 > > +  int silent = 0;
 > >    int tc;
 > >    struct thread_array_cleanup ta_cleanup;
 > >
 > > @@ -1607,6 +1608,13 @@ thread_apply_all_command (char *cmd, int  
from_tty)
 > >        tp_array_compar_ascending = 1;
 > >      }
 > >
 > > +  if (cmd != NULL
 > > +      && check_for_argument (&cmd, "/s", strlen ("/s")))
 > > +    {
 > > +      cmd = skip_spaces (cmd);
 > > +      silent = 1;
 > > +    }
 > > +
 > >    if (cmd == NULL || *cmd == '\000')
 > >      error (_("Please specify a command following the thread ID  
list"));
 > >
 > > @@ -1652,11 +1660,23 @@ thread_apply_all_command (char *cmd, int  
from_tty)
 > >        for (k = 0; k != i; k++)
 > >          if (thread_alive (tp_array[k]))
 > >            {
 > > +	    char *cmd_result = NULL;
 > > +
 > >              switch_to_thread (tp_array[k]->ptid);
 > > -            printf_filtered (_("\nThread %d (%s):\n"),
 > > -			     tp_array[k]->num,
 > > -			     target_pid_to_str (inferior_ptid));
 > > -            execute_command (cmd, from_tty);
 > > +	    if (silent)
 > > +	      cmd_result = execute_command_to_string (cmd, from_tty);
 > > +	    if (!silent || (cmd_result != NULL && *cmd_result != 0))
 > > +	      printf_filtered (_("\nThread %d (%s):\n"),
 > > +			       tp_array[k]->num,
 > > +			       target_pid_to_str (inferior_ptid));
 > > +	    if (cmd_result)
 > > +	      {
 > > +		if (*cmd_result != 0)
 > > +		  printf_filtered ("%s", cmd_result);
 > > +		xfree (cmd_result);
 > > +	      }
 > > +	    else
 > > +	      execute_command (cmd, from_tty);
 > >
 > >              /* Restore exact command used previously.  */
 > >              strcpy (cmd, saved_cmd);
 > > @@ -1677,7 +1697,9 @@ thread_apply_command (char *tidlist, int  
from_tty)
 > >    if (tidlist == NULL || *tidlist == '\000')
 > >      error (_("Please specify a thread ID list"));
 > >
 > > -  for (cmd = tidlist; *cmd != '\000' && !isalpha (*cmd); cmd++);
 > > +  for (cmd = tidlist;
 > > +       *cmd != '\000' && !isalpha (*cmd) && *cmd != '{';
 > > +       cmd++);
 > >
 > >    if (*cmd == '\000')
 > >      error (_("Please specify a command following the thread ID  
list"));
 > > diff --git a/gdb/top.c b/gdb/top.c
 > > index d1e2271..b553028 100644
 > > --- a/gdb/top.c
 > > +++ b/gdb/top.c
 > > @@ -534,6 +534,151 @@ execute_command_to_string (char *p, int from_tty)
 > >    return retval;
 > >  }
 > >
 > > +static void
 > > +execute_one_block_command (char *cmd, int from_tty,
 > > +			   int continue_on_failure, int silent_failure)
 > > +{
 > > +  if (continue_on_failure)
 > > +    {
 > > +      TRY
 > > +	{
 > > +	  execute_command (cmd, from_tty);
 > > +	}
 > > +      CATCH (e, RETURN_MASK_ERROR)
 > > +	{
 > > +	  if (!silent_failure)
 > > +	    exception_print (gdb_stderr, e);
 > > +	}
 > > +      END_CATCH
 > > +	}
 > > +  else
 > > +    execute_command (cmd, from_tty);
 > > +}
 > > +
 > > +/* Parses in cmds optional /CONTROL, then executes each command part
 > > +   of the block. Returns the position in cmds after the block end.  */
 > > +static void
 > > +commands_block_command (char *cmds, int from_tty)
 > > +{
 > > +  int continue_on_failure = 0;
 > > +  int silent_failure = 0;
 > > +  int level = 0;
 > > +  char *cmd_end = NULL;
 > > +
 > > +#if 0
 > > +#define XX(msg) printf("%s cmds%s <%s> %p cmd_end <%s> %p\n", \
 > > +		       msg,					\
 > > +		       silent_failure ? "/s" :			\
 > > +		       continue_on_failure ? "/c" : "",		\
 > > +		       cmds, cmds, cmd_end, cmd_end)
 > > +#else
 > > +#define XX(msg)
 > > +#endif
 > > +
 > > +  cmds = skip_spaces (cmds);
 > > +
 > > +  if (cmds == NULL || *cmds == 0)
 > > +    error (_("Please specify one or more commands separated by ;,\n"
 > > +	     "optionally followed by a block end }"));
 > > +
 > > +  /* Parses the optional /CONTROL.  */
 > > +  if (*cmds == '/')
 > > +    {
 > > +      cmds++;
 > > +      while (*cmds) {
 > > +	if (*cmds == ' ')
 > > +	  break;
 > > +	else if (*cmds == 's')
 > > +	  continue_on_failure = silent_failure = 1;
 > > +	else if (*cmds == 'c')
 > > +	  continue_on_failure = 1;
 > > +	else
 > > +	  error (_("Invalid commands block control flag %c"), *cmds);
 > > +	cmds++;
 > > +      }
 > > +    }
 > > +
 > > +  cmds = skip_spaces (cmds);
 > > +  cmd_end = cmds;
 > > +  level = 0;
 > > +  XX("enter");
 > > +  while (*cmd_end)
 > > +    {
 > > +      if (*cmd_end == '\\'
 > > +	  && (*(cmd_end+1) == '{'
 > > +	      || *(cmd_end+1) == '}'
 > > +	      || *(cmd_end+1) == ';'))
 > > +	{
 > > +	  char *p, *q;
 > > +
 > > +	  p = cmd_end;
 > > +	  q = cmd_end+1;
 > > +	  while (*q)
 > > +	    *p++ = *q++;
 > > +	  cmd_end++;
 > > +	}
 > > +      else if (*cmd_end == '{')
 > > +	{
 > > +	  /* Found the begin of a nested block.  */
 > > +
 > > +	  /* Execute last command of including block, if any.  */
 > > +	  if (cmds <= cmd_end)
 > > +	    {
 > > +	      *cmd_end = 0;
 > > +	      XX("prev block lastcmd");
 > > +	      execute_one_block_command (cmds, from_tty,
 > > +					 continue_on_failure, silent_failure);
 > > +	      *cmd_end = '{';
 > > +	    }
 > > +
 > > +	  /* Search for the nested block end: either a } or end of cmds.  */
 > > +	  cmds = cmd_end;
 > > +	  while (*cmd_end) {
 > > +	    if (*cmd_end == '{')
 > > +	      level++;
 > > +	    else if (*cmd_end == '}')
 > > +	      level--;
 > > +	    if (level == 0)
 > > +	      break;
 > > +	    cmd_end++;
 > > +	  }
 > > +
 > > +	  /* recursively executes the block.  */
 > > +	  *cmd_end = 0;
 > > +	  XX("block");
 > > +	  execute_one_block_command (cmds, from_tty,
 > > +				     continue_on_failure, silent_failure);
 > > +	  if (level == 0)
 > > +	    cmd_end++;
 > > +	  cmds = cmd_end;
 > > +	}
 > > +      else if ((*cmd_end == ';' || *cmd_end == '}') && level == 0)
 > > +	{
 > > +	  /*  When encountering a command terminator or a block end,
 > > +	      executes this command.  Note : the block end comparison
 > > +	      is needed to execute the last command of a block, if
 > > +	      this command is not terminated by ;.  */
 > > +	  *cmd_end = 0;
 > > +	  XX("cmd");
 > > +	  execute_one_block_command (cmds, from_tty,
 > > +				     continue_on_failure, silent_failure);
 > > +	  cmd_end++;
 > > +	  cmds = cmd_end;
 > > +	}
 > > +      else
 > > +	cmd_end++;
 > > +    }
 > > +
 > > +  /* execute last command of this block, if any */
 > > +  if (cmds <= cmd_end && *cmds)
 > > +    {
 > > +      XX("lastcmd");
 > > +      execute_one_block_command (cmds, from_tty,
 > > +				 continue_on_failure, silent_failure);
 > > +    }
 > > +#undef XX
 > > +}
 > > +
 > >  /* Read commands from `instream' and execute them
 > >     until end of file or error reading instream.  */
 > >
 > > @@ -1942,6 +2087,17 @@ Don't repeat this command.\nPrimarily \
 > >  used inside of user-defined commands that should not be repeated  
when\n\
 > >  hitting return."));
 > >
 > > +  add_com ("{", class_support,
 > > +	   commands_block_command, _("\
 > > +Executes a block of commands : {/CONTROL CMD1; CMD2; ... }.\n\
 > > +Commands are separated by the character ';'.\n\
 > > +Nested blocks can be terminated by the character '}'.\n\
 > > +By default, the execution of a block stops if a CMD fails.\n\
 > > +/CONTROL allows to control the execution in case of a CMD failure:\n\
 > > +  /c indicates to report the error and continue the block  
execution.\n\
 > > +  /s indicates to silently continue the block execution.\n\
 > > +You can use '\\' to escape '{', '}' and ';'."));
 > > +
 > >    add_setshow_boolean_cmd ("editing", class_support,
 > >  			   &async_command_editing_p, _("\
 > >  Set editing of command lines as they are typed."), _("\
 > >
 > >
 > >
 > >
 >
 >

--
/dje