From: Pedro Alves <pedro@palves.net>
To: Eli Zaretskii <eliz@gnu.org>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function
Date: Mon, 13 Feb 2023 17:27:39 +0000 [thread overview]
Message-ID: <4867e243-13db-fe1a-3cf6-e31150e6d928@palves.net> (raw)
In-Reply-To: <833579fugd.fsf@gnu.org>
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
--
2.36.0
next prev parent reply other threads:[~2023-02-13 17:27 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-10 23:35 [PATCH 0/6] Don't throw quit while handling inferior events Pedro Alves
2023-02-10 23:35 ` [PATCH 1/6] Fix "ptype INTERNAL_FUNC" (PR gdb/30105) Pedro Alves
2023-02-13 16:02 ` Andrew Burgess
2023-02-14 15:26 ` Tom Tromey
2023-02-15 21:10 ` Pedro Alves
2023-02-15 22:04 ` Tom Tromey
2023-02-10 23:36 ` [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages Pedro Alves
2023-02-13 16:02 ` Andrew Burgess
2023-02-14 15:30 ` Tom Tromey
2023-02-15 13:38 ` Pedro Alves
2023-02-15 15:13 ` Pedro Alves
2023-02-15 16:56 ` Tom Tromey
2023-02-15 21:04 ` [PATCH] Move TYPE_CODE_INTERNAL_FUNCTION type printing to common code (was: Re: [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages) Pedro Alves
2023-02-20 15:28 ` Andrew Burgess
2023-02-10 23:36 ` [PATCH 3/6] Add new "$_shell(CMD)" internal function Pedro Alves
2023-02-11 8:02 ` Eli Zaretskii
2023-02-13 15:11 ` Pedro Alves
2023-02-13 15:36 ` Eli Zaretskii
2023-02-13 16:47 ` [PATCH] gdb/manual: Move @findex entries (was: Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function) Pedro Alves
2023-02-13 17:00 ` Eli Zaretskii
2023-02-13 17:27 ` Pedro Alves [this message]
2023-02-13 18:41 ` [PATCH 3/6] Add new "$_shell(CMD)" internal function Eli Zaretskii
2023-02-14 15:38 ` Tom Tromey
2023-02-10 23:36 ` [PATCH 4/6] Don't throw quit while handling inferior events Pedro Alves
2023-02-14 15:50 ` Tom Tromey
2023-02-10 23:36 ` [PATCH 5/6] GC get_active_ext_lang Pedro Alves
2023-02-14 15:39 ` Tom Tromey
2023-02-10 23:36 ` [PATCH 6/6] Don't throw quit while handling inferior events, part II Pedro Alves
2023-02-14 15:54 ` Tom Tromey
2023-02-15 21:16 ` Pedro Alves
2023-02-15 21:24 ` Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4867e243-13db-fe1a-3cf6-e31150e6d928@palves.net \
--to=pedro@palves.net \
--cc=eliz@gnu.org \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).