[binutils-gdb] gdb: make set/show args work with $_gdb_setting_str

[binutils-gdb] gdb: make set/show args work with $_gdb_setting_str

[Andrew Burgess]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=cc09d372f664410aea226bfaa246aeb74fee8126

commit cc09d372f664410aea226bfaa246aeb74fee8126
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Apr 4 09:56:00 2023 +0100

    gdb: make set/show args work with $_gdb_setting_str
    
    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.
    
    Reviewed-By: Tom Tromey <tom@tromey.com>

Diff:
---
 gdb/infcmd.c                             |  41 +++++++------
 gdb/testsuite/gdb.multi/gdb-settings.c   |  22 +++++++
 gdb/testsuite/gdb.multi/gdb-settings.exp | 100 +++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 21 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 86555dfc74e..310ad625676 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..407bc55b1f2
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/gdb-settings.exp
@@ -0,0 +1,100 @@
+# 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 per-inferior settings in a multi-inferior debug session.  Check
+# that the settings really are per-inferior.
+
+load_lib gdb-python.exp
+load_lib gdb-guile.exp
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+# Cache whether we can run Python and/or Guile tests.
+set run_python_tests [allow_python_tests]
+set run_guile_tests [allow_guile_tests]
+
+# The $_gdb_setting/$_gdb_setting_str tests require running inferiors,
+# because they allocate memory in the inferiors for the produced
+# values.  Since we need two inferiors for this test, we can't run
+# them with stub boards (e.g. gdbserver with non-extended remote
+# protocol), since they can only run one inferior at a time.  We can
+# still run the other tests with multiple inferiors, they just won't
+# be running inferiors.
+set run [expr {![use_gdb_stub]}]
+
+# List of inferior numbers to run tests for.
+set inferiors {1 2}
+
+# Start all the inferiors.
+clean_restart $binfile
+foreach_with_prefix inf $inferiors {
+    if { $inf > 1 } {
+	gdb_test "add-inferior -exec $binfile" "Added inferior 2.*" \
+	    "add second inferior"
+    }
+
+    if { $run } {
+	if { ![runto_main] } {
+	    return -1
+	}
+    }
+}
+
+# Setup some guile helpers -- if we plan to run the guile tests.
+if { $run_guile_tests } {
+    gdb_install_guile_utils
+    gdb_install_guile_module
+}
+
+# Update the settings for each inferior.
+foreach_with_prefix inf $inferiors {
+    gdb_test "inferior ${inf}" "Switching to inferior ${inf}.*" \
+	"switch to inferior ${inf} before set"
+    gdb_test_no_output "set args inf${inf}-args"
+}
+
+# Check settings are still correct for each inferior.
+foreach_with_prefix inf $inferiors {
+    gdb_test "inferior ${inf}" "Switching to inferior ${inf}.*" \
+	"switch back to inferior ${inf}"
+
+    # Check that using 'with' doesn't corrupt the setting value.
+    gdb_test "with args tmp-value -- print 1" " = 1"
+    gdb_test "show args" "inf${inf}-args.*"
+
+    # If the inferiors are running check $_gdb_setting_str and
+    # $_gdb_setting return the correct values.
+    if { $run } {
+	gdb_test {print $_gdb_setting_str("args")} "\"inf${inf}-args\""
+	gdb_test {print $_gdb_setting("args")} "\"inf${inf}-args\""
+    }
+
+    # Check the settings can be read from Python.
+    if { $run_python_tests } {
+	gdb_test "python print(gdb.parameter('args'))" "inf${inf}-args"
+    }
+
+    # Check the settings can be read from Guile.
+    if { $run_guile_tests } {
+	gdb_test "guile (print (parameter-value \"args\"))" \
+	    "inf${inf}-args"
+    }
+}