[PATCH] gdb: allow env var specifications in cmds 'set log', 'source', 'shell'

[PATCH] gdb: allow env var specifications in cmds 'set log', 'source', 'shell'

[Peikes, Wendy]

Rationale:

We at NetApp added a gdb feature allowing users to specify environment variable names when invoking gdb commands 'set logging file', 'source' & 'shell'.

This has been a big help especially in gdb macros since it allows macro writers to customize macro
behavior with environment var settings.

We've implemented this feature for the above three gdb commands but it can easily be  extended to other commands such as 'set substitute-path'

Usage example below.

setenv LOG_DIR /tmp
setenv SCRIPT_DIR "."

I've created a script 'macroScript' in the current directory ("."); it is referenced in the gdb session.

Now, run the gdb with the new env var feature:

gdbWithEnvVarFeature
....
GNU gdb (GDB) 9.1
Copyright (C) 2020 Free Software Foundation, Inc.
...

(gdb) shell head ${SCRIPT_DIR}/macroScript
define macro1
       echo macro1\n
end
define macro2
       echo macro2\n
end
macro1
macro2


gdb) source ${SCRIPT_DIR}/macroScript
macro1
macro2
(gdb) set logging file ${LOG_DIR}/logFile
(gdb) show logging file
The current logfile is "${LOG_DIR}/logFile".
(gdb) set logging on
Copying output to /tmp/logFile.
Copying debug output to /tmp/logFile.

(gdb) help set
Evaluate expression EXP and assign result to variable VAR.
Usage: set VAR = EXP
This uses assignment syntax appropriate for the current language
(VAR = EXP or VAR := EXP for example)
...
set architecture -- Set architecture of target.
...

(gdb) set var $i = 1

(gdb) printf "i:%d\n", $i
i:1
(gdb) shell tail ${LOG_DIR}/logFile
set watchdog -- Set watchdog timer.
set width -- Set number of characters where GDB should wrap lines of its output.
set write -- Set writing into executable and core files.
....
Command name abbreviations are allowed if unambiguous.
...
i:1

The patch to gdb9.1 is here; the changed lines are bracket by
#if defined ALLOW_ENV_VARS
#endif

patch:

*** fsf/cli-cmds.c      2022-09-22 15:59:17.238371000 -0700
--- modifiedFsf/cli-cmds.c      2022-09-22 16:29:59.506242000 -0700
***************
*** 660,665 ****
--- 660,744 ----
    script_from_file (stream, file);
  }

