[PATCH v2] gdb: Copy inferior properties in clone-inferior

[PATCH v2] gdb: Copy inferior properties in clone-inferior

[Lancelot SIX]

Noticable Changes from V1:
  - Fix and typos.
  - Fix unfortunate use of c_str's value after free (in inferior.c).
  - Remove unnecessary copies in for-loops by using const &.
  - Reduce use of auto.
  - Improve test naming (no capital, no "test" in name, better groups).
  - Patch 2 on the original series has been dropped since it has no
    immediate use..

Thanks Baris and Pedro for the reviews.

---

This commit ensures that the following settings are cloned from one
inferior to the new one when processing the clone-inferior command:
  - inferior-tty
  - environment variables
  - cwd
  - args

Some of those parameters can be passed as command line arguments to GDB
(-args and -tty), so one could expect the clone-inferior to respect
those flags.  The following debugging session illustrates that:

    gdb -nx -quiet -batch \
         -ex "show args" \
         -ex "show inferior-tty" \
         -ex "clone-inferior" \
         -ex "inferior 2" \
         -ex "show args" \
         -ex "show inferior-tty" \
         -tty=/some/tty \
         -args echo foo bar
    Argument list to give program being debugged when it is started is "foo bar".
    Terminal for future runs of program being debugged is "/some/tty".
    [New inferior 2]
    Added inferior 2.
    [Switching to inferior 2 [<null>] (/bin/echo)]
    Argument list to give program being debugged when it is started is "".
    Terminal for future runs of program being debugged is "".

The other properties this commit copies on clone (i.e. CWD and the
environment variables) are included since they are related (in the sense
that they influence the runtime behavior of the program) even if they
cannot be directly set using command line switches.

There is a chance that this patch changes existing user workflow.  I
think that this change is mostly harmless.  If users want to start a new
inferior based on an existing one, they probably already propagate those
settings to the new inferior in some way.

Tested on x86_64-linux.

Change-Id: I3b1f28b662f246228b37bb24c2ea1481567b363d
---
 gdb/NEWS                                  |  7 ++
 gdb/doc/gdb.texinfo                       |  8 +-
 gdb/inferior.c                            | 17 +++++
 gdb/testsuite/gdb.base/inferior-clone.exp | 93 +++++++++++++++++++++++
 4 files changed, 123 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/inferior-clone.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 3f78ace82bb..1e566d2a92e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -64,6 +64,13 @@ maint packet
   as escaped hex, e.g. \x?? where '??' is replaces with the value of
   the non-printable character.
 
+clone-inferior
+  The clone-inferior command now ensures that the TTY, CMD and ARGS
+  settings are copied from the original inferior to the new one.
+  All modifications to the environment variables done using the 'set
+  environment' or 'unset environment' commands are also copied to the new
+  inferior.
+
 * Python API
 
   ** New function gdb.add_history(), which takes a gdb.Value object
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d0c5bcf18e1..cb68d6193f2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3329,8 +3329,12 @@ use @code{run} to spawn a local program, etc.
 @item clone-inferior [ -copies @var{n} ] [ @var{infno} ]
 Adds @var{n} inferiors ready to execute the same program as inferior
 @var{infno}; @var{n} defaults to 1, and @var{infno} defaults to the
-number of the current inferior.  This is a convenient command when you
-want to run another instance of the inferior you are debugging.
+number of the current inferior.  This command copies the values of the
+@var{args}, @w{@var{inferior-tty}} and @var{cwd} properties from the
+current inferior to the new one.  It also propagates changes the user
+made to environment variables using the @w{@code{set environment}} and
+@w{@code{unset environment}} commands.  This is a convenient command
+when you want to run another instance of the inferior you are debugging.
 
 @smallexample
 (@value{GDBP}) info inferiors
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 40ab3e7e90b..16275bb94dc 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -932,6 +932,23 @@ clone_inferior_command (const char *args, int from_tty)
 	copy_inferior_target_desc_info (inf, orginf);
 
       clone_program_space (pspace, orginf->pspace);
+
+      /* Copy properties from the original inferior to the new one.  */
+      inf->set_args (orginf->args ());
+      inf->set_cwd (orginf->cwd ());
+      inf->set_tty (orginf->tty ());
+      for (const std::string &set_var : orginf->environment.user_set_env ())
+	{
+	  /* set_var has the form NAME=value.  Split on the first '='.  */
+	  const std::string::size_type pos = set_var.find ('=');
+	  gdb_assert (pos != std::string::npos);
+	  const std::string varname = set_var.substr (0, pos);
+	  inf->environment.set
+	    (varname.c_str (), orginf->environment.get (varname.c_str ()));
+	}
+      for (const std::string &unset_var
+	   : orginf->environment.user_unset_env ())
+	inf->environment.unset (unset_var.c_str ());
     }
 }
 
