[RFC,2/2] Allow and evaluate expressions in command arguments

  This patch adds support for arbitrary expressions to be passed to
GDB commands, evaluated, and the results substituted into command
arguments.

This allows users to pass arbitrary expressions into commands which
currently do not support any way of doing this. This is especially
useful with, for example, convenience variables and functions.

Example:
$ export MYDIR="tmp"
$ export MYFILE="simple"
$ gdb -nx -q
(gdb) file $($_env("HOME"))/$($_env("MYDIR"))/$($_env("MYFILE"))
Reading symbols from /home/keiths/tmp/simple...
(gdb)

In order to mark the bounds of expressions to be evaluated, I've
introduced the (arbitrary) marker "$()". Everything contained inside
this marker will be passed to the expression parser, evaluated, printed
as a string, and then substituted for the original expression.
---
 gdb/top.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)
  

Keith Seitz via Gdb-patches <gdb-patches@sourceware.org> writes:

> This patch adds support for arbitrary expressions to be passed to
> GDB commands, evaluated, and the results substituted into command
> arguments.
>
> This allows users to pass arbitrary expressions into commands which
> currently do not support any way of doing this. This is especially
> useful with, for example, convenience variables and functions.
>
> Example:
> $ export MYDIR="tmp"
> $ export MYFILE="simple"
> $ gdb -nx -q
> (gdb) file $($_env("HOME"))/$($_env("MYDIR"))/$($_env("MYFILE"))
> Reading symbols from /home/keiths/tmp/simple...
> (gdb)
>
> In order to mark the bounds of expressions to be evaluated, I've
> introduced the (arbitrary) marker "$()". Everything contained inside
> this marker will be passed to the expression parser, evaluated, printed
> as a string, and then substituted for the original expression.

I really like the goal of this series, and think something like this
would be awesome to add.

I do worry about the extra $(..) wrapper though, that feels a little
clunky, if the common case turns out to be just doing simple things like
environment variables, I wonder if we could make things like:

  (gdb) file $_env("HOME")/$_env("BLAH")

just work?

We could still retain $(...) for full expression evaluation, because I
guess, in theory this would allow things like:

  (gdb) some_gdb_command $(some_inferior_function())

thought I suspect things like this will be far less common ... maybe?

Thanks,
Andrew

> ---
>  gdb/top.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>
> diff --git a/gdb/top.c b/gdb/top.c
> index 205eb360ba3..3f394b5b63c 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -57,6 +57,8 @@
>  #include "gdbsupport/pathstuff.h"
>  #include "cli/cli-style.h"
>  #include "pager.h"
> +#include "valprint.h"
> +#include "cp-support.h" 	/* for find_toplevel_char  */
>  
>  /* readline include files.  */
>  #include "readline/readline.h"
> @@ -567,6 +569,60 @@ set_repeat_arguments (const char *args)
>    repeat_arguments = args;
>  }
>  
> +/* Evaluate and expand any expressions in the command line list of arguments
> +   given by ORIG_ARGS.  All occurrences of "$(expr)" will be replaced with a
> +   string representation of the evaluated EXPR.  */
> +
> +#define EVAL_START_STRING "$("
> +#define EVAL_END_TOKEN ')'
> +
> +static std::string
> +expand_command_arg (const char *orig_arg)
> +{
> +  std::string new_arg = orig_arg;
> +  std::string::size_type n = new_arg.find (EVAL_START_STRING);
> +
> +  /* These needs adjustment to output "simple" strings.  For example,
> +     if a char value is used, we end up with "97 'a'" instead of simply "a".  */
> +  struct value_print_options opts;
> +  get_user_print_options (&opts);
> +
> +  string_file stb;
> +
> +  while (n != std::string::npos)
> +    {
> +      const char *args = new_arg.c_str ();
> +      const char *c = find_toplevel_char (args + n + 2, EVAL_END_TOKEN);
> +
> +      if (c != nullptr)
> +	{
> +	  std::string expr = new_arg.substr (n + 2, c - args - n - 2);
> +	  struct value *val = parse_and_eval (expr.c_str ());
> +
> +	  value_print (val, &stb, &opts);
> +
> +	  /* If value_print returns a quote-enclosed string, remove the
> +	     quote characters.  */
> +	  std::string r = stb.release ();
> +
> +	  if (*r.begin () == '\"' && *r.rbegin () == '\"')
> +	    {
> +	      r.erase (r.begin ());
> +	      r.pop_back ();
> +	    }
> +
> +	  new_arg.replace (n, c - args - n + 1, r);
> +	  n = new_arg.find (EVAL_START_STRING);
> +	}
> +    }
> +
> +#if 0
> +  if (new_arg != orig_arg)
> +    printf ("NEW_ARG = %s\n", new_arg.c_str ());
> +#endif
> +  return new_arg;
> +}
> +
>  /* Execute the line P as a command, in the current user context.
>     Pass FROM_TTY as second argument to the defining function.  */
>  
> @@ -657,6 +713,15 @@ execute_command (const char *p, int from_tty)
>        if (c->deprecated_warn_user)
>  	deprecated_cmd_warning (line, cmdlist);
>  
> +      /* Do any expression substitutions on ARG.  */
> +      std::string expanded_arg;
> +
> +      if (arg != nullptr)
> +	{
> +	  expanded_arg = expand_command_arg (arg);
> +	  arg = expanded_arg.c_str ();
> +	}
> +
>        /* c->user_commands would be NULL in the case of a python command.  */
>        if (c->theclass == class_user && c->user_commands)
>  	execute_user_command (c, arg);
> -- 
> 2.38.1
  