+ #define ALLOW_ENV_VARS 1
+ #if defined ALLOW_ENV_VARS
+ /* return true if s uses one or more env vars, specified as ${VARNAME} */
+ extern int
+ contains_env_var(const char *s0)
+ {
+   return s0 && (strlen (s0) > 1) && (strstr(s0, "${") != NULL) &&
+         (strchr(s0, '}') != NULL);
+ }
+
+ /* substitute_env_var - given a string, return a copy of the string with
+    env vars interpolated (if any).  env vars are recognized by the
+    syntax ...${VARNAME}...
+ */
+
+
+ extern char *
+ substitute_env_var(const char *s0, const char* msg, const int from_tty)
+ {
+     int slen, i;
+
+     char *s = xstrdup(s0);
+     slen = strlen(s);
+
+     for (i=0; i<slen; i++)
+       {
+       // find the lead-in for an environment variable
+       if ((s[i] == '$') && (s[i+1] == '{'))
+         {
+           char *endbrace = index(s+i+2, '}');
+           if (endbrace == NULL)
+             {
+               break;
+             }
+           else
+             {
+               int e = endbrace - s;
+
+               //    ...${X}...
+               //       ^  ^
+               //       i  e
+
+               int varname_len = e-(i+2);
+               char *varname = xstrndup(s+i+2, varname_len);
+               char *var_value = getenv(varname);
+               if (var_value == NULL)
+                 {
+                   if (from_tty)
+                   {
+                     fprintf_unfiltered
+                       (gdb_stderr,
+                        "Env var '%s' not set but used in '%s'.\n", varname, s0);
+                   }
+                   var_value = (char*)"";
+                 }
+               xfree(varname);
+               {
+                 // resize s to hold the substitution of varname
+                 int delta = strlen(var_value) - (e-i+1);
+
+                 // new_s may be shorter than s
+                 char *new_s = (char*) xmalloc(slen+delta+1);
+                 memcpy(new_s, s, i);
+                 memcpy(new_s+i, var_value, strlen(var_value));
+                 memmove(new_s+e+1+delta, s+e+1, slen-(e+1)+1);  // include \0
+
+                 // Skip ahead to new string at pos right after replacement so
+                 // we don't try recursive replacement so avoid infinite loops.
+                 i += strlen(var_value) - 1;  // -1 to offset i++ in for()
+                 slen += delta;
+                 xfree(s);
+                 s = new_s;
+               }
+             }
+         }
+       }
+     return s;
+ }
+ #endif // ALLOW_ENV_VARS
+
  /* Worker to perform the "source" command.
     Load script FILE.
     If SEARCH_PATH is non-zero, and the file isn't found in cwd,
***************
*** 672,677 ****
--- 751,764 ----
    if (file == NULL || *file == 0)
      error (_("source command requires file name of file to source."));

+ #if defined ALLOW_ENV_VARS
+   /* support env var interpolation in source filename in user/kernel/native*/
+   if (contains_env_var(file))
+     {
+       file = substitute_env_var(file, "'source'-ing", from_tty);
+     }
+ #endif
+
    gdb::optional<open_script> opened = find_and_open_script (file, search_path);
    if (!opened)
      {
***************
*** 844,849 ****
--- 931,945 ----
        if (!arg)
        execl (user_shell, p, (char *) 0);
        else
+ #if defined ALLOW_ENV_VARS
+       /* support interpolation of env vars in shell arg */
+       if (contains_env_var(arg))
+         {
+           char* newArg = substitute_env_var(arg, "exec'ing cmd", from_tty);
+           execl (user_shell, p, "-c", newArg, (char *) 0); //do as usual
+           xfree(newArg);
+         } else
+ #endif //ALLOW_ENV_VARS
        execl (user_shell, p, "-c", arg, (char *) 0);

        fprintf_unfiltered (gdb_stderr, "Cannot execute %s: %s\n", user_shell,

*** fsf/cli-logging.c   2022-09-22 15:59:17.250302000 -0700
--- modifiedFsf/cli-logging.c   2022-09-22 16:26:12.482260000 -0700
***************
*** 139,144 ****
--- 139,151 ----
      current_uiout->redirect (gdb_stdout);
  }

+ #define ALLOW_ENV_VARS 1
+ #if defined (ALLOW_ENV_VARS)
+ extern int contains_env_var(const char *s0);
+ extern char * substitute_env_var
+ (const char *s0, const char* msg, const int from_tty);
+ #endif
+
  static void
  set_logging_on (const char *args, int from_tty)
  {
***************
*** 149,154 ****
--- 156,173 ----
        xfree (logging_filename);
        logging_filename = xstrdup (rest);
      }
+
+ #if defined (ALLOW_ENV_VARS)
+   // recognize ${<envVariable>} in log file name
+   else
+     {
+       if (contains_env_var(logging_filename))
+       {
+         logging_filename = substitute_env_var
+           (logging_filename, "Logging to", from_tty);
+       }
+     }
+ #endif // ALLOW_ENV_VARS
    handle_redirections (from_tty);
  }

[PATCH] gdb: allow env var specifications in cmds 'set log', 'source', 'shell'

[Peikes, Wendy]

[-- Attachment #1: Type: text/plain, Size: 7085 bytes --]


Rationale:

We at NetApp added a gdb feature allowing users to specify environment variable names when invoking gdb commands 'set logging file', 'source' & 'shell'.

This has been a big help especially in gdb macros since it allows macro writers to customize macro
behavior with environment var settings.

We've implemented this feature for the above three gdb commands but it can easily be  extended to other commands such as 'set substitute-path'

Usage example below.

setenv LOG_DIR /tmp
setenv SCRIPT_DIR "."

I've created a script 'macroScript' in the current directory ("."); it is referenced in the gdb session.

Now, run the gdb with the new env var feature:

gdbWithEnvVarFeature
....
GNU gdb (GDB) 9.1
Copyright (C) 2020 Free Software Foundation, Inc.
...

(gdb) shell head ${SCRIPT_DIR}/macroScript
define macro1
       echo macro1\n
end
define macro2
       echo macro2\n
end
macro1
macro2


gdb) source ${SCRIPT_DIR}/macroScript
macro1
macro2
(gdb) set logging file ${LOG_DIR}/logFile
(gdb) show logging file
The current logfile is "${LOG_DIR}/logFile".
(gdb) set logging on
Copying output to /tmp/logFile.
Copying debug output to /tmp/logFile.

(gdb) help set
Evaluate expression EXP and assign result to variable VAR.
Usage: set VAR = EXP
This uses assignment syntax appropriate for the current language
(VAR = EXP or VAR := EXP for example)
...
set architecture -- Set architecture of target.
...

(gdb) set var $i = 1

