[3/6] Add new "$_shell(CMD)" internal function

  For testing a following patch, I wanted a way to send a SIGINT to GDB
from a breakpoint condition.  And I didn't want to do it from a Python
breakpoint or Python function, as I wanted to exercise non-Python code
paths.  So I thought I'd add a new $_shell internal function, that
runs a command under the shell, and returns the exit code.  With this,
I could write:

  (gdb) b foo if $_shell("kill -SIGINT $gdb_pid") != 0 || <other condition>

I think this is generally useful, hence I'm proposing it here.

Here's the new function in action:

 (gdb) p $_shell("true")
 $1 = 0
 (gdb) p $_shell("false")
 $2 = 1
 (gdb) p $_shell("echo hello")
 hello
 $3 = 0
 (gdb) p $_shell("foobar")
 bash: line 1: foobar: command not found
 $4 = 127
 (gdb) help function _shell
 $_shell - execute a shell command and returns the result.
 Usage: $_shell (command)
 Returns the command's exit code: zero on success, non-zero otherwise.
 (gdb)

NEWS and manual changes included.

Change-Id: I7e36d451ee6b428cbf41fded415ae2d6b4efaa4e
---
 gdb/NEWS                           | 10 ++++
 gdb/cli/cli-cmds.c                 | 89 ++++++++++++++++++++++++++++--
 gdb/doc/gdb.texinfo                | 47 ++++++++++++++++
 gdb/testsuite/gdb.base/default.exp |  1 +
 gdb/testsuite/gdb.base/shell.exp   | 36 ++++++++++++
 5 files changed, 179 insertions(+), 4 deletions(-)
  

> From: Pedro Alves <pedro@palves.net>
> Date: Fri, 10 Feb 2023 23:36:01 +0000
> 
>  gdb/NEWS                           | 10 ++++
>  gdb/cli/cli-cmds.c                 | 89 ++++++++++++++++++++++++++++--
>  gdb/doc/gdb.texinfo                | 47 ++++++++++++++++
>  gdb/testsuite/gdb.base/default.exp |  1 +
>  gdb/testsuite/gdb.base/shell.exp   | 36 ++++++++++++
>  5 files changed, 179 insertions(+), 4 deletions(-)

Thanks.

> +* New convenience function "$_shell", to execute a shell command and
> +  return the result.  This lets you run shell commands in expressions.
> +  Some examples:
> +
> +   (gdb) p $_shell("true")
> +   $1 = 0
> +   (gdb) p $_shell("false")
> +   $2 = 1
> +   (gdb) break func if $_shell("some command") == 0
> +
>  * MI changes

This part is OK.

