[15/16] gdb: new maintenance command to help debug remote argument issues

Message ID 4d76e490c2285dd7858b853f03465a1d798d6240.1704809585.git.aburgess@redhat.com
State New
Headers
Series Inferior argument (inc for remote targets) changes |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed

Commit Message

Andrew Burgess Jan. 9, 2024, 2:26 p.m. UTC
  Add a new maintenance command 'maint test-remote-args', this command
takes an argument string and splits it using gdb::remote_args::split
and then joins the result using gdb::remote_args::join and prints all
of the results.  This is useful for diagnosing problems with remote
argument passing.

This new command is identical to what the remote argument self-tests
do, I found it easier to have a maintenance command available for
testing rather than having to add a new selftest and recompile GDB.
---
 gdb/NEWS                                      |  4 ++
 gdb/doc/gdb.texinfo                           | 21 +++++++
 gdb/remote.c                                  | 59 +++++++++++++++++++
 .../gdb.base/maint-test-remote-args.exp       | 40 +++++++++++++
 4 files changed, 124 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/maint-test-remote-args.exp
  

Comments

Eli Zaretskii Jan. 9, 2024, 4:32 p.m. UTC | #1
> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>, Michael Weghorn <m.weghorn@posteo.de>
> Date: Tue,  9 Jan 2024 14:26:38 +0000
> 
> Add a new maintenance command 'maint test-remote-args', this command
> takes an argument string and splits it using gdb::remote_args::split
> and then joins the result using gdb::remote_args::join and prints all
> of the results.  This is useful for diagnosing problems with remote
> argument passing.
> 
> This new command is identical to what the remote argument self-tests
> do, I found it easier to have a maintenance command available for
> testing rather than having to add a new selftest and recompile GDB.
> ---
>  gdb/NEWS                                      |  4 ++
>  gdb/doc/gdb.texinfo                           | 21 +++++++
>  gdb/remote.c                                  | 59 +++++++++++++++++++
>  .../gdb.base/maint-test-remote-args.exp       | 40 +++++++++++++
>  4 files changed, 124 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/maint-test-remote-args.exp

The documentation parts are okay, thanks.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  
Keith Seitz Jan. 21, 2024, 3:57 a.m. UTC | #2
On 1/9/24 06:26, Andrew Burgess wrote:

> diff --git a/gdb/testsuite/gdb.base/maint-test-remote-args.exp b/gdb/testsuite/gdb.base/maint-test-remote-args.exp
> new file mode 100644
> index 00000000000..3daf0725932
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/maint-test-remote-args.exp
> @@ -0,0 +1,40 @@
> +# Copyright 2024 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 the 'maint test-remote-args' command.
> +#
> +# We do minimal testing in here.  If you are thinking of adding a new
> +# test here then you are most likely adding the test in the wrong
> +# place.  Remote argument testing is checked in the following test
> +# scripts: gdb.base/args.exp, gdb.base/inferior-args.exp,
> +# gdb.base/startup-with-shell.exp, and gdb.python/py-inferior.exp.
> +# The test gdb.gdb/unittest.exp also runs 'maint selftest
> +# remote-args', which are the remote argument self tests.
> +#
> +# If you have a new test for an argument that was being passed
> +# incorrectly, then add the test to one of those scripts.
> +#
> +# This file is ONLY for validating that the 'maint test-remote-args'
> +# command itself is working.
> +
> +gdb_start
> +
> +gdb_test "maint test-remote-args a b c" \
> +    [multi_line \
> +	 "Input \\(a b c\\)" \
> +	 "  \\(a\\)" \
> +	 "  \\(b\\)" \
> +	 "  \\(c\\)" \
> +	 "Output \\(a b c\\)"]

Is it possible to add a test to exercise the failure mode, too?

Keith
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 2ba1899f78c..8f0bf34fccc 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -51,6 +51,10 @@  set remote thread-options-packet
 show remote thread-options-packet
   Set/show the use of the thread options packet.
 
+maintenance test-remote-args ARGS
+  Test splitting and joining of inferior arguments ARGS as they would
+  be split and joined when being passed to a remote target.
+
 * Changed commands
 
 set args
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2015293ee0d..6ff059d14ed 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -42204,6 +42204,26 @@ 
 @value{GDBN} supports.  They are used by the testsuite for exercising
 the settings infrastructure.
 
+@kindex maint test-remote-args
+@item maint test-remote-args @var{args}
+For targets that don't support passing inferior arguments as a single
+string (@pxref{single-inf-arg}), @value{GDBN} will attempt to split
+the inferior arguments before passing them to the remote target, and
+the remote target might choose to join the inferior arguments upon
+receipt.  Historically gdbserver did join inferior arguments, but now
+it will request inferior arguments be passed as a single string if
+@value{GDBN} supports this feature.
+
+This maintenance command splits @var{args} as @value{GDBN} would
+normally split such an argument string before passing the arguments to
+a remote target, the split arguments are then printed.
+
+The split arguments are then joined together as gdbserver would join
+them, and the result is printed.
+
+This command is intended to help diagnose issues passing inferior
+arguments to remote targets.
+
 @kindex maint set backtrace-on-fatal-signal
 @kindex maint show backtrace-on-fatal-signal
 @item maint set backtrace-on-fatal-signal [on|off]
@@ -44548,6 +44568,7 @@ 
 This feature indicates whether @value{GDBN} wants to know the
 supported actions in the reply to @samp{vCont?} packet.
 
+@anchor{single-inf-arg}
 @item single-inf-arg
 This feature indicates that @value{GDBN} would like to send the
 inferior arguments as a single string within the @samp{vRun} packet.
diff --git a/gdb/remote.c b/gdb/remote.c
index 75d275d38df..9b4d31dc633 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -12029,6 +12029,51 @@  cli_packet_command (const char *args, int from_tty)
   send_remote_packet (view, &cb);
 }
 
+/* Implement 'maint test-remote-args' command.
+
+   Treat ARGS as an argument string.  Split the remote arguments using
+   gdb::remote_args::split, and then join using gdb::remote_args::join.
+   The split and joined arguments are printed out.  Additionally, the
+   joined arguments are split and joined a second time, and compared to the
+   result of the first join, this provides some basic validation that GDB
+   sess the joined arguments as equivalent to the original argument
+   string.  */
+
+static void
+test_remote_args_command (const char *args, int from_tty)
+{
+  std::vector<std::string> split_args = gdb::remote_args::split (args);
+
+  gdb_printf ("Input (%s)\n", args);
+  for (const auto & a : split_args)
+    gdb_printf ("  (%s)\n", a.c_str ());
+
+  std::vector<gdb::unique_xmalloc_ptr<char>> tmp_split_args;
+  for (const auto & a : split_args)
+    tmp_split_args.emplace_back (xstrdup (a.c_str ()));
+
+  std::string joined_args = gdb::remote_args::join (tmp_split_args);
+
+  gdb_printf ("Output (%s)\n", joined_args.c_str ());
+
+  std::vector<std::string> resplit = gdb::remote_args::split (joined_args);
+
+  tmp_split_args.clear ();
+  for (const auto & a : resplit)
+    tmp_split_args.emplace_back (xstrdup (a.c_str ()));
+
+  std::string rejoined = gdb::remote_args::join (tmp_split_args);
+
+  if (joined_args != rejoined || split_args != resplit)
+    {
+      gdb_printf ("FAILURE ON REJOINING\n");
+      gdb_printf ("Resplit args:\n");
+      for (const auto & a : resplit)
+	gdb_printf ("  (%s)\n", a.c_str ());
+      gdb_printf ("Rejoined (%s)\n", rejoined.c_str ());
+    }
+}
+
 #if 0
 /* --------- UNIT_TEST for THREAD oriented PACKETS ------------------- */
 
@@ -16145,6 +16190,20 @@  from the target."),
   /* Eventually initialize fileio.  See fileio.c */
   initialize_remote_fileio (&remote_set_cmdlist, &remote_show_cmdlist);
 
+  add_cmd ("test-remote-args", class_maintenance,
+	   test_remote_args_command, _("\
+Test remote argument splitting and joining.\n	\
+  maintenance test-remote-args ARGS\n\
+For remote targets that don't support passing inferior arguments as a\n\
+single string, GDB needs to split the inferior arguments before passing\n\
+them, and gdbserver needs to join the arguments it receives.\n\
+This command splits ARGS just as GDB would before passing them to a\n\
+remote target, and prints the result.  This command then joins the\n\
+arguments just as gdbserver would, and prints the results.\n\
+This command is useful in diagnosing problems when passing arguments\n\
+between GDB and a remote target."),
+	   &maintenancelist);
+
 #if GDB_SELF_TEST
   selftests::register_test ("remote_memory_tagging",
 			    selftests::test_memory_tagging_functions);
diff --git a/gdb/testsuite/gdb.base/maint-test-remote-args.exp b/gdb/testsuite/gdb.base/maint-test-remote-args.exp
new file mode 100644
index 00000000000..3daf0725932
--- /dev/null
+++ b/gdb/testsuite/gdb.base/maint-test-remote-args.exp
@@ -0,0 +1,40 @@ 
+# Copyright 2024 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 the 'maint test-remote-args' command.
+#
+# We do minimal testing in here.  If you are thinking of adding a new
+# test here then you are most likely adding the test in the wrong
+# place.  Remote argument testing is checked in the following test
+# scripts: gdb.base/args.exp, gdb.base/inferior-args.exp,
+# gdb.base/startup-with-shell.exp, and gdb.python/py-inferior.exp.
+# The test gdb.gdb/unittest.exp also runs 'maint selftest
+# remote-args', which are the remote argument self tests.
+#
+# If you have a new test for an argument that was being passed
+# incorrectly, then add the test to one of those scripts.
+#
+# This file is ONLY for validating that the 'maint test-remote-args'
+# command itself is working.
+
+gdb_start
+
+gdb_test "maint test-remote-args a b c" \
+    [multi_line \
+	 "Input \\(a b c\\)" \
+	 "  \\(a\\)" \
+	 "  \\(b\\)" \
+	 "  \\(c\\)" \
+	 "Output \\(a b c\\)"]