(gdb) printf "i:%d\n", $i
i:1
(gdb) shell tail ${LOG_DIR}/logFile
set watchdog -- Set watchdog timer.
set width -- Set number of characters where GDB should wrap lines of its output.
set write -- Set writing into executable and core files.
....
Command name abbreviations are allowed if unambiguous.
...
i:1

The patch to gdb9.1 is here; the changed lines are bracket by
#if defined ALLOW_ENV_VARS
#endif

patch:

*** fsf/cli-cmds.c      2022-09-22 15:59:17.238371000 -0700
--- modifiedFsf/cli-cmds.c      2022-09-22 16:29:59.506242000 -0700
***************
*** 660,665 ****
--- 660,744 ----
    script_from_file (stream, file);
  }

+ #define ALLOW_ENV_VARS 1
+ #if defined ALLOW_ENV_VARS
+ /* return true if s uses one or more env vars, specified as ${VARNAME} */
+ extern int
+ contains_env_var(const char *s0)
+ {
+   return s0 && (strlen (s0) > 1) && (strstr(s0, "${") != NULL) &&
+         (strchr(s0, '}') != NULL);
+ }
+
+ /* substitute_env_var - given a string, return a copy of the string with
+    env vars interpolated (if any).  env vars are recognized by the
+    syntax ...${VARNAME}...
+ */
+
+
+ extern char *
+ substitute_env_var(const char *s0, const char* msg, const int from_tty)
+ {
+     int slen, i;
+
+     char *s = xstrdup(s0);
+     slen = strlen(s);
+
+     for (i=0; i<slen; i++)
+       {
+       // find the lead-in for an environment variable
+       if ((s[i] == '$') && (s[i+1] == '{'))
+         {
+           char *endbrace = index(s+i+2, '}');
+           if (endbrace == NULL)
+             {
+               break;
+             }
+           else
+             {
+               int e = endbrace - s;
+
+               //    ...${X}...
+               //       ^  ^
+               //       i  e
+
+               int varname_len = e-(i+2);
+               char *varname = xstrndup(s+i+2, varname_len);
+               char *var_value = getenv(varname);
+               if (var_value == NULL)
+                 {
+                   if (from_tty)
+                   {
+                     fprintf_unfiltered
+                       (gdb_stderr,
+                        "Env var '%s' not set but used in '%s'.\n", varname, s0);
+                   }
+                   var_value = (char*)"";
+                 }
+               xfree(varname);
+               {
+                 // resize s to hold the substitution of varname
+                 int delta = strlen(var_value) - (e-i+1);
+
+                 // new_s may be shorter than s
+                 char *new_s = (char*) xmalloc(slen+delta+1);
+                 memcpy(new_s, s, i);
+                 memcpy(new_s+i, var_value, strlen(var_value));
+                 memmove(new_s+e+1+delta, s+e+1, slen-(e+1)+1);  // include \0
+
+                 // Skip ahead to new string at pos right after replacement so
+                 // we don't try recursive replacement so avoid infinite loops.
+                 i += strlen(var_value) - 1;  // -1 to offset i++ in for()
+                 slen += delta;
+                 xfree(s);
+                 s = new_s;
+               }
+             }
+         }
+       }
+     return s;
+ }
+ #endif // ALLOW_ENV_VARS
+
  /* Worker to perform the "source" command.
     Load script FILE.
     If SEARCH_PATH is non-zero, and the file isn't found in cwd,
***************
*** 672,677 ****
--- 751,764 ----
    if (file == NULL || *file == 0)
      error (_("source command requires file name of file to source."));

+ #if defined ALLOW_ENV_VARS
+   /* support env var interpolation in source filename in user/kernel/native*/
+   if (contains_env_var(file))
+     {
+       file = substitute_env_var(file, "'source'-ing", from_tty);
+     }
+ #endif
+
    gdb::optional<open_script> opened = find_and_open_script (file, search_path);
    if (!opened)
      {
***************
*** 844,849 ****
--- 931,945 ----
        if (!arg)
        execl (user_shell, p, (char *) 0);
        else
+ #if defined ALLOW_ENV_VARS
+       /* support interpolation of env vars in shell arg */
+       if (contains_env_var(arg))
+         {
+           char* newArg = substitute_env_var(arg, "exec'ing cmd", from_tty);
+           execl (user_shell, p, "-c", newArg, (char *) 0); //do as usual
+           xfree(newArg);
+         } else
+ #endif //ALLOW_ENV_VARS
        execl (user_shell, p, "-c", arg, (char *) 0);

        fprintf_unfiltered (gdb_stderr, "Cannot execute %s: %s\n", user_shell,

*** fsf/cli-logging.c   2022-09-22 15:59:17.250302000 -0700
--- modifiedFsf/cli-logging.c   2022-09-22 16:26:12.482260000 -0700
***************
*** 139,144 ****
--- 139,151 ----
      current_uiout->redirect (gdb_stdout);
  }

+ #define ALLOW_ENV_VARS 1
+ #if defined (ALLOW_ENV_VARS)
+ extern int contains_env_var(const char *s0);
+ extern char * substitute_env_var
+ (const char *s0, const char* msg, const int from_tty);
+ #endif
+
  static void
  set_logging_on (const char *args, int from_tty)
  {
***************
*** 149,154 ****
--- 156,173 ----
        xfree (logging_filename);
        logging_filename = xstrdup (rest);
      }
+
+ #if defined (ALLOW_ENV_VARS)
+   // recognize ${<envVariable>} in log file name
+   else
+     {
+       if (contains_env_var(logging_filename))
+       {
+         logging_filename = substitute_env_var
+           (logging_filename, "Logging to", from_tty);
+       }
+     }
+ #endif // ALLOW_ENV_VARS
    handle_redirections (from_tty);
  }

