Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* RFC:  block of commands
@ 2015-11-21 18:41 Philippe Waroquiers
  2015-12-03 22:28 ` Philippe Waroquiers
  2016-01-04 19:28 ` Philippe Waroquiers
  0 siblings, 2 replies; 10+ messages in thread
From: Philippe Waroquiers @ 2015-11-21 18:41 UTC (permalink / raw)
  To: gdb-patches

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.

Thanks

Philippe

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."), _("\






^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: RFC: block of commands
@ 2016-01-11 21:40 Doug Evans
  2016-01-12 20:52 ` Philippe Waroquiers
  0 siblings, 1 reply; 10+ messages in thread
From: Doug Evans @ 2016-01-11 21:40 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: gdb-patches

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


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: RFC: block of commands
@ 2016-01-21  1:29 Doug Evans
  2016-01-22 21:47 ` Philippe Waroquiers
  0 siblings, 1 reply; 10+ messages in thread
From: Doug Evans @ 2016-01-21  1:29 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: gdb-patches

Philippe Waroquiers writes:
  > On Mon, 2016-01-11 at 21:40 +0000, Doug Evans wrote:
  > > 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.
  > The { (and have the terminating } optional) was chosen in order
  > to make the interactive typing of a block of command faster/easier.
  > I agree that this new syntax is very different of the
  > 'usual gdb multi-lines terminated by end'.
  >
  > However, multi-lines with end means the user cannot simply type e.g.
  >    thread apply all { p i; p j}
  > Instead, the user must type:
  >    define pij
  >    p i
  >    p j
  >    end
  >   thread apply all pij
  > And after, if the user wants to also print k,
  > then the pij sequence must be retyped (I do not know a way
  > to change/edit/complete a user defined command).

Yeah. I recognize the difficulty.
But adding {...; ...} on top of what's there now
is going to be a slippery slope I fear.
[maybe not, but we're adding syntax to what
has until now been pretty free form]
I want to make sure that if we add this,
we don't some day end up regretting it.

Note that at least within emacs I can repeat/edit
multi-line commands by going back through the history.
Plus one can redefine user-defined commands
from the keyboard. So it's possible, if tedious.

  > An alternative might be to make the terminating } mandatory.
  > Then I think we could get rid of the { and } escaping

I'd prefer to be strict (e.g., require }), at least for now.
Once we add { ...; ... }, someone is going to want to extend it,
and then we've got if/while/etc. that can all be done as one-liners.
[If we're going to make interactive use easier, why can't
if/while/etc. be done as one-liners? It feels like if a
compelling argument can't be made to allow them to be one-liners,
then supporting one-liners in a more restricted fashion
isn't very compelling and maybe we should just punt.]
Let's think this through.

  > So, the question is how much we want to make the block of commands
  > easy to use interactively ?
  >
  > Maybe it is not that unusual to have 2 concepts
  > of 'list of commands' :
  >     one command per line, with some 'begin/end' keywords
  >    several commands per lines, separated by ;
  > (i.e. similar to what a shell is providing with if/fi and ;).
  >
  > The ; separated list versus multi-lines is clearly the main
  > point of discussion.
  >
  > Note that if ever the {}; idea survives, we have to take care
  > about parsing of existing scripts that would do e.g.
  >    echo { is an open bracket
  > Not too sure it will be easy to make the {}; idea fully backward
  > compatible :(

That's going to be a tricky part, yeah.

Existing "echo {" may not be so bad, as echo doesn't take a command
as an argument. While less than ideal, we could defer parsing
{ ... } until we're parsing a command that takes a command as
an argument (either literally, as in "thread apply ...",
or conceptually, as in "if expr ...").
Of course, extending "if/while" like this would be problematic if
a language allowed { in an expression.

Another thought is: If we extend this to if/while/etc.
then it'd be odd to not be able to terminate a multi-line "if"
with }, except that there is no opening { in a multi-line "if".
So does that mean { ... } is restricted to one line?
Probably, but it may trip people up from time to time.

Another thought: What does this do?

if a == b { echo foo }

If we don't enforce a trailing } then is that "echo foo" or "echo foo }"?
Maybe a good example to suggest we should indeed require a trailing }?

Similarly with ;

if a == b { echo foo; }

These questions apply even if we don't extend "if":

thread apply all { echo foo; }

  > > 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.
  > True, but that makes the interactive use and editing
  > not so easy (cfr above).

Yep.
This is a significant addition,
I'm just asking for a thorough vetting of it.

Deciding how to parse embedded {/;/} is probably
the important part (as you've mentioned).

E.g., if a == b { if c == d { echo foo; }; echo bar }

or

thread apply all { frame apply all { echo \{foo\}; printf "\{foo\}"; } }

We have to find the outer } so we know what command to
execute for each thread, but in order to do that we
have to do some parsing of the inner command in order
to handle the nesting.
Plus, as you've mentioned, we'll need some kind of escaping;
when we run the "frame apply all" command is it:
echo {foo}; printf "\{foo\}"
?

Since this is new syntax, we're free to define things
however we like, within reason.

  > > 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
  > Yes, that looks better than adding an optional command to
  > backtrace.


^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-01-24 22:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-21 18:41 RFC: block of commands Philippe Waroquiers
2015-12-03 22:28 ` Philippe Waroquiers
2015-12-21 13:35   ` Philippe Waroquiers
2016-01-04 19:28 ` Philippe Waroquiers
2016-01-11 21:40 Doug Evans
2016-01-12 20:52 ` Philippe Waroquiers
2016-01-21  1:29 Doug Evans
2016-01-22 21:47 ` Philippe Waroquiers
2016-01-24 20:03   ` Doug Evans
2016-01-24 22:23     ` Philippe Waroquiers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox