From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 126165 invoked by alias); 1 Feb 2017 19:07:08 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 126153 invoked by uid 89); 1 Feb 2017 19:07:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.2 required=5.0 tests=AWL,BAYES_40,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=@end, @var, @cindex, cindex X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 01 Feb 2017 19:06:57 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1cZ0F5-0002Hq-Dz from Luis_Gustavo@mentor.com ; Wed, 01 Feb 2017 11:06:55 -0800 Received: from [172.30.10.1] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Wed, 1 Feb 2017 11:06:52 -0800 Reply-To: Luis Machado Subject: Re: [PATCH v2 6/6] Implement proper "startup-with-shell" support on gdbserver References: <1482464361-4068-1-git-send-email-sergiodj@redhat.com> <20170118153605.4610-1-sergiodj@redhat.com> <20170118153605.4610-7-sergiodj@redhat.com> To: Sergio Durigan Junior , GDB Patches CC: Pedro Alves , Eli Zaretskii From: Luis Machado Message-ID: Date: Wed, 01 Feb 2017 19:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20170118153605.4610-7-sergiodj@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-02.mgc.mentorg.com (147.34.90.202) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2017-02/txt/msg00041.txt.bz2 On 01/18/2017 09:36 AM, Sergio Durigan Junior wrote: > This patch implements the proper support for the "startup-with-shell" > feature on gdbserver. A new packet is added, QStartupWithShell, and > it is sent on initialization. If the host sends a > "QStartupWithShell:1", it means the inferior shall be started using a > shell. If the host sends a "QStartupWithShell:0", it means the > inferior shall be started without using a shell. Any other value is > considered an error. > > There is no way to remotely set the shell that will be used by the > target to start the inferior. In order to do that, the user must > start gdbserver while providing a shell via the $SHELL environment > variable. The same is true for the host side. > > The "set startup-with-shell" setting from the host side is used to > decide whether to start the remote inferior using a shell. This same > setting is also used to decide whether to use a shell to start the > host inferior; this means that it is not really possible to start the > inferior using different mechanisms on target and host. > > A documentation patch is included, along with a new testcase for the > feature. > > gdb/ChangeLog: > 2017-01-17 Sergio Durigan Junior > > * NEWS (Changes since GDB 7.12): Announce that GDBserver is now > able to start inferiors using a shell. > (New remote packets): Announce new packet "QStartupWithShell". > * remote.c: Add PACKET_QStartupWithShell. > (remote_start_remote): Handle new PACKET_QStartupWithShell. > (remote_protocol_features) : New entry for > PACKET_QStartupWithShell. > (_initialize_remote): Call "add_packet_config_cmd" for > QStartupShell. > > gdb/gdbserver/ChangeLog: > 2017-01-17 Sergio Durigan Junior > > * server.c (handle_general_set): Handle new packet > "QStartupWithShell". > (handle_query): Add "QStartupWithShell" to the list of supported > packets. > (gdbserver_usage): Add help text explaining the > new "--startup-with-shell" and "--no-startup-with-shell" CLI > options. > (captured_main): Recognize and act upon the presence of the new > CLI options. > > gdb/testsuite/ChangeLog: > 2017-01-17 Sergio Durigan Junior > > * gdb.server/startup-with-shell.c: New file. > * gdb.server/startup-with-shell.exp: Likewise. > > gdb/doc/ChangeLog: > 2017-01-17 Sergio Durigan Junior > > * gdb.texinfo (Starting) : Add @anchor. > (Connecting) : Add "startup-with-shell" > and "QStartupWithShell" to the table. > (Remote Protocol) : New item, explaining the > packet. > --- > gdb/NEWS | 10 +++ > gdb/doc/gdb.texinfo | 26 ++++++++ > gdb/gdbserver/server.c | 36 ++++++++++- > gdb/remote.c | 20 ++++++ > gdb/testsuite/gdb.server/startup-with-shell.c | 29 +++++++++ > gdb/testsuite/gdb.server/startup-with-shell.exp | 83 +++++++++++++++++++++++++ > 6 files changed, 203 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/NEWS b/gdb/NEWS > index b976815..9bf8df4 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -3,6 +3,13 @@ > > *** Changes since GDB 7.12 > > +* GDBserver is now able to start inferiors using a shell. When using > + "target extended-remote", the host GDB honors the value of "set > + startup-with-shell" in order to inform GDBserver whether the remote > + inferior should be started with a shell or not. When using "target > + remote", it is possible to disable the startup with shell by using > + the new parameter "--no-startup-with-shell" when starting GDBserver. > + > * Building GDB and GDBserver now requires a C++11 compiler. > > For example, GCC 4.8 or later. > @@ -356,6 +363,9 @@ show max-value-size > > * New remote packets > > +QStartupWithShell > + Indicates whether the inferior must be started with a shell or not. > + > exec stop reason > Indicates that an exec system call was executed. > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 2b6b654..bfe11de 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -2153,6 +2153,7 @@ This command is available when debugging locally on most targets, excluding > @sc{djgpp}, Cygwin, MS Windows, and QNX Neutrino. > > @kindex set startup-with-shell > +@anchor{set startup-with-shell} > @item set startup-with-shell > @itemx set startup-with-shell on > @itemx set startup-with-shell off > @@ -20772,6 +20773,10 @@ are: > @tab @code{QDisableRandomization} > @tab @code{set disable-randomization} > > +@item @code{startup-with-shell} > +@tab @code{QStartupWithShell} > +@tab @code{set startup-with-shell} > + > @item @code{conditional-breakpoints-packet} > @tab @code{Z0 and Z1} > @tab @code{Support for target-side breakpoint condition evaluation} > @@ -36330,6 +36335,27 @@ by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}). > This should only be done on targets that actually support disabling > address space randomization. > > +@item QStartupWithShell:@var{value} > +@cindex startup with shell, remote request > +@cindex @samp{QStartupWithShell} packet > +On UNIX-like targets, it is possible to start the inferior using a > +shell program. This is the default behavior on both @value{GDBN} and > +@command{gdbserver} (@pxref{set startup-with-shell}). This packet is > +used to inform @command{gdbserver} whether it should start the > +inferior using a shell or not. Missing documentation aboue what VALUE should be and the range of values it may contain? > + > +This packet is only available in extended mode (@pxref{extended > +mode}). > + > +Reply: > +@table @samp > +@item OK > +The request succeeded. > + > +@item E @var{nn} > +An error occurred. The error number @var{nn} is give as hex digits. > +@end table > + > @item qfThreadInfo > @itemx qsThreadInfo > @cindex list active threads, remote request > diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c > index a26ad52..56bd613 100644 > --- a/gdb/gdbserver/server.c > +++ b/gdb/gdbserver/server.c > @@ -866,6 +866,31 @@ handle_general_set (char *own_buf) > return; > } > > + if (startswith (own_buf, "QStartupWithShell:")) > + { > + char *value = own_buf + strlen ("QStartupWithShell:"); > + > + if (strcmp (value, "1") == 0) > + startup_with_shell = 1; > + else if (strcmp (value, "0") == 0) > + startup_with_shell = 0; > + else > + { > + /* Unknown value. */ > + fprintf (stderr, "Unknown value to startup-with-shell: %s\n", > + own_buf); > + write_enn (own_buf); > + return; > + } > + > + if (remote_debug) > + debug_printf (_("[Inferior will %s started with shell]"), > + startup_with_shell ? "be" : "not be"); > + > + write_ok (own_buf); > + return; > + } > + > /* Otherwise we didn't know what packet it was. Say we didn't > understand it. */ > own_buf[0] = 0; > @@ -2302,7 +2327,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+;QStartupWithShell+", > PBUFSIZ - 1); > > if (target_supports_catch_syscall ()) > @@ -3396,6 +3421,11 @@ gdbserver_usage (FILE *stream) > " --no-disable-randomization\n" > " Don't disable address space randomization when\n" > " starting PROG.\n" > + " --startup-with-shell\n" > + " Start PROG using a shell.\n" > + " --no-startup-with-shell\n" > + " Don't start PROG using a shell (i.e., use the exec*\n" > + " family of functions).\n" > "\n" > "Debug options:\n" > "\n" > @@ -3679,6 +3709,10 @@ captured_main (int argc, char *argv[]) > disable_randomization = 1; > else if (strcmp (*next_arg, "--no-disable-randomization") == 0) > disable_randomization = 0; > + else if (strcmp (*next_arg, "--startup-with-shell") == 0) > + startup_with_shell = 1; > + else if (strcmp (*next_arg, "--no-startup-with-shell") == 0) > + startup_with_shell = 0; > else if (strcmp (*next_arg, "--once") == 0) > run_once = 1; > else > diff --git a/gdb/remote.c b/gdb/remote.c > index 2f7954a..67319f6 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -1428,6 +1428,7 @@ enum { > PACKET_QPassSignals, > PACKET_QCatchSyscalls, > PACKET_QProgramSignals, > + PACKET_QStartupWithShell, > PACKET_qCRC, > PACKET_qSearch_memory, > PACKET_vAttach, > @@ -4079,6 +4080,20 @@ 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 startup-with-shell is on, we inform gdbserver to start the > + remote inferior using a shell. */ > + if (packet_support (PACKET_QStartupWithShell) != PACKET_DISABLE) > + { > + xsnprintf (rs->buf, get_remote_packet_size (), > + "QStartupWithShell:%d", startup_with_shell ? 1 : 0); > + putpkt (rs->buf); > + getpkt (&rs->buf, &rs->buf_size, 0); > + if (strcmp (rs->buf, "OK") != 0) > + error (_("\ > +Remote replied unexpectedly while setting startup-with-shell: %s"), > + rs->buf); > + } > + > /* 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 > @@ -4633,6 +4648,8 @@ static const struct protocol_feature remote_protocol_features[] = { > PACKET_QCatchSyscalls }, > { "QProgramSignals", PACKET_DISABLE, remote_supported_packet, > PACKET_QProgramSignals }, > + { "QStartupWithShell", PACKET_DISABLE, remote_supported_packet, > + PACKET_QStartupWithShell }, > { "QStartNoAckMode", PACKET_DISABLE, remote_supported_packet, > PACKET_QStartNoAckMode }, > { "multiprocess", PACKET_DISABLE, remote_supported_packet, > @@ -14110,6 +14127,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_QStartupWithShell], > + "QStartupWithShell", "startup-with-shell", 0); > + > add_packet_config_cmd (&remote_protocol_packets[PACKET_qSymbol], > "qSymbol", "symbol-lookup", 0); > > 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..6278447 > --- /dev/null > +++ b/gdb/testsuite/gdb.server/startup-with-shell.c > @@ -0,0 +1,29 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2017 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 . */ > + > +#include > + > +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..8992593 > --- /dev/null > +++ b/gdb/testsuite/gdb.server/startup-with-shell.exp > @@ -0,0 +1,83 @@ > +# This testcase is part of GDB, the GNU debugger. > + > +# Copyright 2017 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 . > + > +# 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] } { > + 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" > +} > + > +## 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) > +} > Otherwise i have no further comments.