Re: [PATCH] gdb: allow env var specifications in cmds 'set log', 'source', 'shell'

[Keith Seitz]

On 10/5/22 16:19, Peikes, Wendy via Gdb-patches wrote:
> Rationale:
> 
> We at NetApp added a gdb feature allowing users to specify
> environment variable names when invoking gdb commands 'set logging
> file', 'source' & 'shell'.
> 
> This has been a big help especially in gdb macros since it allows
> macro writers to customize macro behavior with environment var
> settings.
> 
> We've implemented this feature for the above three gdb commands but
> it can easily be  extended to other commands such as 'set
> substitute-path'

Thank you for the patch. I agree with you: this seems like a very
useful feature that we should definitely be offering our users.

I've got a few basic comments and questions, if you don't mind.
[Warning: I am going to treat this patch more like an RFC.]

> Usage example below.
> 
> setenv LOG_DIR /tmp
> setenv SCRIPT_DIR "."
> 
> I've created a script 'macroScript' in the current directory ("."); it is referenced in the gdb session.
> 
> Now, run the gdb with the new env var feature:
> 
> gdbWithEnvVarFeature
> ....
> GNU gdb (GDB) 9.1
> Copyright (C) 2020 Free Software Foundation, Inc.
> ...
> 
> (gdb) shell head ${SCRIPT_DIR}/macroScript
> define macro1
>         echo macro1\n
> end
> define macro2
>         echo macro2\n
> end
> macro1
> macro2
> 

I believe this use case already works:

$ export MY_FILE=/tmp/myfile
$ echo "Hello, gdb!" > $MY_FILE
$ ./gdb -batch -ex 'shell cat $MY_FILE'
Hello, gdb!
$

> gdb) source ${SCRIPT_DIR}/macroScript
> macro1
> macro2
> (gdb) set logging file ${LOG_DIR}/logFile
> (gdb) show logging file
> The current logfile is "${LOG_DIR}/logFile".
> (gdb) set logging on
> Copying output to /tmp/logFile.
> Copying debug output to /tmp/logFile.

This is where things start to get interesting...

This patch uses "${" to refer to an environment variable, and
any command desiring to take advantage of this feature must call
contains_env_var and substitute_env_var.

I would think -- perhaps naively -- that implementing this via the
existing convenience variable system would be easier. Did you
consider or try that?

The convenience variable route would also automatically propagate
this new feature to every existing command that already accepts
convenience variables. [Then "source" et al could simply check for
convenience variable substitutions, fixing a bug in those commands,
which currently do not support convenience variables at all.]

This could be pretty easily (famous last words?) accomplished by
expanding the convenience variable API to accept associative arrays
and then internalizing all the environment variables into a new
convenience var called "env" or something descriptive.

Thus, we would use, for example, "set logfile $env(SCRIPT_DIR)/macroScript"
and so on. I think this form (or some variation of it) is more "GDB-like"
and much more explicit than "${}".

As a bonus, we would automatically gain, e.g., "print $env(LD_LIBRARY_PATH)".
AFAIK there is currently no way to use environment variables in the expression
parser.

That is probably being a little too dreamy-eyed, though. Nonetheless,
I would be curious what you and others think. I am asking for no more
than your opinion right now. [Actually I'm in no position to ask for
more than that, either! :-)]

> The patch to gdb9.1 is here; the changed lines are bracket by
> #if defined ALLOW_ENV_VARS
> #endif

If this patch/feature is accepted, you should remove these fenceposts.
I don't see why they'd be needed. Also, we're well past gdb 9 now. Please
rebase your patch to the master branch. [Warning: fprintf_unfiltered doesn't
exist anymore. Use gdb_printf (gdb_stderr, ...).]