On 2/3/23 09:22, Andrew Burgess wrote:
> Keith Seitz via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> In order to mark the bounds of expressions to be evaluated, I've
>> introduced the (arbitrary) marker "$()". Everything contained inside
>> this marker will be passed to the expression parser, evaluated, printed
>> as a string, and then substituted for the original expression.
> 
> I really like the goal of this series, and think something like this
> would be awesome to add.
> 
> I do worry about the extra $(..) wrapper though, that feels a little
> clunky, if the common case turns out to be just doing simple things like
> environment variables, I wonder if we could make things like:
> 
>    (gdb) file $_env("HOME")/$_env("BLAH")
> 
> just work?

I agree. That $() marker *is* cumbersome.

> We could still retain $(...) for full expression evaluation, because I
> guess, in theory this would allow things like:
> 
>    (gdb) some_gdb_command $(some_inferior_function())
> 
> thought I suspect things like this will be far less common ... maybe?

That's another good idea. We could explicitly permit:

1. $(<any valid expression>): Evaluate ANY arbitrary expression
2. $VAR_OR_FUNC: Evaluate /only/ a convenience variable or expression.
    Convenience funcs are easier than vars, but I think this can be made
    to work.

As you note, if using just a convenience variable or function is insufficient,
users could fallback to the more clumsy $() notation.

Thank you for the input! I will work on completing this patch
for formal submission.

Keith
  

Hi Keith!

On 2023-01-25 7:38 p.m., Keith Seitz via Gdb-patches wrote:
> This patch adds support for arbitrary expressions to be passed to
> GDB commands, evaluated, and the results substituted into command
> arguments.
> 
> This allows users to pass arbitrary expressions into commands which
> currently do not support any way of doing this. This is especially
> useful with, for example, convenience variables and functions.
> 
> Example:
> $ export MYDIR="tmp"
> $ export MYFILE="simple"
> $ gdb -nx -q
> (gdb) file $($_env("HOME"))/$($_env("MYDIR"))/$($_env("MYFILE"))
> Reading symbols from /home/keiths/tmp/simple...
> (gdb)
> 
> In order to mark the bounds of expressions to be evaluated, I've
> introduced the (arbitrary) marker "$()". Everything contained inside
> this marker will be passed to the expression parser, evaluated, printed
> as a string, and then substituted for the original expression.