diff --git a/gdb/testsuite/gdb.base/inferior-clone.exp b/gdb/testsuite/gdb.base/inferior-clone.exp
new file mode 100644
index 00000000000..bb43be69672
--- /dev/null
+++ b/gdb/testsuite/gdb.base/inferior-clone.exp
@@ -0,0 +1,93 @@
+# Copyright (C) 2021 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/>.
+
+# This testcase checks that the clone-inferior command copies important
+# properties from the original to the new inferior such CWD, args and env.
+
+# Add an environment variable that can be modified later for each inferior
+# in GDB.
+set env(ENVVAR) var
+
+# This test does not need a binfile.
+clean_restart
+
+# Set custom properties for Inferior 1
+with_test_prefix "setup inferior 1" {
+    gdb_test_no_output "set args foo bar 'b a z'"
+    gdb_test_no_output "set cwd /any/where"
+    gdb_test_no_output "set environment FOO foo"
+    gdb_test_no_output "set environment BAR bar"
+    gdb_test_no_output "set inferior-tty some_tty"
+}
+
+# Check that properties of inferior 1 have been copied
+with_test_prefix "inferior 2" {
+    gdb_test "clone-inferior" "Added inferior 2"
+    gdb_test "inferior 2" "\[Switching to inferior 2 \[<null>\] (.*)\]" "change inferior"
+    gdb_test "show args" \
+	"Argument list to give program being debugged when it is started is \"foo bar 'b a z'\"\."
+    gdb_test "show cwd" \
+	"Current working directory that will be used when starting the inferior is \"/any/where\"\."
+    gdb_test "show environment FOO" "FOO = foo"
+    gdb_test "show environment BAR" "BAR = bar"
+    gdb_test "show environment ENVVAR" "ENVVAR = var"
+    gdb_test "show inferior-tty" \
+	"Terminal for future runs of program being debugged is \"some_tty\"\."
+}
+
+# Change this second inferior, to check that the next clone-inferior
+# clones from the active inferior, and changing the values for inferior 2
+# does not interfere with values on inferior 1.
+with_test_prefix "update inferior 2" {
+    gdb_test_no_output "set args foo"
+    gdb_test_no_output "set cwd /somewhere/else"
+    gdb_test_no_output "set environment FOO oof"
+}
+
+with_test_prefix "inferior 1" {
+    gdb_test "inferior 1" "\[Switching to inferior 1 \[<null>\] (.*)\]" "change inferior"
+    gdb_test "show args" \
+	"Argument list to give program being debugged when it is started is \"foo bar 'b a z'\"\."
+    gdb_test "show cwd" \
+	"Current working directory that will be used when starting the inferior is \"/any/where\"\."
+    gdb_test "show environment FOO" "FOO = foo"
+    gdb_test "show environment BAR" "BAR = bar"
+    gdb_test "show environment ENVVAR" "ENVVAR = var"
+    gdb_test "show inferior-tty" \
+	"Terminal for future runs of program being debugged is \"some_tty\"\."
+}
+
+# Tweak inferior 1 a bit more.
+with_test_prefix "update inferior 1" {
+    gdb_test_no_output "unset environment FOO"
+    gdb_test_no_output "unset environment ENVVAR"
+}
+
+
+# Check that the values observed on inferior 3 are those copied from
+# inferior 1.
+with_test_prefix "inferior 3" {
+    gdb_test "clone-inferior" "Added inferior 3"
+    gdb_test "inferior 3" "\[Switching to inferior 3 \[<null>\] (.*)\]" "change inferior"
+    gdb_test "show args" \
+	"Argument list to give program being debugged when it is started is \"foo bar 'b a z'\"\."
+    gdb_test "show cwd" \
+	"Current working directory that will be used when starting the inferior is \"/any/where\"\."
+    gdb_test "show environment FOO" "Environment variable \"FOO\" not defined\."
+    gdb_test "show environment BAR" "BAR = bar"
+    gdb_test "show environment ENVVAR" "Environment variable \"ENVVAR\" not defined\."
+    gdb_test "show inferior-tty" \
+	"Terminal for future runs of program being debugged is \"some_tty\"\."
+}

base-commit: 23ff54c27d535727c1c467abdd4bed8fbd46d4a6
-- 
2.25.1

Re: [PATCH v2] gdb: Copy inferior properties in clone-inferior

[Eli Zaretskii]