Now on to a few comments.

First up, please take a look at the Contribution Checklist:

https://sourceware.org/gdb/wiki/ContributionChecklist

There are a number of formatting errors contained in the patch, so please
read the section of the Checklist dealing with that (and the GNU Coding Standard).
Unfortunately, we do not yet have a formatting script for C/C++ that can handle
this automatically for us. :-(

This is definitely a feature which should be documented in both the user manual
and NEWS. Additionally, I think this patch should be accompanied with tests.

Finally, if you/your employer do not have an FSF assignment on file, you will
need one for this contribution. [I see two previous NetApp gdb contributions in
2000/2001.] This isn't really something I deal with regularly, so I just
raise it as a caveat. This patch definitely does not qualify as "trivial"
IMO.

Thanks again for the patch/rfc. I hope we can get this useful feature in!
Keith

RE: [PATCH] gdb: allow env var specifications in cmds 'set log', 'source', 'shell'

[Peikes, Wendy]

Thank you, Keith, for your interest in adding our NetApp environment var feature into the mainstream FSF gdb!

Answering your questions here with your verbiage preceded by >>


>>This patch uses "${" to refer to an environment variable, and any command desiring to take advantage of this feature
>> must call contains_env_var and >>substitute_env_var.

Actually our patch uses getenv
char *var_value = getenv(varname);

>> I would think -- perhaps naively -- that implementing this via the existing convenience variable system would be easier.
>> Did you consider or try that?

Good idea to use convenience vars.  However, for our use case at NetApp, we absolutely need env vars. Our test infrastructure relies on env var settings.

Since you're asking for an opinion, I'll supply one: I believe that the feature is very useful as is, using env vars, and that adding env var support to a few more cmds would make it even more useful.  The convenience var model, while certainly useful, would not directly do what we need at NetApp.  

We could make the convenience var model work for us at NetApp by getting env var values brought in to convenience vars.  One way that comes to mind: have gdb set convenience vars with a certain name pattern (say "env_xyz") to the values of the corresponding env vars.  For example,  gdb automatically sets convenience var $env_xyz to the value of getenv("xyz").  

But then we'd have to implement the convenience var model that you noted below.  The associative array is a great idea.

But here, that's not what we want and need. Kind of like the old "engineering swing cartoon" (can google that if not familiar) where, in the end, the customer only wanted a tire swing.

How's about this:
- I submit based on the feature we find very useful (env var substitution)
- We upload the latest version of gdb and I create a patch based on it
- I fix the formatting errors, remove the fenceposts, and replace the fprintf with gdb_printf 
- I check from my end if we have an FSF assignment on file
 (Is there a way I can check from the RedHat end if NetApp has an FSF assignment?)
- I create a test for it. (Is there documentation on how to present a test for a patch?)
- Then I submit the updated patch (to newest gdb) and test

Does that work for a plan?

Wendy Peikes
Gdb Team
NetApp




-----Original Message-----
From: Keith Seitz <keiths@redhat.com> 
Sent: Thursday, October 6, 2022 12:35 PM
To: Peikes, Wendy <Wendy.Peikes@netapp.com>; gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb: allow env var specifications in cmds 'set log', 'source', 'shell'

NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.




On 10/5/22 16:19, Peikes, Wendy via Gdb-patches wrote:
> Rationale:
>
> We at NetApp added a gdb feature allowing users to specify environment 
> variable names when invoking gdb commands 'set logging file', 'source' 
> & 'shell'.
>
> This has been a big help especially in gdb macros since it allows 
> macro writers to customize macro behavior with environment var 
> settings.
>
> We've implemented this feature for the above three gdb commands but it 
> can easily be  extended to other commands such as 'set 
> substitute-path'

Thank you for the patch. I agree with you: this seems like a very useful feature that we should definitely be offering our users.

I've got a few basic comments and questions, if you don't mind.
[Warning: I am going to treat this patch more like an RFC.]

> Usage example below.
>
> setenv LOG_DIR /tmp
> setenv SCRIPT_DIR "."
>
> I've created a script 'macroScript' in the current directory ("."); it is referenced in the gdb session.
>
> Now, run the gdb with the new env var feature:
>
> gdbWithEnvVarFeature
> ....
> GNU gdb (GDB) 9.1
> Copyright (C) 2020 Free Software Foundation, Inc.
> ...
>
> (gdb) shell head ${SCRIPT_DIR}/macroScript define macro1
>         echo macro1\n
> end
> define macro2
>         echo macro2\n
> end
> macro1
> macro2
>

I believe this use case already works:

$ export MY_FILE=/tmp/myfile
$ echo "Hello, gdb!" > $MY_FILE
$ ./gdb -batch -ex 'shell cat $MY_FILE'
Hello, gdb!
$

> gdb) source ${SCRIPT_DIR}/macroScript
> macro1
> macro2
> (gdb) set logging file ${LOG_DIR}/logFile
> (gdb) show logging file
> The current logfile is "${LOG_DIR}/logFile".
> (gdb) set logging on
> Copying output to /tmp/logFile.
> Copying debug output to /tmp/logFile.