I'm afraid I think we can't take this in this form.  The reason is that CLI commands
don't have a common parser, every command parses things its own way.  And then
we have commands which take language expressions.  So there's substantial risk
that whatever syntax we come up with, conflicts with syntax used by either some
command, or some language.

This is a similar reason why we ended up with a "pipe" command instead of teaching
the command parser about the pipe character itself. 

I think an approach like that would work.

So you'd add a new prefix command whose job is to do some expansion, and
then execute the resulting command.

"eval" is actually similar.  Or maybe exactly it?  Maybe we don't need a new command,
but we add enough internal functions so that you can use "eval" for this.

So if we have your $_env convenience function, we should be able to write:

(gdb) eval "file %s/%s/%s", $_env("HOME"), $_env("MYDIR"), $_env("MYFILE")

If that doesn't work, then I think we should fix it.

Pedro Alves

> ---
>  gdb/top.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/gdb/top.c b/gdb/top.c
> index 205eb360ba3..3f394b5b63c 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -57,6 +57,8 @@
>  #include "gdbsupport/pathstuff.h"
>  #include "cli/cli-style.h"
>  #include "pager.h"
> +#include "valprint.h"
> +#include "cp-support.h" 	/* for find_toplevel_char  */
>  
>  /* readline include files.  */
>  #include "readline/readline.h"
> @@ -567,6 +569,60 @@ set_repeat_arguments (const char *args)
>    repeat_arguments = args;
>  }
>  
> +/* Evaluate and expand any expressions in the command line list of arguments
> +   given by ORIG_ARGS.  All occurrences of "$(expr)" will be replaced with a
> +   string representation of the evaluated EXPR.  */
> +
> +#define EVAL_START_STRING "$("
> +#define EVAL_END_TOKEN ')'
> +
> +static std::string
> +expand_command_arg (const char *orig_arg)
> +{
> +  std::string new_arg = orig_arg;
> +  std::string::size_type n = new_arg.find (EVAL_START_STRING);
> +
> +  /* These needs adjustment to output "simple" strings.  For example,
> +     if a char value is used, we end up with "97 'a'" instead of simply "a".  */
> +  struct value_print_options opts;
> +  get_user_print_options (&opts);
> +
> +  string_file stb;
> +
> +  while (n != std::string::npos)
> +    {
> +      const char *args = new_arg.c_str ();
> +      const char *c = find_toplevel_char (args + n + 2, EVAL_END_TOKEN);
> +
> +      if (c != nullptr)
> +	{
> +	  std::string expr = new_arg.substr (n + 2, c - args - n - 2);
> +	  struct value *val = parse_and_eval (expr.c_str ());
> +
> +	  value_print (val, &stb, &opts);
> +
> +	  /* If value_print returns a quote-enclosed string, remove the
> +	     quote characters.  */
> +	  std::string r = stb.release ();
> +
> +	  if (*r.begin () == '\"' && *r.rbegin () == '\"')
> +	    {
> +	      r.erase (r.begin ());
> +	      r.pop_back ();
> +	    }
> +
> +	  new_arg.replace (n, c - args - n + 1, r);
> +	  n = new_arg.find (EVAL_START_STRING);
> +	}
> +    }
> +
> +#if 0
> +  if (new_arg != orig_arg)
> +    printf ("NEW_ARG = %s\n", new_arg.c_str ());
> +#endif
> +  return new_arg;
> +}
> +
>  /* Execute the line P as a command, in the current user context.
>     Pass FROM_TTY as second argument to the defining function.  */
>  
> @@ -657,6 +713,15 @@ execute_command (const char *p, int from_tty)
>        if (c->deprecated_warn_user)
>  	deprecated_cmd_warning (line, cmdlist);
>  
> +      /* Do any expression substitutions on ARG.  */
> +      std::string expanded_arg;
> +
> +      if (arg != nullptr)
> +	{
> +	  expanded_arg = expand_command_arg (arg);
> +	  arg = expanded_arg.c_str ();
> +	}
> +
>        /* c->user_commands would be NULL in the case of a python command.  */
>        if (c->theclass == class_user && c->user_commands)
>  	execute_user_command (c, arg);
>