From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) by sourceware.org (Postfix) with ESMTPS id 2C6BC3858C20 for ; Mon, 13 Feb 2023 17:27:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2C6BC3858C20 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f47.google.com with SMTP id m10so4127728wrn.4 for ; Mon, 13 Feb 2023 09:27:42 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=RQuycbuEyernEjCN++mbuA0uik9oIyLYOPVjYmo9N9A=; b=wODJ0yzyRhkYlSP5N5abzgq2D0Y9WT13j4PIXDsh4nJTdGBWpl/4fX+/D7sVmaqMAT w7cuN2yqbpsDZUsl+QdcSGNjKfVQO63j4DoJL9739SrH5Cqn3EzgQSXCs8x8iH/0TJGe I7eGVY+rrZacX+w5TClhONYgOsN2GG1EeZBSXMpZUw+IJLAQnvgHa0ntB6MnAyYsAQGU txnKwiay65ftviJcdJtunlG7v/ycn8rJxURt9BUUFMLOCDRqDwDZZKmyDbRkWBf51MI2 SKK7yyUmyf0/5y5oBnrLC1jU4n2JNqT2l+z/cVnyD02bOPTkRdJEPd3Tkm6BzntcT6Hb eiQQ== X-Gm-Message-State: AO0yUKUkInETRgao09eXyXWYsx5FMNNUHPc04zhwKr1w6JVYH5zbaTdF E1IudSsM8s3rQNEXuro00NLQimqvIlFdwQ== X-Google-Smtp-Source: AK7set+9iuZfZETLcJRvfxWCYEGtlhjS2RBLn0246wMnfwqH8Dlpg3kXcTyulDInLAY3B2VgYGb6dw== X-Received: by 2002:a5d:6212:0:b0:2c3:f0a6:43ee with SMTP id y18-20020a5d6212000000b002c3f0a643eemr14191734wru.20.1676309260509; Mon, 13 Feb 2023 09:27:40 -0800 (PST) Received: from ?IPv6:2001:8a0:f92b:9e00::1fe? ([2001:8a0:f92b:9e00::1fe]) by smtp.gmail.com with ESMTPSA id t9-20020adfeb89000000b002be0b1e556esm10896387wrn.59.2023.02.13.09.27.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 13 Feb 2023 09:27:40 -0800 (PST) Subject: Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function To: Eli Zaretskii Cc: gdb-patches@sourceware.org References: <20230210233604.2228450-1-pedro@palves.net> <20230210233604.2228450-4-pedro@palves.net> <83r0uwk4tm.fsf@gnu.org> <439ab6f5-ec2d-87ef-d84c-3ace9ead93a8@palves.net> <833579fugd.fsf@gnu.org> From: Pedro Alves Message-ID: <4867e243-13db-fe1a-3cf6-e31150e6d928@palves.net> Date: Mon, 13 Feb 2023 17:27:39 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <833579fugd.fsf@gnu.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2023-02-13 3:36 p.m., Eli Zaretskii wrote: >> Cc: gdb-patches@sourceware.org >> From: Pedro Alves >>>> +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 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 || 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 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 = } \ {$_creal = } \ {$_isvoid = } \ + {$_shell = } \ {$_gdb_maint_setting_str = } \ {$_gdb_maint_setting = } \ {$_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" \ + " = " + gdb_test "ptype \$_shell" \ + "type = " + + # 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