public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 2/5] gdb: make set/show args work with $_gdb_setting_str
Date: Tue,  4 Apr 2023 13:45:28 +0100	[thread overview]
Message-ID: <14b084ae3950c36f8e631404975d3468813ef3fc.1680608960.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1680608960.git.aburgess@redhat.com>

I noticed that $_gdb_setting_str was not working with 'args', e.g.:

  $ gdb -q --args /tmp/hello.x arg1 arg2 arg3
  Reading symbols from /tmp/hello.x...
  (gdb) show args
  Argument list to give program being debugged when it is started is "arg1 arg2 arg3".
  (gdb) print $_gdb_setting_str("args")
  $1 = ""

This is because the 'args' setting is implemented using a scratch
variable ('inferior_args_scratch') which is updated when the user does
'set args ...'.  There is then a function 'set_args_command' which is
responsible for copying the scratch area into the current inferior.

However, when the user sets the arguments via the command line the
scratch variable is not updated, instead the arguments are pushed
straight into the current inferior.

There is a second problem, when the current inferior changes the
scratch area is not updated, which means that the value returned will
only ever reflect the last call to 'set args ...' regardless of which
inferior is currently selected.

Luckily, the fix is pretty easy, set/show variables have an
alternative API which requires we provide some getter and setter
functions.  With this done the scratch variable can be removed and the
value returned will now always reflect the current inferior.

While working on set/show args I also rewrote show_args_command to
remove the use of deprecated_show_value_hack.
---
 gdb/infcmd.c                             | 41 +++++-----
 gdb/testsuite/gdb.multi/gdb-settings.c   | 22 ++++++
 gdb/testsuite/gdb.multi/gdb-settings.exp | 98 ++++++++++++++++++++++++
 3 files changed, 140 insertions(+), 21 deletions(-)
 create mode 100644 gdb/testsuite/gdb.multi/gdb-settings.c
 create mode 100644 gdb/testsuite/gdb.multi/gdb-settings.exp

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index dd5808b17e7..2f0d4f38c85 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -66,14 +66,6 @@ static void step_1 (int, int, const char *);
 #define ERROR_NO_INFERIOR \
    if (!target_has_execution ()) error (_("The program is not being run."));
 
-/* Scratch area where string containing arguments to give to the
-   program will be stored by 'set args'.  As soon as anything is
-   stored, notice_args_set will move it into per-inferior storage.
-   Arguments are separated by spaces.  Empty string (pointer to '\0')
-   means no args.  */
-
-static std::string inferior_args_scratch;
-
 /* Scratch area where the new cwd will be stored by 'set cwd'.  */
 
 static std::string inferior_cwd_scratch;
@@ -136,26 +128,33 @@ set_inferior_args_vector (int argc, char **argv)
   current_inferior ()->set_args (std::move (n));
 }
 
-/* Notice when `set args' is run.  */
+/* Store the new value passed to 'set args'.  */
 
 static void
-set_args_command (const char *args, int from_tty, struct cmd_list_element *c)
+set_args_value (const std::string &args)
 {
-  /* CLI has assigned the user-provided value to inferior_args_scratch.
-     Now route it to current inferior.  */
-  current_inferior ()->set_args (inferior_args_scratch);
+  current_inferior ()->set_args (args);
 }
 
-/* Notice when `show args' is run.  */
+/* Return the value for 'show args' to display.  */
+
+static const std::string &
+get_args_value ()
+{
+  return current_inferior ()->args ();
+}
+
+/* Callback to implement 'show args' command.  */
 
 static void
 show_args_command (struct ui_file *file, int from_tty,
 		   struct cmd_list_element *c, const char *value)
 {
-  /* Note that we ignore the passed-in value in favor of computing it
-     directly.  */
-  deprecated_show_value_hack (file, from_tty, c,
-			      current_inferior ()->args ().c_str ());
+  /* Ignore the passed in value, pull the argument directly from the
+     inferior.  However, these should always be the same.  */
+  gdb_printf (_("\
+Argument list to give program being debugged when it is started is \"%s\".\n"),
+	      current_inferior ()->args ().c_str ());
 }
 
 /* See gdbsupport/common-inferior.h.  */
@@ -3154,12 +3153,12 @@ is restored."),
   add_alias_cmd ("tty", tty_set_show.set, class_run, 0, &cmdlist);
 
   auto args_set_show