> Date: Thu, 16 Dec 2021 05:06:43 -0500
> From: Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org>
> Cc: lsix@lancelotsix.com, Lancelot SIX <lancelot.six@amd.com>
> 
> Noticable Changes from V1:
>   - Fix and typos.
>   - Fix unfortunate use of c_str's value after free (in inferior.c).
>   - Remove unnecessary copies in for-loops by using const &.
>   - Reduce use of auto.
>   - Improve test naming (no capital, no "test" in name, better groups).
>   - Patch 2 on the original series has been dropped since it has no
>     immediate use..
> 
> Thanks Baris and Pedro for the reviews.
> 
> ---
> 
> This commit ensures that the following settings are cloned from one
> inferior to the new one when processing the clone-inferior command:
>   - inferior-tty
>   - environment variables
>   - cwd
>   - args
> 
> Some of those parameters can be passed as command line arguments to GDB
> (-args and -tty), so one could expect the clone-inferior to respect
> those flags.  The following debugging session illustrates that:
> 
>     gdb -nx -quiet -batch \
>          -ex "show args" \
>          -ex "show inferior-tty" \
>          -ex "clone-inferior" \
>          -ex "inferior 2" \
>          -ex "show args" \
>          -ex "show inferior-tty" \
>          -tty=/some/tty \
>          -args echo foo bar
>     Argument list to give program being debugged when it is started is "foo bar".
>     Terminal for future runs of program being debugged is "/some/tty".
>     [New inferior 2]
>     Added inferior 2.
>     [Switching to inferior 2 [<null>] (/bin/echo)]
>     Argument list to give program being debugged when it is started is "".
>     Terminal for future runs of program being debugged is "".
> 
> The other properties this commit copies on clone (i.e. CWD and the
> environment variables) are included since they are related (in the sense
> that they influence the runtime behavior of the program) even if they
> cannot be directly set using command line switches.
> 
> There is a chance that this patch changes existing user workflow.  I
> think that this change is mostly harmless.  If users want to start a new
> inferior based on an existing one, they probably already propagate those
> settings to the new inferior in some way.
> 
> Tested on x86_64-linux.
> 
> Change-Id: I3b1f28b662f246228b37bb24c2ea1481567b363d
> ---
>  gdb/NEWS                                  |  7 ++
>  gdb/doc/gdb.texinfo                       |  8 +-
>  gdb/inferior.c                            | 17 +++++
>  gdb/testsuite/gdb.base/inferior-clone.exp | 93 +++++++++++++++++++++++
>  4 files changed, 123 insertions(+), 2 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/inferior-clone.exp

Thanks, the documentation changes are OK.

Re: [PATCH v2] gdb: Copy inferior properties in clone-inferior

[Pedro Alves]

On 2021-12-16 10:06, Lancelot SIX via Gdb-patches wrote:
> Noticable Changes from V1:
>   - Fix and typos.
>   - Fix unfortunate use of c_str's value after free (in inferior.c).
>   - Remove unnecessary copies in for-loops by using const &.
>   - Reduce use of auto.
>   - Improve test naming (no capital, no "test" in name, better groups).
>   - Patch 2 on the original series has been dropped since it has no
>     immediate use..
> 
> Thanks Baris and Pedro for the reviews.

Thanks.  This version is OK.

RE: [PATCH v2] gdb: Copy inferior properties in clone-inferior

[Six, Lancelot]

[AMD Official Use Only]

Thanks,

I have just pushed it.

Lancelot.

-----Original Message-----
From: Pedro Alves <pedro@palves.net> 
Sent: 16 December 2021 16:02
To: Six, Lancelot <Lancelot.Six@amd.com>; gdb-patches@sourceware.org
Cc: lsix@lancelotsix.com
Subject: Re: [PATCH v2] gdb: Copy inferior properties in clone-inferior

[CAUTION: External Email]

On 2021-12-16 10:06, Lancelot SIX via Gdb-patches wrote:
> Noticable Changes from V1:
>   - Fix and typos.
>   - Fix unfortunate use of c_str's value after free (in inferior.c).
>   - Remove unnecessary copies in for-loops by using const &.
>   - Reduce use of auto.
>   - Improve test naming (no capital, no "test" in name, better groups).
>   - Patch 2 on the original series has been dropped since it has no
>     immediate use..
>
> Thanks Baris and Pedro for the reviews.

Thanks.  This version is OK.

Re: [PATCH v2] gdb: Copy inferior properties in clone-inferior

[Simon Marchi]

On 2021-12-29 03:35, Six, Lancelot via Gdb-patches wrote:
> [AMD Official Use Only]
> 
> Thanks,
> 
> I have just pushed it.
> 
> Lancelot.

Hi Lancelot,

I now see this with --target_board=native-extended-gdbserver:

Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/inferior-clone.exp ...
FAIL: gdb.base/inferior-clone.exp: inferior 2: clone-inferior
FAIL: gdb.base/inferior-clone.exp: inferior 3: clone-inferior

Simon

RE: [PATCH v2] gdb: Copy inferior properties in clone-inferior

[Six, Lancelot]

[AMD Official Use Only]

Hi,

I see it. Sorry, I did not test it against this board.
I’ll propose a fix shortly.

Lancelot.

-----Original Message-----
From: Simon Marchi <simon.marchi@polymtl.ca> 
Sent: 29 December 2021 13:55
To: Six, Lancelot <Lancelot.Six@amd.com>; Pedro Alves <pedro@palves.net>; gdb-patches@sourceware.org
Cc: lsix@lancelotsix.com
Subject: Re: [PATCH v2] gdb: Copy inferior properties in clone-inferior

[CAUTION: External Email]

On 2021-12-29 03:35, Six, Lancelot via Gdb-patches wrote:
> [AMD Official Use Only]
>
> Thanks,
>
> I have just pushed it.
>
> Lancelot.

Hi Lancelot,

I now see this with --target_board=native-extended-gdbserver:

Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/inferior-clone.exp ...
FAIL: gdb.base/inferior-clone.exp: inferior 2: clone-inferior
FAIL: gdb.base/inferior-clone.exp: inferior 3: clone-inferior

Simon