This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 6/6] Implement proper "startup-with-shell" support on gdbserver
- From: Luis Machado <lgustavo at codesourcery dot com>
- To: Sergio Durigan Junior <sergiodj at redhat dot com>, GDB Patches <gdb-patches at sourceware dot org>
- Cc: <palves at redhat dot com>
- Date: Mon, 26 Dec 2016 15:34:19 -0600
- Subject: Re: [PATCH 6/6] Implement proper "startup-with-shell" support on gdbserver
- Authentication-results: sourceware.org; auth=none
- References: <1482464361-4068-1-git-send-email-sergiodj@redhat.com> <1482464361-4068-7-git-send-email-sergiodj@redhat.com>
- Reply-to: Luis Machado <lgustavo at codesourcery dot com>
On 12/22/2016 09:39 PM, Sergio Durigan Junior wrote:
This patch implements the proper support for the "startup-with-shell"
feature on gdbserver. A new packet is added, QStartupShell, and this
packet is sent on initialization. If the host sends a
"QStartupShell:" (i.e., without arguments), it means the inferior
shall be started without using a shell. Otherwise, the argument on
the packet is the shell to be used to start the inferior.
I decided to use the "startup-with-shell" setting from the host in
order to decide whether to start the remote inferior using a shell or
not. However, since there is no easy way to change which shell to use
when starting things, I have also added a new command, "set/show
remote startup-shell", which can contain the shell that the remote
target will use.
A documentation patch is included, along with a new testcase for the
feature.
gdb/ChangeLog:
2016-12-22 Sergio Durigan Junior <sergiodj@redhat.com>
* remote.c (remote_startup_shell_var): New variable.
(set_remote_startup_shell): New function.
(show_remote_startup_shell): Likewise.
Add PACKET_QStartupShell.
(remote_start_remote): Handle new PACKET_QStartupShell.
(struct protocol_feature remote_protocol_features): New entry for
PACKET_QStartupShell.
(_initialize_remote): Call "add_packet_config_cmd" for
QStartupShell. Add new set/show command "set remote
startup-shell".
gdb/gdbserver/ChangeLog:
2016-12-22 Sergio Durigan Junior <sergiodj@redhat.com>
* server.c (handle_general_set): Handle new packet
"QStartupShell".
(handle_query): Add "QStartupShell" to the list of supported
packets.
gdb/testsuite/ChangeLog:
2016-12-22 Sergio Durigan Junior <sergiodj@redhat.com>
* gdb.server/startup-with-shell.c: New file.
* gdb.server/startup-with-shell.exp: Likewise.
gdb/doc/ChangeLog:
2016-12-22 Sergio Durigan Junior <sergiodj@redhat.com>
* gdb.texinfo (startup-with-shell): Add note mentioning that the
setting is also used by remote targets.
(set remote startup-shell): New item.
---
gdb/doc/gdb.texinfo | 14 +++
gdb/gdbserver/server.c | 22 ++++-
gdb/remote.c | 62 +++++++++++++
gdb/testsuite/gdb.server/startup-with-shell.c | 12 +++
gdb/testsuite/gdb.server/startup-with-shell.exp | 110 ++++++++++++++++++++++++
5 files changed, 219 insertions(+), 1 deletion(-)
create mode 100644 gdb/testsuite/gdb.server/startup-with-shell.c
create mode 100644 gdb/testsuite/gdb.server/startup-with-shell.exp
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a0de7d1..3250cae 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2174,6 +2174,10 @@ initialization file---such as @file{.cshrc} for C-shell,
$@file{.zshenv} for the Z shell, or the file specified in the
@samp{BASH_ENV} environment variable for BASH.
+This setting is also used by @code{target extended-remote} to
+determine whether the target system will start the inferior using the
+shell (@pxref{set remote startup-shell}).
+
@anchor{set auto-connect-native-target}
@kindex set auto-connect-native-target
@item set auto-connect-native-target
@@ -20486,6 +20490,16 @@ extended-remote}. This should be set to a filename valid on the
target system. If it is not set, the target will use a default
filename (e.g.@: the last program run).
+@item set remote startup-shell @var{shell}
+@itemx show remote startup-shell
+@anchor{set remote startup-shell}
+@cindex startup shell, for remote target
+Set the shell that is going to be used to start the inferior with
+@code{target extended-remote}. This should be set to a valid shell on
+the target system, and is only effective when
+@code{startup-with-shell} is on. If it is not set, the target system
+will use the shell pointed by the @code{SHELL} environment variable.
+
@item set remote interrupt-sequence
@cindex interrupt remote programs
@cindex select Ctrl-C, BREAK or BREAK-g
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 55eebd9..5faf0b5 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -863,6 +863,26 @@ handle_general_set (char *own_buf)
return;
}
+ if (startswith (own_buf, "QStartupShell:"))
+ {
+ const char *p = own_buf + strlen ("QStartupShell:");
+
+ if (*p == '\0')
+ {
+ startup_with_shell = 0;
+ xfree (startup_shell);
+ }
+ else
+ {
+ startup_with_shell = 1;
+ xfree (startup_shell);
+ startup_shell = xstrdup (p);
+ }
+
+ write_ok (own_buf);
+ return;
+ }
+
/* Otherwise we didn't know what packet it was. Say we didn't
understand it. */
own_buf[0] = 0;
@@ -2299,7 +2319,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
}
sprintf (own_buf,
- "PacketSize=%x;QPassSignals+;QProgramSignals+",
+ "PacketSize=%x;QPassSignals+;QProgramSignals+;QStartupShell+",
PBUFSIZ - 1);
if (target_supports_catch_syscall ())
diff --git a/gdb/remote.c b/gdb/remote.c
index 0b5f837..2d2a87a 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -86,6 +86,12 @@ static const struct program_space_data *remote_pspace_data;
location of the remote exec-file value. */
static char *remote_exec_file_var;
+/* The variable that holds the startup shell that will be used by the
+ remote target to start the inferior. If this variable is NULL or
+ empty, we use the value of 'get_startup_shell'. */
+
Spurious newline.
+static char *remote_startup_shell_var;
+
/* The size to align memory write packets, when practical. The protocol
does not guarantee any alignment, and gdb will generate short
writes and unaligned writes, but even as a best-effort attempt this
@@ -727,6 +733,22 @@ show_remote_exec_file (struct ui_file *file, int from_tty,
fprintf_filtered (file, "%s\n", remote_exec_file_var);
}
+/* The "set/show remote startup-shell" set command hook. */
+
Technically shouldn't we break this up into a couple comments?
Or maybe "... set command hooks."
+static void
+set_remote_startup_shell (char *ignored, int from_tty,
+ struct cmd_list_element *c)
+{
+ gdb_assert (remote_startup_shell_var != NULL);
+}
+
+static void
+show_remote_startup_shell (struct ui_file *file, int from_tty,
+ struct cmd_list_element *cmd, const char *value)
+{
+ fprintf_filtered (file, "%s\n", remote_startup_shell_var);
+}
+
static int
compare_pnums (const void *lhs_, const void *rhs_)
{
@@ -1423,6 +1445,7 @@ enum {
PACKET_QPassSignals,
PACKET_QCatchSyscalls,
PACKET_QProgramSignals,
+ PACKET_QStartupShell,
PACKET_qCRC,
PACKET_qSearch_memory,
PACKET_vAttach,
@@ -4074,6 +4097,31 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
if (packet_support (PACKET_QAllow) != PACKET_DISABLE)
remote_set_permissions (target);
+ if (packet_support (PACKET_QStartupShell) != PACKET_DISABLE)
+ {
+ if (startup_with_shell)
+ {
+ const char *sh;
+
+ if (remote_startup_shell_var != NULL
+ && *remote_startup_shell_var != '\0')
+ sh = remote_startup_shell_var;
+ else
+ sh = get_startup_shell ();
+
+ xsnprintf (rs->buf, get_remote_packet_size (),
+ "QStartupShell:%s", sh);
+ }
+ else
+ xsnprintf (rs->buf, get_remote_packet_size (),
+ "QStartupShell:");
+
+ putpkt (rs->buf);
+ getpkt (&rs->buf, &rs->buf_size, 0);
+ if (strcmp (rs->buf, "OK") != 0)
+ error (_("Could not set remote startup-with-shell"));
+ }
+
/* gdbserver < 7.7 (before its fix from 2013-12-11) did reply to any
unknown 'v' packet with string "OK". "OK" gets interpreted by GDB
as a reply to known packet. For packet "vFile:setfs:" it is an
@@ -4628,6 +4676,8 @@ static const struct protocol_feature remote_protocol_features[] = {
PACKET_QCatchSyscalls },
{ "QProgramSignals", PACKET_DISABLE, remote_supported_packet,
PACKET_QProgramSignals },
+ { "QStartupShell", PACKET_DISABLE, remote_supported_packet,
+ PACKET_QStartupShell },
{ "QStartNoAckMode", PACKET_DISABLE, remote_supported_packet,
PACKET_QStartNoAckMode },
{ "multiprocess", PACKET_DISABLE, remote_supported_packet,
@@ -14081,6 +14131,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_QProgramSignals],
"QProgramSignals", "program-signals", 0);
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_QStartupShell],
+ "QStartupShell", "startup-shell", 0);
+
add_packet_config_cmd (&remote_protocol_packets[PACKET_qSymbol],
"qSymbol", "symbol-lookup", 0);
@@ -14380,6 +14433,15 @@ Show the remote pathname for \"run\""), NULL,
&remote_set_cmdlist,
&remote_show_cmdlist);
+ add_setshow_string_noescape_cmd ("startup-shell", class_files,
+ &remote_startup_shell_var, _("\
+Set the remote shell for starting the inferior"), _("\
+Show the remote shell for starting the inferior"), NULL,
+ set_remote_startup_shell,
+ show_remote_startup_shell,
+ &remote_set_cmdlist,
+ &remote_show_cmdlist);
+
add_setshow_boolean_cmd ("range-stepping", class_run,
&use_range_stepping, _("\
Enable or disable range stepping."), _("\
diff --git a/gdb/testsuite/gdb.server/startup-with-shell.c b/gdb/testsuite/gdb.server/startup-with-shell.c
new file mode 100644
index 0000000..ba77b10
--- /dev/null
+++ b/gdb/testsuite/gdb.server/startup-with-shell.c
@@ -0,0 +1,12 @@
Missing copyright notice in the source file?
+#include <stdio.h>
+
+int
+main (int argc, char *argv[])
+{
+ int i;
+
+ for (i = 0; argv[i] != NULL; ++i)
+ printf ("ARG %d = %s\n", i, argv[i]);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.server/startup-with-shell.exp b/gdb/testsuite/gdb.server/startup-with-shell.exp
new file mode 100644
index 0000000..1bf3654
--- /dev/null
+++ b/gdb/testsuite/gdb.server/startup-with-shell.exp
@@ -0,0 +1,110 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test startup-with-shell support using extended-remote.
+
+load_lib gdbserver-support.exp
+
+standard_testfile
+
+if { [skip_gdbserver_tests] } {
untested "skipping gdbserver tests"
+ return 0
+}
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
+ untested "failed to compile"
The above untested call is not needed as prepare_for_testing will handle
it with the "failed to prepare" message.
+ return -1
+}
+
+# Initial setup for simple test (wildcard expansion, variable substitution).
+
+proc initial_setup_simple { startup_with_shell run_args } {
+ global hex decimal binfile
+
+ clean_restart $binfile
+ # Make sure we're disconnected, in case we're testing with an
+ # extended-remote board, therefore already connected.
+ gdb_test "disconnect" ".*"
+
+ gdb_test_no_output "set startup-with-shell $startup_with_shell"
+
+ set target_exec [gdbserver_download_current_prog]
+ gdbserver_start_extended
+ gdb_test_no_output "set remote exec-file $target_exec" "set remote exec-file"
+
+ gdb_breakpoint main
+
+ gdb_test "run $run_args" \
+ "Breakpoint ${decimal}, main \\(argc=${decimal}, argv=${hex}\\).*" \
+ "run to main"
+}
+
+proc invalid_remote_shell_test { } {
+ global binfile
+
+ clean_restart $binfile
+ # Make sure we're disconnected, in case we're testing with an
+ # extended-remote board, therefore already connected.
+ gdb_test "disconnect" ".*"
+
+ gdb_test_no_output "set startup-with-shell on"
+ gdb_test_no_output "set remote startup-shell /path/to/invalid/shell"
+
+ set target_exec [gdbserver_download_current_prog]
+ gdbserver_start_extended
+ gdb_test_no_output "set remote exec-file $target_exec" "set remote exec-file"
+
+ gdb_breakpoint main
+
+ gdb_test "run" \
+ "Running \"$binfile\" on the remote target failed" \
+ "run to main (should fail)"
+}
+
+## Doing the actual tests
+
+with_test_prefix "startup_with_shell = on; run_args = *.log" {
+ initial_setup_simple "on" "*.log"
+ gdb_test "print argv\[1\]" "\\\$$decimal = $hex \"config\.log\"" \
+ "testing first argument"
+}
+
+with_test_prefix "startup_with_shell = off; run_args = *.log" {
+ initial_setup_simple "off" "*.log"
+ gdb_test "print argv\[1\]" "\\\$$decimal = $hex \"\\\*\.log\"" \
+ "testing first argument"
+}
+
+with_test_prefix "startup_with_shell = on; run_args = \$TEST" {
+ set env(TEST) "1234"
+ initial_setup_simple "on" "\$TEST"
+ gdb_test "print argv\[1\]" "\\\$$decimal = $hex \"1234\"" \
+ "testing first argument"
+ unset env(TEST)
+}
+
+with_test_prefix "startup_with_shell = off; run_args = \$TEST" {
+ set env(TEST) "1234"
+ initial_setup_simple "off" "\$TEST"
+ gdb_test "print argv\[1\]" "\\\$$decimal = $hex \"\\\$TEST\"" \
+ "testing first argument"
+ unset env(TEST)
+}
+
+with_test_prefix "startup_with_shell = on; invalid remote startup-shell" {
+ invalid_remote_shell_test
+}
Otherwise looks OK.