This is where things start to get interesting...

This patch uses "${" to refer to an environment variable, and any command desiring to take advantage of this feature must call contains_env_var and substitute_env_var.

I would think -- perhaps naively -- that implementing this via the existing convenience variable system would be easier. Did you consider or try that?

The convenience variable route would also automatically propagate this new feature to every existing command that already accepts convenience variables. [Then "source" et al could simply check for convenience variable substitutions, fixing a bug in those commands, which currently do not support convenience variables at all.]

This could be pretty easily (famous last words?) accomplished by expanding the convenience variable API to accept associative arrays and then internalizing all the environment variables into a new convenience var called "env" or something descriptive.

Thus, we would use, for example, "set logfile $env(SCRIPT_DIR)/macroScript"
and so on. I think this form (or some variation of it) is more "GDB-like"
and much more explicit than "${}".

As a bonus, we would automatically gain, e.g., "print $env(LD_LIBRARY_PATH)".
AFAIK there is currently no way to use environment variables in the expression parser.

That is probably being a little too dreamy-eyed, though. Nonetheless, I would be curious what you and others think. I am asking for no more than your opinion right now. [Actually I'm in no position to ask for more than that, either! :-)]

> The patch to gdb9.1 is here; the changed lines are bracket by #if 
> defined ALLOW_ENV_VARS #endif

If this patch/feature is accepted, you should remove these fenceposts.
I don't see why they'd be needed. Also, we're well past gdb 9 now. Please rebase your patch to the master branch. [Warning: fprintf_unfiltered doesn't exist anymore. Use gdb_printf (gdb_stderr, ...).]

Now on to a few comments.

First up, please take a look at the Contribution Checklist:

https://sourceware.org/gdb/wiki/ContributionChecklist

There are a number of formatting errors contained in the patch, so please read the section of the Checklist dealing with that (and the GNU Coding Standard).
Unfortunately, we do not yet have a formatting script for C/C++ that can handle this automatically for us. :-(

This is definitely a feature which should be documented in both the user manual and NEWS. Additionally, I think this patch should be accompanied with tests.

Finally, if you/your employer do not have an FSF assignment on file, you will need one for this contribution. [I see two previous NetApp gdb contributions in 2000/2001.] This isn't really something I deal with regularly, so I just raise it as a caveat. This patch definitely does not qualify as "trivial"
IMO.

Thanks again for the patch/rfc. I hope we can get this useful feature in!
Keith

[RFC PATCH 0/2] Command expression evaulation substitution

[Keith Seitz]

This RFC proposes a new syntax to allow arbitrary expressions to be substituted
into command arguments.

For example, the first patch in this series adds a new `_env' Python convenience
function which gives users access to the shell's environment variables. If a user
wanted to access the value of an environment variable within a GDB command,
though, they cannot currently do that:

(gdb) set logging file $_env("HOME")/$_env("MYLOG")
(gdb) show logging file
The current logfile is "$_env("HOME")/$_env("MYLOG")".
(gdb) eval "set logging file %s/%s", $_env("HOME"), $_env("MYLOG")
evaluation of this expression requires the target program to be active

That `eval' doesn't work is likely a bug, but I nonetheless find the syntax quite
cumbersome for this purpose.

Therefore, I've chosen to introduce a new marker for expression substitution, $().
The second patch implements a proof-of-concept implementation following this
concept:

(gdb) set logging file $($_env("HOME"))/$($_env("MYLOG"))
(gdb) show logging file
The current logfile is "/home/keiths/my.log".

There is a lot more to do to refine this new feature. For example, the value
almost certainly needs to have a lot of format tweaking, since, for example,
using chars does not work:

(gdb) p 'a'
$1 = 97 'a'
(gdb) echo $($1)\n
97 'a'

There are probably a multitude of other similar issues to be sorted out,
such as arrays, repeating elements, symbol/object printing and so on.
If anyone has any suggestions on how to improve this value printing code, I'd
love to hear 'em.

I've also chosen to hook this in at quite a low level so that all commands can
immediately benefit from this. [That is, we don't have to add handling code into
every command function in which we want to support this new syntax.] This may
not be the best approach.

@Wendy.Peikes@netapp.com: Would this proposal satisfy NetApp's needs?

Keith Seitz (2):
  Add $_env convenience function
  Allow and evaluate expressions in command arguments

 gdb/data-directory/Makefile.in     |  1 +
 gdb/python/lib/gdb/function/env.py | 21 ++++++++++
 gdb/top.c                          | 65 ++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+)
 create mode 100644 gdb/python/lib/gdb/function/env.py