> -static void
> -shell_escape (const char *arg, int from_tty)
> +/* Run ARG under the shell, and return the exit status.  If ARG is
> +   NULL, run an interactive shell.  */
> +
> +static int
> +run_under_shell (const char *arg, int from_tty)
>  {
>  #if defined(CANT_FORK) || \
>        (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
> @@ -915,7 +922,7 @@ shell_escape (const char *arg, int from_tty)
>       the shell command we just ran changed it.  */
>    chdir (current_directory);
>  #endif
> -  exit_status_set_internal_vars (rc);
> +  return rc;
>  #else /* Can fork.  */
>    int status, pid;
>  
> @@ -942,10 +949,21 @@ shell_escape (const char *arg, int from_tty)
>      waitpid (pid, &status, 0);
>    else
>      error (_("Fork failed"));
> -  exit_status_set_internal_vars (status);
> +  return status;
>  #endif /* Can fork.  */
>  }

This part seems to leave in place the error message in case invoking
the shell command failed (i.e. 'system' or 'execl' failed).  I wonder
whether we should still show that error message if this was called via
the new built-in function.  Perhaps it would be better to instead
return a distinct return value?  Having these messages emitted during
breakpoint commands, for example, disrupts the "normal" display of
breakpoint data and output from breakpoint commands, so perhaps it is
better to leave the handling of this situation to the caller?

And another comment/question: on Posix hosts this calls fork/exec, so
if GDB was configured to follow forks, does it mean it might start
debugging the program invoked via the shell function? if so, what does
that mean for using that function in breakpoint commands?  If there is
some subtleties to be aware of here, I think at least the manual
should mention them.

> +  add_internal_function ("_shell", _("\
> +$_shell - execute a shell command and return the result.\n\
> +Usage: $_shell (command)\n\
> +Returns the command's exit code: zero on success, non-zero otherwise."),
   ^^^^^^^
I think our usual style is to say "Return", not "Returns".

> +@anchor{$_shell convenience function}
> +@item $_shell (@var{command-string})
> +@findex $_shell@r{, convenience function}

@fined should be _before_ @item, so that the index records the
position of @item, and the Info reader places you before the @item
when you use index-search.

> +Invoke a standard shell to execute @var{command-string}.  Returns the
          ^^^^^^^^^^^^^^^^
"the standard system shell" is better, I think.

> +command's exit status.  On Unix systems, a command which exits with a
> +zero exit status has succeeded, and non-zero exit status indicates
> +failure.  When a command terminates on a fatal signal whose number is
> +N, @value{GDBN} uses the value 128+N as the exit status, as is
> +standard in Unix shells.

"N" should be "@var{n}" here.

Also, this text could benefit from a cross-reference to where we
describe the commands that convert from signal numbers to their
mnemonics, since that is system-dependent.  Maybe "info signal N" is
the right tool here?  If so, a cross-reference to "Signals" is what we
should have here.  (Is there a better way of asking GDB about signal
number N?)

> +In this scenario, you'll want to make sure that the shell command you
> +run in the breakpoint condition takes the least amount of time
> +possible.  This is important to minimize the time it takes to evaluate
> +the condition and re-resume the program if the condition turns out to
> +be false.

I understand what this says, but not what it means in practice.
Perhaps one or two additional sentences to help the reader understand
how to "make sure the shell command in a breakpoint condition takes
the least amount of time" would be in order?

Reviewed-by: Eli Zaretskii <eliz@gnu.org>
  

Hi,

On 2023-02-11 8:02 a.m., Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@palves.net>
>> Date: Fri, 10 Feb 2023 23:36:01 +0000
>>
>>  gdb/NEWS                           | 10 ++++
>>  gdb/cli/cli-cmds.c                 | 89 ++++++++++++++++++++++++++++--
>>  gdb/doc/gdb.texinfo                | 47 ++++++++++++++++
>>  gdb/testsuite/gdb.base/default.exp |  1 +
>>  gdb/testsuite/gdb.base/shell.exp   | 36 ++++++++++++
>>  5 files changed, 179 insertions(+), 4 deletions(-)
> 
> Thanks.
> 
>> +* New convenience function "$_shell", to execute a shell command and
>> +  return the result.  This lets you run shell commands in expressions.
>> +  Some examples:
>> +
>> +   (gdb) p $_shell("true")
>> +   $1 = 0
>> +   (gdb) p $_shell("false")
>> +   $2 = 1
>> +   (gdb) break func if $_shell("some command") == 0
>> +
>>  * MI changes
> 
> This part is OK.
> 
>> -static void
>> -shell_escape (const char *arg, int from_tty)
>> +/* Run ARG under the shell, and return the exit status.  If ARG is
>> +   NULL, run an interactive shell.  */
>> +
>> +static int
>> +run_under_shell (const char *arg, int from_tty)
>>  {
>>  #if defined(CANT_FORK) || \
>>        (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
>> @@ -915,7 +922,7 @@ shell_escape (const char *arg, int from_tty)
>>       the shell command we just ran changed it.  */
>>    chdir (current_directory);
>>  #endif
>> -  exit_status_set_internal_vars (rc);
>> +  return rc;
>>  #else /* Can fork.  */
>>    int status, pid;
>>  
>> @@ -942,10 +949,21 @@ shell_escape (const char *arg, int from_tty)
>>      waitpid (pid, &status, 0);
>>    else
>>      error (_("Fork failed"));
>> -  exit_status_set_internal_vars (status);
>> +  return status;
>>  #endif /* Can fork.  */
>>  }
> 
> This part seems to leave in place the error message in case invoking
> the shell command failed (i.e. 'system' or 'execl' failed).  

Actually, if 'system' failed, we return "rc", not throw an error.
Similarly, if 'execl' fails, the child calls _exit(0177), which is reported to
the parent (gdb) via waitpid, and so the function ends up returning the status.

The only scenario we throw an error is if fork fails, which is really a
catastrophic failure indicating GDB is out of resources, and not a problem
with the command the user wanted to run.

For example:

 (gdb) p $_shell ("doesnt exit")
 bash: line 1: doesnt: command not found
 $1 = 127

Note we still printed the $1 result, not aborted with an error.  The printout
comes from bash.

Here's what the patched function looks like:

static int
run_under_shell (const char *arg, int from_tty)
{
#if defined(CANT_FORK) || \
      (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
  /* If ARG is NULL, they want an inferior shell, but `system' just
     reports if the shell is available when passed a NULL arg.  */
  int rc = system (arg ? arg : "");

  if (!arg)
    arg = "inferior shell";

  if (rc == -1)
    gdb_printf (gdb_stderr, "Cannot execute %s: %s\n", arg,
		safe_strerror (errno));
  else if (rc)
    gdb_printf (gdb_stderr, "%s exited with status %d\n", arg, rc);
#ifdef GLOBAL_CURDIR
  /* Make sure to return to the directory GDB thinks it is, in case
     the shell command we just ran changed it.  */
  chdir (current_directory);
#endif
  return rc;
#else /* Can fork.  */
  int status, pid;

  if ((pid = vfork ()) == 0)
    {
      const char *p, *user_shell = get_shell ();

      close_most_fds ();

      /* Get the name of the shell for arg0.  */
      p = lbasename (user_shell);

      if (!arg)
	execl (user_shell, p, (char *) 0);
      else
	execl (user_shell, p, "-c", arg, (char *) 0);

      gdb_printf (gdb_stderr, "Cannot execute %s: %s\n", user_shell,
		  safe_strerror (errno));
      _exit (0177);
    }

  if (pid != -1)
    waitpid (pid, &status, 0);
  else
    error (_("Fork failed"));
  return status;
#endif /* Can fork.  */
}

Note there's only on error call in the function.


> I wonder
> whether we should still show that error message if this was called via
> the new built-in function.  Perhaps it would be better to instead
> return a distinct return value?  Having these messages emitted during
> breakpoint commands, for example, disrupts the "normal" display of
> breakpoint data and output from breakpoint commands, so perhaps it is
> better to leave the handling of this situation to the caller?
> 
> And another comment/question: on Posix hosts this calls fork/exec, so
> if GDB was configured to follow forks, does it mean it might start
> debugging the program invoked via the shell function? if so, what does
> that mean for using that function in breakpoint commands?  If there is
> some subtleties to be aware of here, I think at least the manual
> should mention them.

It does not mean that (follow fork), because like with the "shell" command, it is
GDB that invokes the shell, not the inferior.  We follow forks when the
inferior itself forks, as we decide whether to continue debugging the parent,
or switch to debugging the child, or debugging both.  The inferior could even
be a remote inferior running under gdbserver or some other stub, and the
shell would run on the gdb side.

> 
>> +  add_internal_function ("_shell", _("\
>> +$_shell - execute a shell command and return the result.\n\
>> +Usage: $_shell (command)\n\
>> +Returns the command's exit code: zero on success, non-zero otherwise."),
>    ^^^^^^^
> I think our usual style is to say "Return", not "Returns".


Hmmm.  If I do "apropos returns", I get:

(gdb) apropos returns
finish, fin -- Execute until selected stack frame returns.
function _any_caller_is -- Check all calling function's names.
function _any_caller_matches -- Compare all calling function's names with a regexp.
function _as_string -- Return the string representation of a value.
function _caller_is -- Check the calling function's name.
function _caller_matches -- Compare the calling function's name with a regexp.
function _gdb_maint_setting -- $_gdb_maint_setting - returns the value of a GDB maintenance setting.
function _gdb_maint_setting_str -- $_gdb_maint_setting_str - returns the value of a GDB maintenance setting as a string.
function _gdb_setting -- $_gdb_setting - returns the value of a GDB setting.
function _gdb_setting_str -- $_gdb_setting_str - returns the value of a GDB setting as a string.
function _memeq -- $_memeq - compare bytes of memory.
function _regex -- $_regex - check if a string matches a regular expression.
function _shell -- $_shell - execute a shell command and return the result.
function _streq -- $_streq - check string equality.
function _strlen -- $_strlen - compute string length.

and looking at several of those builtin function's help, we see this style:

(gdb) help function _any_caller_is 
Check all calling function's names.

    Usage: $_any_caller_is (NAME [, NUMBER-OF-FRAMES])

    Arguments:

      NAME: The name of the function to search for.

      NUMBER-OF-FRAMES: How many stack frames to traverse back from the currently
        selected frame to compare with.  If the value is greater than the depth of
        the stack from that point then the result is False.
        The default is 1.

    Returns:
      True if any function's name is equal to NAME.


I guess I should instead use that style here too?  I've done that now, and here's
what we get:

(gdb) help function _shell
$_shell - execute a shell command and return the result.

    Usage: $_shell (COMMAND)

    Arguments:

      COMMAND: The command to execute.  Must be a string.

    Returns:
      The command's exit code: zero on success, non-zero otherwise.


WDYT?


> 
>> +@anchor{$_shell convenience function}
>> +@item $_shell (@var{command-string})
>> +@findex $_shell@r{, convenience function}
> 
> @fined should be _before_ @item, so that the index records the
> position of @item, and the Info reader places you before the @item
> when you use index-search.

But the whole "Convenience Funs" node has has item/findex sorted like that,
I just copied the preceding example.  Here's what we have:

 @item $_isvoid (@var{expr})
 @findex $_isvoid@r{, convenience function}
 ...
 @item $_gdb_setting_str (@var{setting})
 @findex $_gdb_setting_str@r{, convenience function}
 ...
 @item $_gdb_setting (@var{setting})
 @findex $_gdb_setting@r{, convenience function}
 ...
 @item $_gdb_maint_setting_str (@var{setting})
 @findex $_gdb_maint_setting_str@r{, convenience function}
 ...
 @item $_gdb_maint_setting (@var{setting})
 @findex $_gdb_maint_setting@r{, convenience function}

Are all these wrong?  Sounds like they should all be fixed in one go.
I don't usually use the info reader, so I'm not really sure what to look for.

> 
>> +Invoke a standard shell to execute @var{command-string}.  Returns the
>           ^^^^^^^^^^^^^^^^
> "the standard system shell" is better, I think.

Note this is the same wording used when describing the "shell" command:

 @table @code
 @kindex shell
 @kindex !
 @cindex shell escape
 @item shell @var{command-string}
 @itemx !@var{command-string}
 Invoke a standard shell to execute @var{command-string}.
 Note that no space is needed between @code{!} and @var{command-string}.
 On GNU and Unix systems, the environment variable @env{SHELL}, if it
 exists, determines which shell to run.  Otherwise @value{GDBN} uses
 the default shell (@file{/bin/sh} on GNU and Unix systems,
 @file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
 @end table


Maybe the intention was to avoid "system" as @env{SHELL} may point
to some shell the user installed, which doesn't come with or from
the "system"?  So I'd prefer to use the same terminology in $_shell too.

> 
>> +command's exit status.  On Unix systems, a command which exits with a
>> +zero exit status has succeeded, and non-zero exit status indicates
>> +failure.  When a command terminates on a fatal signal whose number is
>> +N, @value{GDBN} uses the value 128+N as the exit status, as is
>> +standard in Unix shells.
> 
> "N" should be "@var{n}" here.

Thanks, done.

BTW, I forgot to mention, but I had borrowed that wording from the
Bash documentation:

  https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html

> Also, this text could benefit from a cross-reference to where we
> describe the commands that convert from signal numbers to their
> mnemonics, since that is system-dependent.  Maybe "info signal N" is
> the right tool here?  If so, a cross-reference to "Signals" is what we
> should have here.  (Is there a better way of asking GDB about signal
> number N?)

I don't think it's the right tool here, because "info signal" and
"handle", etc., are about target signals, signals that trigger in the
inferior, which GDB is able to intercept.  OTOH, the $_shell function
and the "shell" command run the shell on the host, so any fatal signal code
is a host signal number, and doesn't go via any translation at all.

I'll note that where we document $_shell_exitsignal, the convenience
variable that the "shell" command sets, we don't describe the host signals
either.

I've moved the "The shell runs on the host machine, the machine @value{GDBN} is
running on." sentence earlier, in case that helps set expectations, and
added a couple sentences about host vs target signal numbers.  

I had also forgotten to document that the argument must be a string.

Here is the result:

"Invoke a standard shell to execute @var{command-string}.
@var{command-string} must be a string.  The shell runs on the host
machine, the machine @value{GDBN} is running on.  Returns the
command's exit status.  On Unix systems, a command which exits with a
zero exit status has succeeded, and non-zero exit status indicates
failure.  When a command terminates on a fatal signal whose number is
@var{N}, @value{GDBN} uses the value 128+@var{N} as the exit status,
as is standard in Unix shells.  Note that @var{N} is a host signal
number, not a target signal number.  If you're cross debugging, the
host vs target signal numbers may be completely unrelated.  The shell
to run is determined in the same way as for the @code{shell} command.
@xref{Shell Commands, ,Shell Commands}."

WDYT?

> 
>> +In this scenario, you'll want to make sure that the shell command you
>> +run in the breakpoint condition takes the least amount of time
>> +possible.  This is important to minimize the time it takes to evaluate
>> +the condition and re-resume the program if the condition turns out to
>> +be false.
> 
> I understand what this says, but not what it means in practice.
> Perhaps one or two additional sentences to help the reader understand
> how to "make sure the shell command in a breakpoint condition takes
> the least amount of time" would be in order?

Maybe something like this?

"In this scenario, you'll want to make sure that the shell command you
run in the breakpoint condition takes the least amount of time
possible.  For example, avoid running a command that may block
indefinitely, or that sleeps for a while before exiting.  Prefer a
command or script which analyzes some state and exits immediately.
This is important because the debugged program stops for the
breakpoint every time, and then @value{GDBN} evaluates the breakpoint
condition.  If the condition is false, the program is re-resumed
transparently, without informing you of the stop.  A quick shell
command thus avoids significantly slowing down the debugged program
unnecessarily."

> 
> Reviewed-by: Eli Zaretskii <eliz@gnu.org>
> 

Thanks.  Here's the updated patch with the changes addressing the comments
above.  Let me know what you think.

From 1fc4d7e1fd145df7bca31c0630cb44fd8c85bbd4 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 8 Feb 2023 16:06:23 +0000
Subject: [PATCH] Add new "$_shell(CMD)" internal function

For testing a following patch, I wanted a way to send a SIGINT to GDB
from a breakpoint condition.  And I didn't want to do it from a Python
breakpoint or Python function, as I wanted to exercise non-Python code
paths.  So I thought I'd add a new $_shell internal function, that
runs a command under the shell, and returns the exit code.  With this,
I could write:

  (gdb) b foo if $_shell("kill -SIGINT $gdb_pid") != 0 || <other condition>

I think this is generally useful, hence I'm proposing it here.

Here's the new function in action:

 (gdb) p $_shell("true")
 $1 = 0
 (gdb) p $_shell("false")
 $2 = 1
 (gdb) p $_shell("echo hello")
 hello
 $3 = 0
 (gdb) p $_shell("foobar")
 bash: line 1: foobar: command not found
 $4 = 127
 (gdb) help function _shell
 $_shell - execute a shell command and returns the result.
 Usage: $_shell (command)
 Returns the command's exit code: zero on success, non-zero otherwise.
 (gdb)

NEWS and manual changes included.

Change-Id: I7e36d451ee6b428cbf41fded415ae2d6b4efaa4e
---
 gdb/NEWS                           | 10 ++++
 gdb/cli/cli-cmds.c                 | 96 ++++++++++++++++++++++++++++--
 gdb/doc/gdb.texinfo                | 56 +++++++++++++++++
 gdb/testsuite/gdb.base/default.exp |  1 +
 gdb/testsuite/gdb.base/shell.exp   | 36 +++++++++++
 5 files changed, 195 insertions(+), 4 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index b85923cf80d..0d3445438b1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -56,6 +56,16 @@ maintenance info frame-unwinders
   List the frame unwinders currently in effect, starting with the highest
   priority.
 
+* New convenience function "$_shell", to execute a shell command and
+  return the result.  This lets you run shell commands in expressions.
+  Some examples:
+
+   (gdb) p $_shell("true")
+   $1 = 0
+   (gdb) p $_shell("false")
+   $2 = 1
+   (gdb) break func if $_shell("some command") == 0
+
 * MI changes
 
 ** mi now reports 'no-history' as a stop reason when hitting the end of the
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 6c0d780face..7607fe59b05 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -39,6 +39,7 @@
 #include "gdbsupport/filestuff.h"
 #include "location.h"
 #include "block.h"
+#include "valprint.h"
 
 #include "ui-out.h"
 #include "interps.h"
@@ -873,6 +874,9 @@ exit_status_set_internal_vars (int exit_status)
 
   clear_internalvar (var_code);
   clear_internalvar (var_signal);
+
+  /* Keep the logic here in sync with shell_internal_fn.  */
+
   if (WIFEXITED (exit_status))
     set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
 #ifdef __MINGW32__
@@ -893,8 +897,11 @@ exit_status_set_internal_vars (int exit_status)
     warning (_("unexpected shell command exit status %d"), exit_status);
 }
 
-static void
-shell_escape (const char *arg, int from_tty)
+/* Run ARG under the shell, and return the exit status.  If ARG is
+   NULL, run an interactive shell.  */
+
+static int
+run_under_shell (const char *arg, int from_tty)
 {
 #if defined(CANT_FORK) || \
       (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
@@ -915,7 +922,7 @@ shell_escape (const char *arg, int from_tty)
      the shell command we just ran changed it.  */
   chdir (current_directory);
 #endif
-  exit_status_set_internal_vars (rc);
+  return rc;
 #else /* Can fork.  */
   int status, pid;
 
@@ -942,10 +949,21 @@ shell_escape (const char *arg, int from_tty)
     waitpid (pid, &status, 0);
   else
     error (_("Fork failed"));
-  exit_status_set_internal_vars (status);
+  return status;
 #endif /* Can fork.  */
 }
 
+/* Escape out to the shell to run ARG.  If ARG is NULL, launch and
+   interactive shell.  Sets $_shell_exitcode and $_shell_exitsignal
+   convenience variables based on the exits status.  */
+
+static void
+shell_escape (const char *arg, int from_tty)
+{
+  int status = run_under_shell (arg, from_tty);
+  exit_status_set_internal_vars (status);
+}
+
 /* Implementation of the "shell" command.  */
 
 static void
@@ -2417,6 +2435,63 @@ gdb_maint_setting_str_internal_fn (struct gdbarch *gdbarch,
   return str_value_from_setting (*show_cmd->var, gdbarch);
 }
 
+/* Implementation of the convenience function $_shell.  */
+
+static struct value *
+shell_internal_fn (struct gdbarch *gdbarch,
+		   const struct language_defn *language,
+		   void *cookie, int argc, struct value **argv)
+{
+  if (argc != 1)
+    error (_("You must provide one argument for $_shell."));
+
+  value *val = argv[0];
+  struct type *type = check_typedef (value_type (val));
+
+  if (!language->is_string_type_p (type))
+    error (_("Argument must be a string."));
+
+  value_print_options opts;
+  get_no_prettyformat_print_options (&opts);
+
+  string_file stream;
+  value_print (val, &stream, &opts);
+
+  /* We should always have two quote chars, which we'll strip.  */
+  gdb_assert (stream.size () >= 2);
+
+  /* Now strip them.  We don't need the original string, so it's
+     cheaper to do it in place, avoiding a string allocation.  */
+  std::string str = stream.release ();
+  str[str.size () - 1] = 0;
+  const char *command = str.c_str () + 1;
+
+  int exit_status = run_under_shell (command, 0);
+
+  struct type *int_type = builtin_type (gdbarch)->builtin_int;
+
+  /* Keep the logic here in sync with
+     exit_status_set_internal_vars.  */
+
+  if (WIFEXITED (exit_status))
+    return value_from_longest (int_type, WEXITSTATUS (exit_status));
+#ifdef __MINGW32__
+  else if (WIFSIGNALED (exit_status) && WTERMSIG (exit_status) == -1)
+    {
+      /* See exit_status_set_internal_vars.  */
+      return value_from_longest (int_type, exit_status);
+    }
+#endif
+  else if (WIFSIGNALED (exit_status))
+    {
+      /* (0x80 | SIGNO) is what most (all?) POSIX-like shells set as
+	 exit code on fatal signal termination.  */
+      return value_from_longest (int_type, 0x80 | WTERMSIG (exit_status));
+    }
+  else
+    return allocate_optimized_out_value (int_type);
+}
+
 void _initialize_cli_cmds ();
 void
 _initialize_cli_cmds ()
@@ -2606,6 +2681,19 @@ Some integer settings accept an unlimited value, returned\n\
 as 0 or -1 depending on the setting."),
 			 gdb_maint_setting_internal_fn, NULL);
 
+  add_internal_function ("_shell", _("\
+$_shell - execute a shell command and return the result.\n\
+\n\
+    Usage: $_shell (COMMAND)\n\
+\n\
+    Arguments:\n\
+\n\
+      COMMAND: The command to execute.  Must be a string.\n\
+\n\
+    Returns:\n\
+      The command's exit code: zero on success, non-zero otherwise."),
+			 shell_internal_fn, NULL);
+
   add_cmd ("commands", no_set_class, show_commands, _("\
 Show the history of commands you typed.\n\
 You can supply a command number to start with, or a `+' to start after\n\
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7b128053b5a..310f857f09a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1629,6 +1629,10 @@ the default shell (@file{/bin/sh} on GNU and Unix systems,
 @file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
 @end table
 
+You may also invoke shell commands from expressions, using the
+@code{$_shell} convenience function.  @xref{$_shell convenience
+function}.
+
 The utility @code{make} is often needed in development environments.
 You do not have to use the @code{shell} command for this purpose in
 @value{GDBN}:
@@ -12969,6 +12973,58 @@ Like the @code{$_gdb_setting_str} function, but works with
 Like the @code{$_gdb_setting} function, but works with
 @code{maintenance set} variables.
 
+@anchor{$_shell convenience function}
+@item $_shell (@var{command-string})
+@findex $_shell@r{, convenience function}
+
+Invoke a standard shell to execute @var{command-string}.
+@var{command-string} must be a string.  The shell runs on the host
+machine, the machine @value{GDBN} is running on.  Returns the
+command's exit status.  On Unix systems, a command which exits with a
+zero exit status has succeeded, and non-zero exit status indicates
+failure.  When a command terminates on a fatal signal whose number is
+@var{N}, @value{GDBN} uses the value 128+@var{N} as the exit status,
+as is standard in Unix shells.  Note that @var{N} is a host signal
+number, not a target signal number.  If you're cross debugging, the
+host vs target signal numbers may be completely unrelated.  The shell
+to run is determined in the same way as for the @code{shell} command.
+@xref{Shell Commands, ,Shell Commands}.
+
+@smallexample
+(@value{GDBP}) print $_shell("true")
+$1 = 0
+(@value{GDBP}) print $_shell("false")
+$2 = 1
+(@value{GDBP}) p $_shell("echo hello")
+hello
+$3 = 0
+(@value{GDBP}) p $_shell("foobar")
+bash: line 1: foobar: command not found
+$4 = 127
+@end smallexample
+
+This may also be useful in breakpoint conditions.  For example:
+
+@smallexample
+(@value{GDBP}) break function if $_shell("some command") == 0
+@end smallexample
+
+In this scenario, you'll want to make sure that the shell command you
+run in the breakpoint condition takes the least amount of time
+possible.  For example, avoid running a command that may block
+indefinitely, or that sleeps for a while before exiting.  Prefer a
+command or script which analyzes some state and exits immediately.
+This is important because the debugged program stops for the
+breakpoint every time, and then @value{GDBN} evaluates the breakpoint
+condition.  If the condition is false, the program is re-resumed
+transparently, without informing you of the stop.  A quick shell
+command thus avoids significantly slowing down the debugged program
+unnecessarily.
+
+Note: unlike the @code{shell} command, the @code{$_shell} convenience
+function does not affect the @code{$_shell_exitcode} and
+@code{$_shell_exitsignal} convenience variables.
+
 @end table
 
 The following functions require @value{GDBN} to be configured with
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index d0789a64401..7e73db0576a 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -606,6 +606,7 @@ set show_conv_list \
 	{$_cimag = <internal function _cimag>} \
 	{$_creal = <internal function _creal>} \
 	{$_isvoid = <internal function _isvoid>} \
+	{$_shell = <internal function _shell>} \
 	{$_gdb_maint_setting_str = <internal function _gdb_maint_setting_str>} \
 	{$_gdb_maint_setting = <internal function _gdb_maint_setting>} \
 	{$_gdb_setting_str = <internal function _gdb_setting_str>} \
diff --git a/gdb/testsuite/gdb.base/shell.exp b/gdb/testsuite/gdb.base/shell.exp
index 31cdcb41af5..ba1691ea2b0 100644
--- a/gdb/testsuite/gdb.base/shell.exp
+++ b/gdb/testsuite/gdb.base/shell.exp
@@ -41,6 +41,42 @@ if { ! [ishost *-*-mingw*] } {
     gdb_test "p \$_shell_exitsignal" " = 2" "shell interrupt exitsignal"
 }
 
+# Test the $_shell convenience function.
+
+with_test_prefix "\$_shell convenience function" {
+    # Simple commands, check the result code.
+    gdb_test "p \$_shell(\"true\")" " = 0"
+    gdb_test "p \$_shell(\"false\")" " = 1"
+
+    # Test command with arguments.
+    gdb_test "p \$_shell(\"echo foo\")" "foo\r\n\\$${decimal} = 0"
+
+    # Check the type of the result.
+    gdb_test "ptype \$_shell(\"true\")" "type = int"
+
+    # Test passing a non-literal string as command name.
+    gdb_test "p \$cmd = \"echo bar\"" " = \"echo bar\""
+    gdb_test "p \$_shell(\$cmd)" "bar\r\n\\$${decimal} = 0"
+
+    # Test executing a non-existing command.  The result is
+    # shell-dependent, but most (all?) POSIX-like shells return 127 in
+    # this case.
+    gdb_test "p \$_shell(\"non-existing-command-foo-bar-qux\")" " = 127"
+
+    gdb_test "p \$_shell" \
+	" = <internal function _shell>"
+    gdb_test "ptype \$_shell" \
+	"type = <internal function>"
+
+    # Test error scenarios.
+    gdb_test "p \$_shell()" \
+	"You must provide one argument for \\\$_shell\\\."
+    gdb_test "p \$_shell(\"a\", \"b\")" \
+	"You must provide one argument for \\\$_shell\\\."
+    gdb_test "p \$_shell(1)" \
+	"Argument must be a string\\\."
+}
+
 # Define the user command "foo", used to test "pipe" command.
 gdb_test_multiple "define foo" "define foo" {
     -re "End with"  {

base-commit: 5036bde964bc1a18282dde536a95aecd0d2c08fb
prerequisite-patch-id: 36aeba89f6bb6550d7bdf756dbd3c265f2d95d58
prerequisite-patch-id: cc6142da7c38349c0dfe95325b5b501998ba7e67
  

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> Date: Mon, 13 Feb 2023 15:11:45 +0000
> 
> I guess I should instead use that style here too?  I've done that now, and here's
> what we get:
> 
> (gdb) help function _shell
> $_shell - execute a shell command and return the result.
> 
>     Usage: $_shell (COMMAND)
> 
>     Arguments:
> 
>       COMMAND: The command to execute.  Must be a string.
> 
>     Returns:
>       The command's exit code: zero on success, non-zero otherwise.
> 
> 
> WDYT?

OK.

> >> +@anchor{$_shell convenience function}
> >> +@item $_shell (@var{command-string})
> >> +@findex $_shell@r{, convenience function}
> > 
> > @fined should be _before_ @item, so that the index records the
> > position of @item, and the Info reader places you before the @item
> > when you use index-search.
> 
> But the whole "Convenience Funs" node has has item/findex sorted like that,
> I just copied the preceding example.  Here's what we have:
> 
>  @item $_isvoid (@var{expr})
>  @findex $_isvoid@r{, convenience function}
>  ...
>  @item $_gdb_setting_str (@var{setting})
>  @findex $_gdb_setting_str@r{, convenience function}
>  ...
>  @item $_gdb_setting (@var{setting})
>  @findex $_gdb_setting@r{, convenience function}
>  ...
>  @item $_gdb_maint_setting_str (@var{setting})
>  @findex $_gdb_maint_setting_str@r{, convenience function}
>  ...
>  @item $_gdb_maint_setting (@var{setting})
>  @findex $_gdb_maint_setting@r{, convenience function}
> 
> Are all these wrong?  Sounds like they should all be fixed in one go.

Yes, they are all wrong.  The result is not a catastrophe, it's just
sub-optimal.

> I don't usually use the info reader, so I'm not really sure what to look for.

In an Info reader, type "i NAME" where NAME is the name of a function,
and see where it lands you.

> >> +Invoke a standard shell to execute @var{command-string}.  Returns the
> >           ^^^^^^^^^^^^^^^^
> > "the standard system shell" is better, I think.
> 
> Note this is the same wording used when describing the "shell" command:
> 
>  @table @code
>  @kindex shell
>  @kindex !
>  @cindex shell escape
>  @item shell @var{command-string}
>  @itemx !@var{command-string}
>  Invoke a standard shell to execute @var{command-string}.
>  Note that no space is needed between @code{!} and @var{command-string}.
>  On GNU and Unix systems, the environment variable @env{SHELL}, if it
>  exists, determines which shell to run.  Otherwise @value{GDBN} uses
>  the default shell (@file{/bin/sh} on GNU and Unix systems,
>  @file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
>  @end table
> 
> 
> Maybe the intention was to avoid "system" as @env{SHELL} may point
> to some shell the user installed, which doesn't come with or from
> the "system"?  So I'd prefer to use the same terminology in $_shell too.

I thought 'system' is not affected by $SHELL, but if it is, then
something like "Invoke a shell to execute @var{command-string}", I
think.  The explanation of how $SHELL affects the shell describes the
details.

> > Also, this text could benefit from a cross-reference to where we
> > describe the commands that convert from signal numbers to their
> > mnemonics, since that is system-dependent.  Maybe "info signal N" is
> > the right tool here?  If so, a cross-reference to "Signals" is what we
> > should have here.  (Is there a better way of asking GDB about signal
> > number N?)
> 
> I don't think it's the right tool here, because "info signal" and
> "handle", etc., are about target signals, signals that trigger in the
> inferior, which GDB is able to intercept.  OTOH, the $_shell function
> and the "shell" command run the shell on the host, so any fatal signal code
> is a host signal number, and doesn't go via any translation at all.

For native debugging, they are the same, of course.  So mentioning
that with the caveat of native debugging will help in some cases.

> I'll note that where we document $_shell_exitsignal, the convenience
> variable that the "shell" command sets, we don't describe the host signals
> either.

Then maybe we should have some text to tell the user how to map
numbers to signal mnemonics in the non-native case.

> I've moved the "The shell runs on the host machine, the machine @value{GDBN} is
> running on." sentence earlier, in case that helps set expectations, and
> added a couple sentences about host vs target signal numbers.  
> 
> I had also forgotten to document that the argument must be a string.
> 
> Here is the result:
> 
> "Invoke a standard shell to execute @var{command-string}.
> @var{command-string} must be a string.  The shell runs on the host
> machine, the machine @value{GDBN} is running on.  Returns the
> command's exit status.  On Unix systems, a command which exits with a
> zero exit status has succeeded, and non-zero exit status indicates
> failure.  When a command terminates on a fatal signal whose number is
> @var{N}, @value{GDBN} uses the value 128+@var{N} as the exit status,
> as is standard in Unix shells.  Note that @var{N} is a host signal
> number, not a target signal number.  If you're cross debugging, the
> host vs target signal numbers may be completely unrelated.  The shell
> to run is determined in the same way as for the @code{shell} command.
> @xref{Shell Commands, ,Shell Commands}."
> 
> WDYT?

OK, with the above nits.

> >> +In this scenario, you'll want to make sure that the shell command you
> >> +run in the breakpoint condition takes the least amount of time
> >> +possible.  This is important to minimize the time it takes to evaluate
> >> +the condition and re-resume the program if the condition turns out to
> >> +be false.
> > 
> > I understand what this says, but not what it means in practice.
> > Perhaps one or two additional sentences to help the reader understand
> > how to "make sure the shell command in a breakpoint condition takes
> > the least amount of time" would be in order?
> 
> Maybe something like this?
> 
> "In this scenario, you'll want to make sure that the shell command you
> run in the breakpoint condition takes the least amount of time
> possible.  For example, avoid running a command that may block
> indefinitely, or that sleeps for a while before exiting.  Prefer a
> command or script which analyzes some state and exits immediately.
> This is important because the debugged program stops for the
> breakpoint every time, and then @value{GDBN} evaluates the breakpoint
> condition.  If the condition is false, the program is re-resumed
> transparently, without informing you of the stop.  A quick shell
> command thus avoids significantly slowing down the debugged program
> unnecessarily."

Thanks, this is much more helpful.
  

On 2023-02-13 3:36 p.m., Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>

>>>> +Invoke a standard shell to execute @var{command-string}.  Returns the
>>>           ^^^^^^^^^^^^^^^^
>>> "the standard system shell" is better, I think.
>>
>> Note this is the same wording used when describing the "shell" command:
>>
>>  @table @code
>>  @kindex shell
>>  @kindex !
>>  @cindex shell escape
>>  @item shell @var{command-string}
>>  @itemx !@var{command-string}
>>  Invoke a standard shell to execute @var{command-string}.
>>  Note that no space is needed between @code{!} and @var{command-string}.
>>  On GNU and Unix systems, the environment variable @env{SHELL}, if it
>>  exists, determines which shell to run.  Otherwise @value{GDBN} uses
>>  the default shell (@file{/bin/sh} on GNU and Unix systems,
>>  @file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
>>  @end table
>>
>>
>> Maybe the intention was to avoid "system" as @env{SHELL} may point
>> to some shell the user installed, which doesn't come with or from
>> the "system"?  So I'd prefer to use the same terminology in $_shell too.
> 
> I thought 'system' is not affected by $SHELL, but if it is, then
> something like "Invoke a shell to execute @var{command-string}", I
> think.  The explanation of how $SHELL affects the shell describes the
> details.

I made that change, to both shell command and $_shell function.

But note, I meant, maybe the existing description of the "shell" command avoids
using the word "system" (not the 'system' function), as $SHELL may point to
a non-system shell.

But I see now value in saying "standard" either, so I'm going with your
suggestion, just saying "shell" should be sufficient.

> 
>>> Also, this text could benefit from a cross-reference to where we
>>> describe the commands that convert from signal numbers to their
>>> mnemonics, since that is system-dependent.  Maybe "info signal N" is
>>> the right tool here?  If so, a cross-reference to "Signals" is what we
>>> should have here.  (Is there a better way of asking GDB about signal
>>> number N?)
>>
>> I don't think it's the right tool here, because "info signal" and
>> "handle", etc., are about target signals, signals that trigger in the
>> inferior, which GDB is able to intercept.  OTOH, the $_shell function
>> and the "shell" command run the shell on the host, so any fatal signal code
>> is a host signal number, and doesn't go via any translation at all.
> 
> For native debugging, they are the same, of course.  So mentioning
> that with the caveat of native debugging will help in some cases.
> 
>> I'll note that where we document $_shell_exitsignal, the convenience
>> variable that the "shell" command sets, we don't describe the host signals
>> either.
> 
> Then maybe we should have some text to tell the user how to map
> numbers to signal mnemonics in the non-native case.

We really have no such mapping documented anywhere, not even for target signals,
and I don't think it's feasible to have it, as each different OS will have a
different mapping.

I think this is the best we can do:

 ...
 +shells.  Note that @var{N} is a host signal number, not a target
 +signal number.  If you're native debugging, they will be the same, but
 +if cross debugging, the host vs target signal numbers may be
 +completely unrelated.  Please consult your host operating system's
 +documentation for the mapping between host signal numbers and signal
 +names.  The shell to run is determined in the same way as for the
 ...

> 
>> I've moved the "The shell runs on the host machine, the machine @value{GDBN} is
>> running on." sentence earlier, in case that helps set expectations, and
>> added a couple sentences about host vs target signal numbers.  
>>
>> I had also forgotten to document that the argument must be a string.
>>
>> Here is the result:
>>
>> "Invoke a standard shell to execute @var{command-string}.
>> @var{command-string} must be a string.  The shell runs on the host
>> machine, the machine @value{GDBN} is running on.  Returns the
>> command's exit status.  On Unix systems, a command which exits with a
>> zero exit status has succeeded, and non-zero exit status indicates
>> failure.  When a command terminates on a fatal signal whose number is
>> @var{N}, @value{GDBN} uses the value 128+@var{N} as the exit status,
>> as is standard in Unix shells.  Note that @var{N} is a host signal
>> number, not a target signal number.  If you're cross debugging, the
>> host vs target signal numbers may be completely unrelated.  The shell
>> to run is determined in the same way as for the @code{shell} command.
>> @xref{Shell Commands, ,Shell Commands}."
>>
>> WDYT?
> 
> OK, with the above nits.
Would you mind taking another look?  Here's what it looks like currently:

From: Pedro Alves <pedro@palves.net>
Subject: [PATCH] Add new "$_shell(CMD)" internal function

For testing a following patch, I wanted a way to send a SIGINT to GDB
from a breakpoint condition.  And I didn't want to do it from a Python
breakpoint or Python function, as I wanted to exercise non-Python code
paths.  So I thought I'd add a new $_shell internal function, that
runs a command under the shell, and returns the exit code.  With this,
I could write:

  (gdb) b foo if $_shell("kill -SIGINT $gdb_pid") != 0 || <other condition>

I think this is generally useful, hence I'm proposing it here.

Here's the new function in action:

 (gdb) p $_shell("true")
 $1 = 0
 (gdb) p $_shell("false")
 $2 = 1
 (gdb) p $_shell("echo hello")
 hello
 $3 = 0
 (gdb) p $_shell("foobar")
 bash: line 1: foobar: command not found
 $4 = 127
 (gdb) help function _shell
 $_shell - execute a shell command and returns the result.
 Usage: $_shell (command)
 Returns the command's exit code: zero on success, non-zero otherwise.
 (gdb)

NEWS and manual changes included.

Reviewed-by: Eli Zaretskii <eliz@gnu.org>
Change-Id: I7e36d451ee6b428cbf41fded415ae2d6b4efaa4e
---
 gdb/NEWS                           | 10 ++++
 gdb/cli/cli-cmds.c                 | 96 ++++++++++++++++++++++++++++--
 gdb/doc/gdb.texinfo                | 60 ++++++++++++++++++-
 gdb/testsuite/gdb.base/default.exp |  1 +
 gdb/testsuite/gdb.base/shell.exp   | 36 +++++++++++
 5 files changed, 198 insertions(+), 5 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index b85923cf80d..0d3445438b1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -56,6 +56,16 @@ maintenance info frame-unwinders
   List the frame unwinders currently in effect, starting with the highest
   priority.
 
+* New convenience function "$_shell", to execute a shell command and
+  return the result.  This lets you run shell commands in expressions.
+  Some examples:
+
+   (gdb) p $_shell("true")
+   $1 = 0
+   (gdb) p $_shell("false")
+   $2 = 1
+   (gdb) break func if $_shell("some command") == 0
+
 * MI changes
 
 ** mi now reports 'no-history' as a stop reason when hitting the end of the
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 6c0d780face..7607fe59b05 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -39,6 +39,7 @@
 #include "gdbsupport/filestuff.h"
 #include "location.h"
 #include "block.h"
+#include "valprint.h"
 
 #include "ui-out.h"
 #include "interps.h"
@@ -873,6 +874,9 @@ exit_status_set_internal_vars (int exit_status)
 
   clear_internalvar (var_code);
   clear_internalvar (var_signal);
+
+  /* Keep the logic here in sync with shell_internal_fn.  */
+
   if (WIFEXITED (exit_status))
     set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
 #ifdef __MINGW32__
@@ -893,8 +897,11 @@ exit_status_set_internal_vars (int exit_status)
     warning (_("unexpected shell command exit status %d"), exit_status);
 }
 
-static void
-shell_escape (const char *arg, int from_tty)
+/* Run ARG under the shell, and return the exit status.  If ARG is
+   NULL, run an interactive shell.  */
+
+static int
+run_under_shell (const char *arg, int from_tty)
 {
 #if defined(CANT_FORK) || \
       (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
@@ -915,7 +922,7 @@ shell_escape (const char *arg, int from_tty)
      the shell command we just ran changed it.  */
   chdir (current_directory);
 #endif
-  exit_status_set_internal_vars (rc);
+  return rc;
 #else /* Can fork.  */
   int status, pid;
 
@@ -942,10 +949,21 @@ shell_escape (const char *arg, int from_tty)
     waitpid (pid, &status, 0);
   else
     error (_("Fork failed"));
-  exit_status_set_internal_vars (status);
+  return status;
 #endif /* Can fork.  */
 }
 
+/* Escape out to the shell to run ARG.  If ARG is NULL, launch and
+   interactive shell.  Sets $_shell_exitcode and $_shell_exitsignal
+   convenience variables based on the exits status.  */
+
+static void
+shell_escape (const char *arg, int from_tty)
+{
+  int status = run_under_shell (arg, from_tty);
+  exit_status_set_internal_vars (status);
+}
+
 /* Implementation of the "shell" command.  */
 
 static void
@@ -2417,6 +2435,63 @@ gdb_maint_setting_str_internal_fn (struct gdbarch *gdbarch,
   return str_value_from_setting (*show_cmd->var, gdbarch);
 }
 
+/* Implementation of the convenience function $_shell.  */
+
+static struct value *
+shell_internal_fn (struct gdbarch *gdbarch,
+		   const struct language_defn *language,
+		   void *cookie, int argc, struct value **argv)
+{
+  if (argc != 1)
+    error (_("You must provide one argument for $_shell."));
+
+  value *val = argv[0];
+  struct type *type = check_typedef (value_type (val));
+
+  if (!language->is_string_type_p (type))
+    error (_("Argument must be a string."));
+
+  value_print_options opts;
+  get_no_prettyformat_print_options (&opts);
+
+  string_file stream;
+  value_print (val, &stream, &opts);
+
+  /* We should always have two quote chars, which we'll strip.  */
+  gdb_assert (stream.size () >= 2);
+
+  /* Now strip them.  We don't need the original string, so it's
+     cheaper to do it in place, avoiding a string allocation.  */
+  std::string str = stream.release ();
+  str[str.size () - 1] = 0;
+  const char *command = str.c_str () + 1;
+
+  int exit_status = run_under_shell (command, 0);
+
+  struct type *int_type = builtin_type (gdbarch)->builtin_int;
+
+  /* Keep the logic here in sync with
+     exit_status_set_internal_vars.  */
+
+  if (WIFEXITED (exit_status))
+    return value_from_longest (int_type, WEXITSTATUS (exit_status));
+#ifdef __MINGW32__
+  else if (WIFSIGNALED (exit_status) && WTERMSIG (exit_status) == -1)
+    {
+      /* See exit_status_set_internal_vars.  */
+      return value_from_longest (int_type, exit_status);
+    }
+#endif
+  else if (WIFSIGNALED (exit_status))
+    {
+      /* (0x80 | SIGNO) is what most (all?) POSIX-like shells set as
+	 exit code on fatal signal termination.  */
+      return value_from_longest (int_type, 0x80 | WTERMSIG (exit_status));
+    }
+  else
+    return allocate_optimized_out_value (int_type);
+}
+
 void _initialize_cli_cmds ();
 void
 _initialize_cli_cmds ()
@@ -2606,6 +2681,19 @@ Some integer settings accept an unlimited value, returned\n\
 as 0 or -1 depending on the setting."),
 			 gdb_maint_setting_internal_fn, NULL);
 
+  add_internal_function ("_shell", _("\
+$_shell - execute a shell command and return the result.\n\
+\n\
+    Usage: $_shell (COMMAND)\n\
+\n\
+    Arguments:\n\
+\n\
+      COMMAND: The command to execute.  Must be a string.\n\
+\n\
+    Returns:\n\
+      The command's exit code: zero on success, non-zero otherwise."),
+			 shell_internal_fn, NULL);
+
   add_cmd ("commands", no_set_class, show_commands, _("\
 Show the history of commands you typed.\n\
 You can supply a command number to start with, or a `+' to start after\n\
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 79cb06e496e..d78627ad262 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1621,7 +1621,7 @@ just use the @code{shell} command.
 @cindex shell escape
 @item shell @var{command-string}
 @itemx !@var{command-string}
-Invoke a standard shell to execute @var{command-string}.
+Invoke a shell to execute @var{command-string}.
 Note that no space is needed between @code{!} and @var{command-string}.
 On GNU and Unix systems, the environment variable @env{SHELL}, if it
 exists, determines which shell to run.  Otherwise @value{GDBN} uses
@@ -1629,6 +1629,10 @@ the default shell (@file{/bin/sh} on GNU and Unix systems,
 @file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
 @end table
 
+You may also invoke shell commands from expressions, using the
+@code{$_shell} convenience function.  @xref{$_shell convenience
+function}.
+
 The utility @code{make} is often needed in development environments.
 You do not have to use the @code{shell} command for this purpose in
 @value{GDBN}:
@@ -12969,6 +12973,60 @@ Like the @code{$_gdb_setting_str} function, but works with
 Like the @code{$_gdb_setting} function, but works with
 @code{maintenance set} variables.
 
+@anchor{$_shell convenience function}
+@findex $_shell@r{, convenience function}
+@item $_shell (@var{command-string})
+
+Invoke a shell to execute @var{command-string}.  @var{command-string}
+must be a string.  The shell runs on the host machine, the machine
+@value{GDBN} is running on.  Returns the command's exit status.  On
+Unix systems, a command which exits with a zero exit status has
+succeeded, and non-zero exit status indicates failure.  When a command
+terminates on a fatal signal whose number is @var{N}, @value{GDBN}
+uses the value 128+@var{N} as the exit status, as is standard in Unix
+shells.  Note that @var{N} is a host signal number, not a target
+signal number.  If you're native debugging, they will be the same, but
+if cross debugging, the host vs target signal numbers may be
+completely unrelated.  Please consult your host operating system's
+documentation for the mapping between host signal numbers and signal
+names.  The shell to run is determined in the same way as for the
+@code{shell} command.  @xref{Shell Commands, ,Shell Commands}.
+
+@smallexample
+(@value{GDBP}) print $_shell("true")
+$1 = 0
+(@value{GDBP}) print $_shell("false")
+$2 = 1
+(@value{GDBP}) p $_shell("echo hello")
+hello
+$3 = 0
+(@value{GDBP}) p $_shell("foobar")
+bash: line 1: foobar: command not found
+$4 = 127
+@end smallexample
+
+This may also be useful in breakpoint conditions.  For example:
+
+@smallexample
+(@value{GDBP}) break function if $_shell("some command") == 0
+@end smallexample
+
+In this scenario, you'll want to make sure that the shell command you
+run in the breakpoint condition takes the least amount of time
+possible.  For example, avoid running a command that may block
+indefinitely, or that sleeps for a while before exiting.  Prefer a
+command or script which analyzes some state and exits immediately.
+This is important because the debugged program stops for the
+breakpoint every time, and then @value{GDBN} evaluates the breakpoint
+condition.  If the condition is false, the program is re-resumed
+transparently, without informing you of the stop.  A quick shell
+command thus avoids significantly slowing down the debugged program
+unnecessarily.
+
+Note: unlike the @code{shell} command, the @code{$_shell} convenience
+function does not affect the @code{$_shell_exitcode} and
+@code{$_shell_exitsignal} convenience variables.
+
 @end table
 
 The following functions require @value{GDBN} to be configured with
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index d0789a64401..7e73db0576a 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -606,6 +606,7 @@ set show_conv_list \
 	{$_cimag = <internal function _cimag>} \
 	{$_creal = <internal function _creal>} \
 	{$_isvoid = <internal function _isvoid>} \
+	{$_shell = <internal function _shell>} \
 	{$_gdb_maint_setting_str = <internal function _gdb_maint_setting_str>} \
 	{$_gdb_maint_setting = <internal function _gdb_maint_setting>} \
 	{$_gdb_setting_str = <internal function _gdb_setting_str>} \
diff --git a/gdb/testsuite/gdb.base/shell.exp b/gdb/testsuite/gdb.base/shell.exp
index 31cdcb41af5..ba1691ea2b0 100644
--- a/gdb/testsuite/gdb.base/shell.exp
+++ b/gdb/testsuite/gdb.base/shell.exp
@@ -41,6 +41,42 @@ if { ! [ishost *-*-mingw*] } {
     gdb_test "p \$_shell_exitsignal" " = 2" "shell interrupt exitsignal"
 }
 
+# Test the $_shell convenience function.
+
+with_test_prefix "\$_shell convenience function" {
+    # Simple commands, check the result code.
+    gdb_test "p \$_shell(\"true\")" " = 0"
+    gdb_test "p \$_shell(\"false\")" " = 1"
+
+    # Test command with arguments.
+    gdb_test "p \$_shell(\"echo foo\")" "foo\r\n\\$${decimal} = 0"
+
+    # Check the type of the result.
+    gdb_test "ptype \$_shell(\"true\")" "type = int"
+
+    # Test passing a non-literal string as command name.
+    gdb_test "p \$cmd = \"echo bar\"" " = \"echo bar\""
+    gdb_test "p \$_shell(\$cmd)" "bar\r\n\\$${decimal} = 0"
+
+    # Test executing a non-existing command.  The result is
+    # shell-dependent, but most (all?) POSIX-like shells return 127 in
+    # this case.
+    gdb_test "p \$_shell(\"non-existing-command-foo-bar-qux\")" " = 127"
+
+    gdb_test "p \$_shell" \
+	" = <internal function _shell>"
+    gdb_test "ptype \$_shell" \
+	"type = <internal function>"
+
+    # Test error scenarios.
+    gdb_test "p \$_shell()" \
+	"You must provide one argument for \\\$_shell\\\."
+    gdb_test "p \$_shell(\"a\", \"b\")" \
+	"You must provide one argument for \\\$_shell\\\."
+    gdb_test "p \$_shell(1)" \
+	"Argument must be a string\\\."
+}
+
 # Define the user command "foo", used to test "pipe" command.
 gdb_test_multiple "define foo" "define foo" {
     -re "End with"  {

base-commit: 5036bde964bc1a18282dde536a95aecd0d2c08fb
  

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> Date: Mon, 13 Feb 2023 17:27:39 +0000
> 
> > OK, with the above nits.
> Would you mind taking another look?  Here's what it looks like currently:

Thanks, LGTM.
  

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> From: Pedro Alves <pedro@palves.net>
Pedro> Subject: [PATCH] Add new "$_shell(CMD)" internal function
[...]

Looks good, thank you.
Approved-By: Tom Tromey <tom@tromey.com>

Tom