-    = add_setshow_string_noescape_cmd ("args", class_run,
-				       &inferior_args_scratch, _("\
+    = add_setshow_string_noescape_cmd ("args", class_run, _("\
 Set argument list to give program being debugged when it is started."), _("\
 Show argument list to give program being debugged when it is started."), _("\
 Follow this command with any number of args, to be passed to the program."),
-				       set_args_command,
+				       set_args_value,
+				       get_args_value,
 				       show_args_command,
 				       &setlist, &showlist);
   set_cmd_completer (args_set_show.set, filename_completer);
diff --git a/gdb/testsuite/gdb.multi/gdb-settings.c b/gdb/testsuite/gdb.multi/gdb-settings.c
new file mode 100644
index 00000000000..3a264f239ed
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/gdb-settings.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 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/>.  */
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.multi/gdb-settings.exp b/gdb/testsuite/gdb.multi/gdb-settings.exp
new file mode 100644
index 00000000000..d2741d16157
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/gdb-settings.exp
@@ -0,0 +1,98 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2023 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 use of $_gdb_setting_str with multiple inferiors.  Mostly
+# this test is interested in checking that per-inferior settings work
+# correctly.
+
+standard_testfile
+
+require !use_gdb_stub
+
+if {[build_executable "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+# Return a string containing the args to pass to inferior NUM.
+
+proc inferior_args {num} {
+    return "a_${num}_1 a_${num}_2 a_${num}_3"
+}
+
+# Start inferior NUM.
+
+proc start_inferior {num} {
+    with_test_prefix "start_inferior $num" {
+	global srcfile binfile
+
+	set args [inferior_args ${num}]
+
+	if {$num != 1} {
+	    gdb_test "add-inferior" "Added inferior $num.*" \
+		"add empty inferior"
+	    gdb_test "inferior $num" "Switching to inferior $num.*" \
+		"switch to inferior"
+
+	    gdb_test_no_output "set args $args"
+	}
+
+	gdb_test {print $_gdb_setting_str("args")} \
+	    " = \"$args\"" \
+	    "check args before loading a file"
+
+	gdb_load $binfile
+
+	if {[gdb_start_cmd] < 0} {
+	    fail "could not start"
+	    return -1
+	}
+	gdb_test "" ".*reakpoint .*, main .*${srcfile}.*" "start"
+
+	gdb_test {print $_gdb_setting_str("args")} \
+	    " = \"$args\"" \
+	    "check after after starting"
+    }
+
+    return 0
+}
+
+# For the first inferior we set the arguments on the command line.
+save_vars { GDBFLAGS } {
+    set args [inferior_args 1]
+    append GDBFLAGS " --args ${binfile} $args"
+    clean_restart
+}
+
+# An arbitrary number of inferiors to start.
+set NUM_INFS 3
+
+# Start each inferior, setting (and checking) its args as we do.
+for {set i 1} {$i <= $NUM_INFS} {incr i} {
+    if {[start_inferior $i] < 0} {
+	return -1
+    }
+}
+
+# Switch back to each inferior and check the args are correct.
+for {set i 1} {$i <= $NUM_INFS} {incr i} {
+    gdb_test "inferior $i" "Switching to inferior $i .*"
+
+    set args [inferior_args $i]
+    gdb_test {print $_gdb_setting_str("args")} \
+	" = \"$args\"" \
+	"check args after after switching to inferior $i"
+}
-- 
2.25.4


  parent reply	other threads:[~2023-04-04 12:45 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-04 12:45 [PATCH 0/5] Fixes for per-inferior settings and $_gdb_setting_str() Andrew Burgess
2023-04-04 12:45 ` [PATCH 1/5] gdb: cleanup command creation in infcmd.c Andrew Burgess
2023-04-17 16:27   ` Tom Tromey
2023-04-04 12:45 ` Andrew Burgess [this message]
2023-04-17 16:37   ` [PATCH 2/5] gdb: make set/show args work with $_gdb_setting_str Tom Tromey
2023-04-17 18:04     ` Simon Marchi
2023-04-04 12:45 ` [PATCH 3/5] gdb: make set/show cwd " Andrew Burgess
2023-04-04 12:45 ` [PATCH 4/5] gdb: make set/show inferior-tty " Andrew Burgess
2023-04-04 12:45 ` [PATCH 5/5] gdb: make deprecated_show_value_hack static Andrew Burgess
2023-04-17 16:41   ` Tom Tromey
2023-04-28 14:57     ` Andrew Burgess
2023-07-10 17:25       ` Tom Tromey
2023-04-17 16:42 ` [PATCH 0/5] Fixes for per-inferior settings and $_gdb_setting_str() Tom Tromey
2023-04-17 18:09 ` Simon Marchi
2023-04-17 18:21   ` Simon Marchi
2023-04-28 21:53     ` Andrew Burgess
2023-04-28 16:43   ` Andrew Burgess

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=14b084ae3950c36f8e631404975d3468813ef3fc.1680608960.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --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).