-- 
2.38.1

[RFC PATCH 1/2] Add $_env convenience function

[Keith Seitz]

This patch adds a new Python convenience function which simply
returns the value of the given environment variable or throws
a KeyError.

(gdb) p $_env("HOME")
$1 = "/home/keiths"

[This patch is here for the purposes of demonstarting an alternative
design/implementation to a previously proposed patch. See
https://sourceware.org/pipermail/gdb-patches/2022-October/192396.html .]
---
 gdb/data-directory/Makefile.in     |  1 +
 gdb/python/lib/gdb/function/env.py | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+)
 create mode 100644 gdb/python/lib/gdb/function/env.py

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index f1139291eed..3b0a05d61dc 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -107,6 +107,7 @@ PYTHON_FILE_LIST = \
 	gdb/function/as_string.py \
 	gdb/function/caller_is.py \
 	gdb/function/strfns.py \
+	gdb/function/env.py \
 	gdb/printer/__init__.py \
 	gdb/printer/bound_registers.py
 
diff --git a/gdb/python/lib/gdb/function/env.py b/gdb/python/lib/gdb/function/env.py
new file mode 100644
index 00000000000..54a441cea54
--- /dev/null
+++ b/gdb/python/lib/gdb/function/env.py
@@ -0,0 +1,21 @@
+"""$_env"""
+
+import gdb
+import os
+
+class _Env(gdb.Function):
+    """$_env - return the value of an environment variable.
+
+    Usage: $_env("NAME")
+
+    Returns:
+      Value of the environment variable named NAME or throws KeyError if NAME is
+      undefined in the environment."""
+
+    def __init__(self):
+        super(_Env, self).__init__("_env")
+
+    def invoke(self, name):
+        return os.environ[name.string()]
+
+_Env()
-- 
2.38.1

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

[Keith Seitz]

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(+)

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

RE: [RFC PATCH 0/2] Command expression evaulation substitution

[Peikes, Wendy]

Thank you so much for writing back.

So is this patch you incorporated based on the idea I sent you? If so THANK YOU!

wendy

-----Original Message-----
From: Keith Seitz <keiths@redhat.com> 
Sent: Wednesday, January 25, 2023 11:38 AM
To: gdb-patches@sourceware.org
Cc: Peikes, Wendy <Wendy.Peikes@netapp.com>
Subject: [RFC PATCH 0/2] Command expression evaulation substitution

NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.




This RFC proposes a new syntax to allow arbitrary expressions to be substituted into command arguments.

For example, the first patch in this series adds a new `_env' Python convenience function which gives users access to the shell's environment variables. If a user wanted to access the value of an environment variable within a GDB command, though, they cannot currently do that:

(gdb) set logging file $_env("HOME")/$_env("MYLOG")
(gdb) show logging file
The current logfile is "$_env("HOME")/$_env("MYLOG")".
(gdb) eval "set logging file %s/%s", $_env("HOME"), $_env("MYLOG") evaluation of this expression requires the target program to be active

That `eval' doesn't work is likely a bug, but I nonetheless find the syntax quite cumbersome for this purpose.

Therefore, I've chosen to introduce a new marker for expression substitution, $().
The second patch implements a proof-of-concept implementation following this
concept:

(gdb) set logging file $($_env("HOME"))/$($_env("MYLOG"))
(gdb) show logging file
The current logfile is "/home/keiths/my.log".

There is a lot more to do to refine this new feature. For example, the value almost certainly needs to have a lot of format tweaking, since, for example, using chars does not work:

(gdb) p 'a'
$1 = 97 'a'
(gdb) echo $($1)\n
97 'a'

There are probably a multitude of other similar issues to be sorted out, such as arrays, repeating elements, symbol/object printing and so on.
If anyone has any suggestions on how to improve this value printing code, I'd love to hear 'em.

I've also chosen to hook this in at quite a low level so that all commands can immediately benefit from this. [That is, we don't have to add handling code into every command function in which we want to support this new syntax.] This may not be the best approach.

@Wendy.Peikes@netapp.com: Would this proposal satisfy NetApp's needs?

Keith Seitz (2):
  Add $_env convenience function
  Allow and evaluate expressions in command arguments

 gdb/data-directory/Makefile.in     |  1 +
 gdb/python/lib/gdb/function/env.py | 21 ++++++++++
 gdb/top.c                          | 65 ++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+)
 create mode 100644 gdb/python/lib/gdb/function/env.py

--
2.38.1

Re: [RFC PATCH 1/2] Add $_env convenience function

[Andrew Burgess]

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

> This patch adds a new Python convenience function which simply
> returns the value of the given environment variable or throws
> a KeyError.
>
> (gdb) p $_env("HOME")
> $1 = "/home/keiths"
>
> [This patch is here for the purposes of demonstarting an alternative
> design/implementation to a previously proposed patch. See
> https://sourceware.org/pipermail/gdb-patches/2022-October/192396.html .]

It probably has value on its own I think.

> ---
>  gdb/data-directory/Makefile.in     |  1 +
>  gdb/python/lib/gdb/function/env.py | 21 +++++++++++++++++++++
>  2 files changed, 22 insertions(+)
>  create mode 100644 gdb/python/lib/gdb/function/env.py
>
> diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
> index f1139291eed..3b0a05d61dc 100644
> --- a/gdb/data-directory/Makefile.in
> +++ b/gdb/data-directory/Makefile.in
> @@ -107,6 +107,7 @@ PYTHON_FILE_LIST = \
>  	gdb/function/as_string.py \
>  	gdb/function/caller_is.py \
>  	gdb/function/strfns.py \
> +	gdb/function/env.py \
>  	gdb/printer/__init__.py \
>  	gdb/printer/bound_registers.py
>  
> diff --git a/gdb/python/lib/gdb/function/env.py b/gdb/python/lib/gdb/function/env.py
> new file mode 100644
> index 00000000000..54a441cea54
> --- /dev/null
> +++ b/gdb/python/lib/gdb/function/env.py
> @@ -0,0 +1,21 @@

Missing copyright header here.

> +"""$_env"""
> +
> +import gdb
> +import os
> +
> +class _Env(gdb.Function):
> +    """$_env - return the value of an environment variable.
> +
> +    Usage: $_env("NAME")
> +
> +    Returns:
> +      Value of the environment variable named NAME or throws KeyError if NAME is
> +      undefined in the environment."""
> +
> +    def __init__(self):
> +        super(_Env, self).__init__("_env")
> +
> +    def invoke(self, name):
> +        return os.environ[name.string()]

Something like this:

    def invoke(self, name, default=None):
        if default is not None and not name.string() in os.environ:
          return default
        return os.environ[name.string()]

would allow users to supply a default:

  p $_env("UNKNOWN", "default_value")

But retains:

  p $_env("HOME")

Might be useful?

Thanks,
Andrew

> +
> +_Env()
> -- 
> 2.38.1

Re: [RFC PATCH 2/2] Allow and evaluate expressions in command arguments

[Andrew Burgess]

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

Re: [RFC PATCH 1/2] Add $_env convenience function

[Keith Seitz]

On 2/3/23 09:18, Andrew Burgess wrote:

>> diff --git a/gdb/python/lib/gdb/function/env.py b/gdb/python/lib/gdb/function/env.py
>> new file mode 100644
>> index 00000000000..54a441cea54
>> --- /dev/null
>> +++ b/gdb/python/lib/gdb/function/env.py
>> @@ -0,0 +1,21 @@
> 
> Missing copyright header here.

It *was* just an RFC. :-P

I will, of course, add appropriate header, copyright, etc
when I submit for final approval (after I write a few tests
and docs).

I will submit independently of the expression-evaluation
substitution patch (#2 in this series).

> 
>> +"""$_env"""
>> +
>> +import gdb
>> +import os
>> +
>> +class _Env(gdb.Function):
>> +    """$_env - return the value of an environment variable.
>> +
>> +    Usage: $_env("NAME")
>> +
>> +    Returns:
>> +      Value of the environment variable named NAME or throws KeyError if NAME is
>> +      undefined in the environment."""
>> +
>> +    def __init__(self):
>> +        super(_Env, self).__init__("_env")
>> +
>> +    def invoke(self, name):
>> +        return os.environ[name.string()]
> 
> Something like this:
> 
>      def invoke(self, name, default=None):
>          if default is not None and not name.string() in os.environ:
>            return default
>          return os.environ[name.string()]
> 
> would allow users to supply a default:
> 
>    p $_env("UNKNOWN", "default_value")
> 
> But retains:
> 
>    p $_env("HOME")
> 
> Might be useful?

That's a very good idea. I'll steal that. :-)

Keith

Re: [RFC PATCH 2/2] Allow and evaluate expressions in command arguments

[Keith Seitz]

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

Re: [RFC PATCH 2/2] Allow and evaluate expressions in command arguments

[Pedro Alves]

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