public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fixing regression in mi -add-inferior command
@ 2022-02-24 17:25 Muhammad Umair Sair
  2022-03-02 11:57 ` [PATCHv2 0/2] " Andrew Burgess
  0 siblings, 1 reply; 16+ messages in thread
From: Muhammad Umair Sair @ 2022-02-24 17:25 UTC (permalink / raw)
  To: gdb-patches

 From 6b0edd1364f5ea63416838c3d08631adeed55474 Mon Sep 17 00:00:00 2001
From: Umair Sair <umair_sair@hotmail.com>
Date: Tue, 22 Feb 2022 01:40:06 +0500
Subject: [PATCH] Fixing regression in mi -add-inferior command

This commit fixes the regression (backward compatibility issue) in 
-add-inferior mi command after the addition of multi-target debugging 
support. Previously adding a new inferior via mi or cli uses same 
connection as other inferiors are using.
Now after the addition of multi-target support, add-inferior command 
without arguments in cli adds a new inferior and sets the connection of 
previously selected inferior as the connection of new inferior as well 
which keeps the backward compatibility intact. But the same command for 
machine interpreter is not setting the connection for new inferior and 
causing backward compatibility issue. This commit fixes the backward 
compatibility issue and makes the console and machine interpreter 
behavior consistent for add-inferior command with no arguments.
---
  gdb/inferior.c                           |  2 +-
  gdb/inferior.h                           |  3 ++
  gdb/mi/mi-main.c                         |  2 +
  gdb/testsuite/gdb.mi/mi-add-inferior.exp | 49 ++++++++++++++++++++++++
  4 files changed, 55 insertions(+), 1 deletion(-)
  create mode 100644 gdb/testsuite/gdb.mi/mi-add-inferior.exp

diff --git a/gdb/inferior.c b/gdb/inferior.c
index bebddb44173..82fd39c142d 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -761,7 +761,7 @@ add_inferior_with_spaces (void)
     NO_CONNECTION is true, push the process_stratum_target of ORG_INF
     to NEW_INF.  */

-static void
+void
  switch_to_inferior_and_push_target (inferior *new_inf,
  				    bool no_connection, inferior *org_inf)
  {
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 45de3c2d9c8..00978bec404 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -740,4 +740,7 @@ extern struct inferior *add_inferior_with_spaces (void);
  /* Print the current selected inferior.  */
  extern void print_selected_inferior (struct ui_out *uiout);

+extern void switch_to_inferior_and_push_target (inferior *new_inf,
+            bool no_connection, inferior *org_inf);
+
  #endif /* !defined (INFERIOR_H) */
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 4860da7536a..3a477bf3d05 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1710,6 +1710,8 @@ mi_cmd_add_inferior (const char *command, char 
**argv, int argc)

    inf = add_inferior_with_spaces ();

+  switch_to_inferior_and_push_target (inf, false, current_inferior ());
+
    current_uiout->field_fmt ("inferior", "i%d", inf->num);
  }

diff --git a/gdb/testsuite/gdb.mi/mi-add-inferior.exp 
b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
new file mode 100644
index 00000000000..6f10e9cc2d6
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
@@ -0,0 +1,49 @@
+# Copyright 2022 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 MI -add-inferior
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile basics.c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" 
executable {debug}] != "" } {
+     untested "failed to compile"
+     return -1
+}
+
+mi_gdb_load ${binfile}
+
+# Start execution to establish a connection
+mi_runto_main
+
+# Adding new inferior should use the connection of previous inferior
+# Match following
+#  -add-inferior
+#  =thread-group-added,id="i2"
+#  ~"[New inferior 2]\n"
+#  ~"Added inferior 2 on connection 1.*"
+#  ^done,inferior="i2"
+mi_gdb_test "-add-inferior" 
"=thread-group-added,id=\"i2\"\r\n~\"\\\[New inferior 
2\\\]\\\\n\"\r\n\~\"Added inferior 2 on connection 
.*\"\r\n\\^done,inferior=\"i2\"" \
+            "mi add inferior"
+
+mi_gdb_exit
+return 0
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCHv2 0/2] Fixing regression in mi -add-inferior command
  2022-02-24 17:25 [PATCH] Fixing regression in mi -add-inferior command Muhammad Umair Sair
@ 2022-03-02 11:57 ` Andrew Burgess
  2022-03-02 11:57   ` [PATCHv2 1/2] gdb/mi: fix " Andrew Burgess
                     ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Andrew Burgess @ 2022-03-02 11:57 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Hi Muhammad,

Thanks for working on this.  I've taken your patch and incorporated it
into the two patch series below, the first patch is still credited to
you, but has some changes:

 - During testing I noticed that with your patch, after -add-inferior,
   the new inferior has become the selected inferior.  I believe this
   was not the case before the original mutli-target patch, and isn't
   the case for the CLI command, so I fixed this, and extended the
   testing to cover this case.

 - I've moved the comment for switch_to_inferior_and_push_target into
   the .h file, and updated the .c file to match inline with the GDB
   style.

 - I tightened some of the pattern matching in the test case, for
   example, parsing the 'info inferiors' output to create an expected
   pattern for the connection name of the new inferior.

 - I tweaked the commit message to directly reference the commit that
   introduced the regression, I think this helps for anyone in the
   future trying to track down the history of a particular change.

Then I added a second commit that adds the --no-connection option for
the -add-inferior command.  I felt that it was important that these
two changes go in as a single series as, after the first commit, we
effectively loose the ability to create a new inferior with no
connection.

I'd welcome your feedback on both patches.  I think we just need to
wait for a doc review on patch #2, then we can probably get this
merged.

Thanks,
Andrew

---

Andrew Burgess (1):
  gdb/mi: add --no-connection to MI -add-inferior command

Muhammad Umair Sair via Gdb-patches (1):
  gdb/mi: fix regression in mi -add-inferior command

 gdb/doc/gdb.texinfo                      |  39 ++++++-
 gdb/inferior.c                           |   6 +-
 gdb/inferior.h                           |   7 ++
 gdb/mi/mi-main.c                         |  47 +++++++-
 gdb/testsuite/gdb.mi/mi-add-inferior.exp | 130 +++++++++++++++++++++++
 5 files changed, 217 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-add-inferior.exp

-- 
2.25.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCHv2 1/2] gdb/mi: fix regression in mi -add-inferior command
  2022-03-02 11:57 ` [PATCHv2 0/2] " Andrew Burgess
@ 2022-03-02 11:57   ` Andrew Burgess
  2022-03-02 11:57   ` [PATCHv2 2/2] gdb/mi: add --no-connection to MI " Andrew Burgess
  2022-03-03 14:59   ` [PATCHv2 0/2] Fixing regression in mi " Pedro Alves
  2 siblings, 0 replies; 16+ messages in thread
From: Andrew Burgess @ 2022-03-02 11:57 UTC (permalink / raw)
  To: gdb-patches

From: Muhammad Umair Sair via Gdb-patches <gdb-patches@sourceware.org>

Prior to the multi-target support commit:

  commit 5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2
  Date:   Fri Jan 10 20:06:08 2020 +0000

      Multi-target support

When a new inferior was added using the MI -add-inferior command, the
new inferior would be using the same target as all the other
inferiors.  This makes sense, GDB only supported a single target stack
at a time.

After the above commit, each inferior has its own target stack.

To maintain backward compatibility, for the CLI add-inferior command,
when a new inferior is added the above commit has the new inferior
inherit a copy of the target stack from the current inferior.

Unfortunately, this same backward compatibility is missing for the MI.

This commit fixes this oversight.

Now, when the -add-inferior MI command is used, the new inferior will
inherit a copy of the target stack from the current inferior.
---
 gdb/inferior.c                           |   6 +-
 gdb/inferior.h                           |   7 ++
 gdb/mi/mi-main.c                         |   4 +
 gdb/testsuite/gdb.mi/mi-add-inferior.exp | 122 +++++++++++++++++++++++
 4 files changed, 135 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-add-inferior.exp

diff --git a/gdb/inferior.c b/gdb/inferior.c
index bebddb44173..7fa69c3bf52 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -757,11 +757,9 @@ add_inferior_with_spaces (void)
   return inf;
 }
 
-/* Switch to inferior NEW_INF, a new inferior, and unless
-   NO_CONNECTION is true, push the process_stratum_target of ORG_INF
-   to NEW_INF.  */
+/* See inferior.h.  */
 
-static void
+void
 switch_to_inferior_and_push_target (inferior *new_inf,
 				    bool no_connection, inferior *org_inf)
 {
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 45de3c2d9c8..caabc2ee8d8 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -740,4 +740,11 @@ extern struct inferior *add_inferior_with_spaces (void);
 /* Print the current selected inferior.  */
 extern void print_selected_inferior (struct ui_out *uiout);
 
+/* Switch to inferior NEW_INF, a new inferior, and unless
+   NO_CONNECTION is true, push the process_stratum_target of ORG_INF
+   to NEW_INF.  */
+
+extern void switch_to_inferior_and_push_target
+  (inferior *new_inf, bool no_connection, inferior *org_inf);
+
 #endif /* !defined (INFERIOR_H) */
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 4860da7536a..2fab592d6fb 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1708,8 +1708,12 @@ mi_cmd_add_inferior (const char *command, char **argv, int argc)
   if (argc != 0)
     error (_("-add-inferior should be passed no arguments"));
 
+  scoped_restore_current_pspace_and_thread restore_pspace_thread;
+
   inf = add_inferior_with_spaces ();
 
+  switch_to_inferior_and_push_target (inf, false, current_inferior ());
+
   current_uiout->field_fmt ("inferior", "i%d", inf->num);
 }
 
diff --git a/gdb/testsuite/gdb.mi/mi-add-inferior.exp b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
new file mode 100644
index 00000000000..3f0cd7cc06c
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
@@ -0,0 +1,122 @@
+# Copyright 2022 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 MI '-add-inferior'.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile basics.c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	   executable {debug}] != "" } {
+    untested "failed to compile"
+    return -1
+}
+
+mi_clean_restart ${binfile}
+
+# Start execution to establish a connection.
+mi_runto_main
+
+# Use 'info inferiors' to find the details of the current connection.
+set header_line ""
+set inf_line ""
+gdb_test_multiple "info inferiors" "" {
+
+    -re "^info inferiors\r\n" {
+	exp_continue
+    }
+
+    -re "^&\[^\r\n\]+\r\n" {
+	exp_continue
+    }
+
+    -re "~\"(  Num\\s+Description\\s+Connection\[^\r\n\]+)\r\n" {
+	set header_line $expect_out(1,string)
+	exp_continue
+    }
+
+    -re "^~\"(\\*\\s+1\\s+\[^\r\n\]+)\r\n" {
+	set inf_line $expect_out(1,string)
+	exp_continue
+    }
+
+    -re "^\\^done\r\n" {
+	exp_continue
+    }
+
+    -re "^$mi_gdb_prompt$" {
+	gdb_assert { [string length "$header_line"] > 0 }
+	gdb_assert { [string length "$inf_line"] > 0 }
+	pass $gdb_test_name
+    }
+}
+
+# Now extract the string that represents the connection, and convert
+# it into a regexp.
+set idx [string first "Connection" "${header_line}"]
+gdb_assert { $idx > -1 }
+set inf_line [string range "${inf_line}" $idx end]
+regexp "^(${decimal} \\(\[^)\]+\\))" $inf_line conn_info
+set conn_pattern [string_to_regexp "${conn_info}"]
+
+# Now add a new inferior, this should use the connection of the
+# current inferior.
+mi_gdb_test "-add-inferior" \
+    [multi_line "=thread-group-added,id=\"\[^\"\]+\"" \
+	 "~\"\\\[New inferior 2\\\]\\\\n\"" \
+	 "\~\"Added inferior 2 on connection ${conn_pattern}\\\\n\"" \
+	 "\\^done,inferior=\"\[^\"\]+\"" ] \
+    "mi add inferior"
+
+# Now run 'info inferiors' again to check that the currently selected
+# inferior has not changed.
+set saw_current_inferior false
+set saw_new_inferior false
+gdb_test_multiple "info inferiors" \
+    "info inferiors, after new inferior was created" {
+
+	-re "^info inferiors\r\n" {
+	    exp_continue
+	}
+
+	-re "^&\[^\r\n\]+\r\n" {
+	    exp_continue
+	}
+
+	-re "~\"\\s+Num\\s+Description\\s+Connection\[^\r\n\]+\r\n" {
+	    exp_continue
+	}
+
+	-re "^~\"\\*\\s+1\\s+\[^\r\n\]+\\s+${conn_pattern}\\s+\[^\r\n\]+\r\n" {
+	    set saw_current_inferior true
+	    exp_continue
+	}
+
+	-re "^~\"\\s+2\\s+\[^\r\n\]+\\s+${conn_pattern}\\s+\[^\r\n\]+\r\n" {
+	    set saw_new_inferior true
+	    exp_continue
+	}
+
+	-re "^\\^done\r\n" {
+	    exp_continue
+	}
+
+	-re "^$mi_gdb_prompt$" {
+	    gdb_assert { $saw_current_inferior && $saw_new_inferior }
+	    pass $gdb_test_name
+	}
+    }
-- 
2.25.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCHv2 2/2] gdb/mi: add --no-connection to MI -add-inferior command
  2022-03-02 11:57 ` [PATCHv2 0/2] " Andrew Burgess
  2022-03-02 11:57   ` [PATCHv2 1/2] gdb/mi: fix " Andrew Burgess
@ 2022-03-02 11:57   ` Andrew Burgess
  2022-03-02 20:01     ` Eli Zaretskii
  2022-03-03 14:59   ` [PATCHv2 0/2] Fixing regression in mi " Pedro Alves
  2 siblings, 1 reply; 16+ messages in thread
From: Andrew Burgess @ 2022-03-02 11:57 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Following on from the previous commit, where the -add-inferior command
now uses the same connection as the current inferior, this commit adds
a --no-connection option to -add-inferior.

This new option matches the existing option of the same name for the
CLI version of add-inferior; the new inferior is created with no
connection.

I've added a new 'connection' field to the MI output of -add-inferior,
which includes the connection number and short name.  I haven't
included the longer description field, this is the MI after all.  My
expectation would be that if the frontend wanted to display all the
connection details then this would be looked up from 'info
connection' (or the MI equivalent if/when such a command is added).

The existing -add-inferior tests are updated, as are the docs.
---
 gdb/doc/gdb.texinfo                      | 39 +++++++++++++++++---
 gdb/mi/mi-main.c                         | 45 +++++++++++++++++++++---
 gdb/testsuite/gdb.mi/mi-add-inferior.exp | 10 +++++-
 3 files changed, 84 insertions(+), 10 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 504eb663c14..00824f840eb 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3310,6 +3310,7 @@
 @w{@code{remove-inferiors}} command.
 
 @table @code
+@anchor{add_inferior_cli}
 @kindex add-inferior
 @item add-inferior [ -copies @var{n} ] [ -exec @var{executable} ] [-no-connection ]
 Adds @var{n} inferiors to be run using @var{executable} as the
@@ -37101,15 +37102,45 @@
 @subheading Synopsis
 
 @smallexample
--add-inferior
+-add-inferior [ --no-connection ]
 @end smallexample
 
 Creates a new inferior (@pxref{Inferiors Connections and Programs}).  The created
 inferior is not associated with any executable.  Such association may
 be established with the @samp{-file-exec-and-symbols} command
-(@pxref{GDB/MI File Commands}).  The command response has a single
-field, @samp{inferior}, whose value is the identifier of the
-thread group corresponding to the new inferior.
+(@pxref{GDB/MI File Commands}).
+
+By default, the new inferior begins connected to the same target
+connection as the current inferior.  For example, if the current
+inferior was connected to @code{gdbserver} with @code{target remote},
+then the new inferior will be connected to the same @code{gdbserver}
+instance.  The @samp{--no-connection} option starts the new inferior
+with no connection yet.  You can then for example use the
+@code{-target-select remote} command to connect to some other
+@code{gdbserver} instance, use @code{-exec-run} to spawn a local
+program, etc.
+
+The command response always has a field, @samp{inferior}, whose value
+is the identifier of the thread group corresponding to the new
+inferior.
+
+An additional section field, @samp{connection}, is optional.  This
+field will only exist if the new inferior has a target connection.  If
+this field exists, then its value will be a tuple containing the
+following fields:
+
+@table @samp
+@item number
+The number of the connection used for the new inferior.
+
+@item name
+The name of the connection type used for the new inferior.
+@end table
+
+@subheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{add-inferior}
+(@pxref{add_inferior_cli,,@samp{add-inferior}}).
 
 @subheading Example
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 2fab592d6fb..b3592964e3d 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1703,18 +1703,53 @@ mi_cmd_list_target_features (const char *command, char **argv, int argc)
 void
 mi_cmd_add_inferior (const char *command, char **argv, int argc)
 {
-  struct inferior *inf;
+  bool no_connection = false;
 
-  if (argc != 0)
-    error (_("-add-inferior should be passed no arguments"));
+  /* Parse the command options.  */
+  enum opt
+    {
+      NO_CONNECTION_OPT,
+    };
+  static const struct mi_opt opts[] =
+    {
+	{"-no-connection", NO_CONNECTION_OPT, 0},
+	{NULL, 0, 0},
+    };
+
+  int oind = 0;
+  char *oarg;
+
+  while (1)
+    {
+      int opt = mi_getopt ("-add-inferior", argc, argv, opts, &oind, &oarg);
+
+      if (opt < 0)
+	break;
+      switch ((enum opt) opt)
+	{
+	case NO_CONNECTION_OPT:
+	  no_connection = true;
+	  break;
+	}
+    }
 
   scoped_restore_current_pspace_and_thread restore_pspace_thread;
 
-  inf = add_inferior_with_spaces ();
+  inferior *inf = add_inferior_with_spaces ();
 
-  switch_to_inferior_and_push_target (inf, false, current_inferior ());
+  switch_to_inferior_and_push_target (inf, no_connection,
+				      current_inferior ());
 
   current_uiout->field_fmt ("inferior", "i%d", inf->num);
+
+  process_stratum_target *proc_target = inf->process_target ();
+
+  if (proc_target != nullptr)
+    {
+      ui_out_emit_tuple tuple_emitter (current_uiout, "connection");
+      current_uiout->field_unsigned ("number", proc_target->connection_number);
+      current_uiout->field_string ("name", proc_target->shortname ());
+    }
 }
 
 void
diff --git a/gdb/testsuite/gdb.mi/mi-add-inferior.exp b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
index 3f0cd7cc06c..85cd6a5271d 100644
--- a/gdb/testsuite/gdb.mi/mi-add-inferior.exp
+++ b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
@@ -79,7 +79,7 @@ mi_gdb_test "-add-inferior" \
     [multi_line "=thread-group-added,id=\"\[^\"\]+\"" \
 	 "~\"\\\[New inferior 2\\\]\\\\n\"" \
 	 "\~\"Added inferior 2 on connection ${conn_pattern}\\\\n\"" \
-	 "\\^done,inferior=\"\[^\"\]+\"" ] \
+	 "\\^done,inferior=\"\[^\"\]+\",connection=\{number=\"$decimal\",name=\"\[^\"\]+\"\}" ] \
     "mi add inferior"
 
 # Now run 'info inferiors' again to check that the currently selected
@@ -120,3 +120,11 @@ gdb_test_multiple "info inferiors" \
 	    pass $gdb_test_name
 	}
     }
+
+# Add a third inferior, but this time, use --no-connection.
+mi_gdb_test "-add-inferior --no-connection" \
+    [multi_line "=thread-group-added,id=\"\[^\"\]+\"" \
+	 "~\"\\\[New inferior 3\\\]\\\\n\"" \
+	 "\~\"Added inferior 3\\\\n\"" \
+	 "\\^done,inferior=\"\[^\"\]+\"" ] \
+    "mi add inferior with no connection"
-- 
2.25.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCHv2 2/2] gdb/mi: add --no-connection to MI -add-inferior command
  2022-03-02 11:57   ` [PATCHv2 2/2] gdb/mi: add --no-connection to MI " Andrew Burgess
@ 2022-03-02 20:01     ` Eli Zaretskii
  2022-03-03 12:44       ` Andrew Burgess
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2022-03-02 20:01 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

> Date: Wed,  2 Mar 2022 11:57:36 +0000
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <aburgess@redhat.com>
> 
> +The command response always has a field, @samp{inferior}, whose value
> +is the identifier of the thread group corresponding to the new
> +inferior.
> +
> +An additional section field, @samp{connection}, is optional.  This
> +field will only exist if the new inferior has a target connection.  If
> +this field exists, then its value will be a tuple containing the
> +following fields:

The above two @samp should be @var, since they aren't literal symbols.

OK with that fixed, thanks.

Shouldn't this have a NEWS entry?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCHv2 2/2] gdb/mi: add --no-connection to MI -add-inferior command
  2022-03-02 20:01     ` Eli Zaretskii
@ 2022-03-03 12:44       ` Andrew Burgess
  2022-03-03 14:02         ` Eli Zaretskii
  2022-03-03 14:58         ` Pedro Alves
  0 siblings, 2 replies; 16+ messages in thread
From: Andrew Burgess @ 2022-03-03 12:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

* Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> [2022-03-02 22:01:19 +0200]:

> > Date: Wed,  2 Mar 2022 11:57:36 +0000
> > From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> > Cc: Andrew Burgess <aburgess@redhat.com>
> > 
> > +The command response always has a field, @samp{inferior}, whose value
> > +is the identifier of the thread group corresponding to the new
> > +inferior.
> > +
> > +An additional section field, @samp{connection}, is optional.  This
> > +field will only exist if the new inferior has a target connection.  If
> > +this field exists, then its value will be a tuple containing the
> > +following fields:
> 
> The above two @samp should be @var, since they aren't literal symbols.
> 
> OK with that fixed, thanks.
> 
> Shouldn't this have a NEWS entry?

Good point.  The patch below has the @samp -> @var change, plus a NEWS
entry.

Thanks,
Andrew

---

commit 3480e32d5fc5dd306447feb3d3dead70801f9a10
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Wed Mar 2 11:11:47 2022 +0000

    gdb/mi: add --no-connection to MI -add-inferior command
    
    Following on from the previous commit, where the -add-inferior command
    now uses the same connection as the current inferior, this commit adds
    a --no-connection option to -add-inferior.
    
    This new option matches the existing option of the same name for the
    CLI version of add-inferior; the new inferior is created with no
    connection.
    
    I've added a new 'connection' field to the MI output of -add-inferior,
    which includes the connection number and short name.  I haven't
    included the longer description field, this is the MI after all.  My
    expectation would be that if the frontend wanted to display all the
    connection details then this would be looked up from 'info
    connection' (or the MI equivalent if/when such a command is added).
    
    The existing -add-inferior tests are updated, as are the docs.

diff --git a/gdb/NEWS b/gdb/NEWS
index 41ea84e6063..1e53a55c259 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -136,6 +136,12 @@ info win
   This command now includes information about the width of the tui
   windows in its output.
 
+* MI changes
+
+ ** The '-add-inferior' command now accepts a '--no-connection'
+    option, which causes the new inferior to start without a
+    connection.
+
 * New targets
 
 GNU/Linux/LoongArch    loongarch*-*-linux*
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 504eb663c14..2222e91b573 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3310,6 +3310,7 @@
 @w{@code{remove-inferiors}} command.
 
 @table @code
+@anchor{add_inferior_cli}
 @kindex add-inferior
 @item add-inferior [ -copies @var{n} ] [ -exec @var{executable} ] [-no-connection ]
 Adds @var{n} inferiors to be run using @var{executable} as the
@@ -37101,15 +37102,45 @@
 @subheading Synopsis
 
 @smallexample
--add-inferior
+-add-inferior [ --no-connection ]
 @end smallexample
 
 Creates a new inferior (@pxref{Inferiors Connections and Programs}).  The created
 inferior is not associated with any executable.  Such association may
 be established with the @samp{-file-exec-and-symbols} command
-(@pxref{GDB/MI File Commands}).  The command response has a single
-field, @samp{inferior}, whose value is the identifier of the
-thread group corresponding to the new inferior.
+(@pxref{GDB/MI File Commands}).
+
+By default, the new inferior begins connected to the same target
+connection as the current inferior.  For example, if the current
+inferior was connected to @code{gdbserver} with @code{target remote},
+then the new inferior will be connected to the same @code{gdbserver}
+instance.  The @samp{--no-connection} option starts the new inferior
+with no connection yet.  You can then for example use the
+@code{-target-select remote} command to connect to some other
+@code{gdbserver} instance, use @code{-exec-run} to spawn a local
+program, etc.
+
+The command response always has a field, @var{inferior}, whose value
+is the identifier of the thread group corresponding to the new
+inferior.
+
+An additional section field, @var{connection}, is optional.  This
+field will only exist if the new inferior has a target connection.  If
+this field exists, then its value will be a tuple containing the
+following fields:
+
+@table @samp
+@item number
+The number of the connection used for the new inferior.
+
+@item name
+The name of the connection type used for the new inferior.
+@end table
+
+@subheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{add-inferior}
+(@pxref{add_inferior_cli,,@samp{add-inferior}}).
 
 @subheading Example
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 2fab592d6fb..b3592964e3d 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1703,18 +1703,53 @@ mi_cmd_list_target_features (const char *command, char **argv, int argc)
 void
 mi_cmd_add_inferior (const char *command, char **argv, int argc)
 {
-  struct inferior *inf;
+  bool no_connection = false;
 
-  if (argc != 0)
-    error (_("-add-inferior should be passed no arguments"));
+  /* Parse the command options.  */
+  enum opt
+    {
+      NO_CONNECTION_OPT,
+    };
+  static const struct mi_opt opts[] =
+    {
+	{"-no-connection", NO_CONNECTION_OPT, 0},
+	{NULL, 0, 0},
+    };
+
+  int oind = 0;
+  char *oarg;
+
+  while (1)
+    {
+      int opt = mi_getopt ("-add-inferior", argc, argv, opts, &oind, &oarg);
+
+      if (opt < 0)
+	break;
+      switch ((enum opt) opt)
+	{
+	case NO_CONNECTION_OPT:
+	  no_connection = true;
+	  break;
+	}
+    }
 
   scoped_restore_current_pspace_and_thread restore_pspace_thread;
 
-  inf = add_inferior_with_spaces ();
+  inferior *inf = add_inferior_with_spaces ();
 
-  switch_to_inferior_and_push_target (inf, false, current_inferior ());
+  switch_to_inferior_and_push_target (inf, no_connection,
+				      current_inferior ());
 
   current_uiout->field_fmt ("inferior", "i%d", inf->num);
+
+  process_stratum_target *proc_target = inf->process_target ();
+
+  if (proc_target != nullptr)
+    {
+      ui_out_emit_tuple tuple_emitter (current_uiout, "connection");
+      current_uiout->field_unsigned ("number", proc_target->connection_number);
+      current_uiout->field_string ("name", proc_target->shortname ());
+    }
 }
 
 void
diff --git a/gdb/testsuite/gdb.mi/mi-add-inferior.exp b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
index 3f0cd7cc06c..85cd6a5271d 100644
--- a/gdb/testsuite/gdb.mi/mi-add-inferior.exp
+++ b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
@@ -79,7 +79,7 @@ mi_gdb_test "-add-inferior" \
     [multi_line "=thread-group-added,id=\"\[^\"\]+\"" \
 	 "~\"\\\[New inferior 2\\\]\\\\n\"" \
 	 "\~\"Added inferior 2 on connection ${conn_pattern}\\\\n\"" \
-	 "\\^done,inferior=\"\[^\"\]+\"" ] \
+	 "\\^done,inferior=\"\[^\"\]+\",connection=\{number=\"$decimal\",name=\"\[^\"\]+\"\}" ] \
     "mi add inferior"
 
 # Now run 'info inferiors' again to check that the currently selected
@@ -120,3 +120,11 @@ gdb_test_multiple "info inferiors" \
 	    pass $gdb_test_name
 	}
     }
+
+# Add a third inferior, but this time, use --no-connection.
+mi_gdb_test "-add-inferior --no-connection" \
+    [multi_line "=thread-group-added,id=\"\[^\"\]+\"" \
+	 "~\"\\\[New inferior 3\\\]\\\\n\"" \
+	 "\~\"Added inferior 3\\\\n\"" \
+	 "\\^done,inferior=\"\[^\"\]+\"" ] \
+    "mi add inferior with no connection"


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCHv2 2/2] gdb/mi: add --no-connection to MI -add-inferior command
  2022-03-03 12:44       ` Andrew Burgess
@ 2022-03-03 14:02         ` Eli Zaretskii
  2022-03-03 14:58         ` Pedro Alves
  1 sibling, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2022-03-03 14:02 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

> Date: Thu, 3 Mar 2022 12:44:39 +0000
> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> > > Date: Wed,  2 Mar 2022 11:57:36 +0000
> > > From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> > > Cc: Andrew Burgess <aburgess@redhat.com>
> > > 
> > > +The command response always has a field, @samp{inferior}, whose value
> > > +is the identifier of the thread group corresponding to the new
> > > +inferior.
> > > +
> > > +An additional section field, @samp{connection}, is optional.  This
> > > +field will only exist if the new inferior has a target connection.  If
> > > +this field exists, then its value will be a tuple containing the
> > > +following fields:
> > 
> > The above two @samp should be @var, since they aren't literal symbols.
> > 
> > OK with that fixed, thanks.
> > 
> > Shouldn't this have a NEWS entry?
> 
> Good point.  The patch below has the @samp -> @var change, plus a NEWS
> entry.

LGTM, thanks.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCHv2 2/2] gdb/mi: add --no-connection to MI -add-inferior command
  2022-03-03 12:44       ` Andrew Burgess
  2022-03-03 14:02         ` Eli Zaretskii
@ 2022-03-03 14:58         ` Pedro Alves
  2022-03-07 19:40           ` Andrew Burgess
  1 sibling, 1 reply; 16+ messages in thread
From: Pedro Alves @ 2022-03-03 14:58 UTC (permalink / raw)
  To: Andrew Burgess, Eli Zaretskii; +Cc: gdb-patches

On 2022-03-03 12:44, Andrew Burgess via Gdb-patches wrote:
> +* MI changes
> +
> + ** The '-add-inferior' command now accepts a '--no-connection'
> +    option, which causes the new inferior to start without a
> +    connection.

I think this should also say that -add-inferior with no option now inherits
the connection of the current inferior, restoring the behavior of GDB xxx.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCHv2 0/2] Fixing regression in mi -add-inferior command
  2022-03-02 11:57 ` [PATCHv2 0/2] " Andrew Burgess
  2022-03-02 11:57   ` [PATCHv2 1/2] gdb/mi: fix " Andrew Burgess
  2022-03-02 11:57   ` [PATCHv2 2/2] gdb/mi: add --no-connection to MI " Andrew Burgess
@ 2022-03-03 14:59   ` Pedro Alves
  2 siblings, 0 replies; 16+ messages in thread
From: Pedro Alves @ 2022-03-03 14:59 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 2022-03-02 11:57, Andrew Burgess via Gdb-patches wrote:
> Hi Muhammad,
> 
> Thanks for working on this.  I've taken your patch and incorporated it
> into the two patch series below, the first patch is still credited to
> you, but has some changes:
> 
>  - During testing I noticed that with your patch, after -add-inferior,
>    the new inferior has become the selected inferior.  I believe this
>    was not the case before the original mutli-target patch, and isn't
>    the case for the CLI command, so I fixed this, and extended the
>    testing to cover this case.
> 
>  - I've moved the comment for switch_to_inferior_and_push_target into
>    the .h file, and updated the .c file to match inline with the GDB
>    style.
> 
>  - I tightened some of the pattern matching in the test case, for
>    example, parsing the 'info inferiors' output to create an expected
>    pattern for the connection name of the new inferior.
> 
>  - I tweaked the commit message to directly reference the commit that
>    introduced the regression, I think this helps for anyone in the
>    future trying to track down the history of a particular change.
> 
> Then I added a second commit that adds the --no-connection option for
> the -add-inferior command.  I felt that it was important that these
> two changes go in as a single series as, after the first commit, we
> effectively loose the ability to create a new inferior with no
> connection.
> 
> I'd welcome your feedback on both patches.  I think we just need to
> wait for a doc review on patch #2, then we can probably get this
> merged.
> 

Other than a small remark on the NEWS entry, this LGTM.  Thanks for addressing
this.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCHv2 2/2] gdb/mi: add --no-connection to MI -add-inferior command
  2022-03-03 14:58         ` Pedro Alves
@ 2022-03-07 19:40           ` Andrew Burgess
  2022-03-07 20:37             ` Pedro Alves
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Burgess @ 2022-03-07 19:40 UTC (permalink / raw)
  To: Pedro Alves, Eli Zaretskii; +Cc: gdb-patches

Pedro Alves <pedro@palves.net> writes:

> On 2022-03-03 12:44, Andrew Burgess via Gdb-patches wrote:
>> +* MI changes
>> +
>> + ** The '-add-inferior' command now accepts a '--no-connection'
>> +    option, which causes the new inferior to start without a
>> +    connection.
>
> I think this should also say that -add-inferior with no option now inherits
> the connection of the current inferior, restoring the behavior of GDB xxx.

I added this note, and pushed these two patches.

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCHv2 2/2] gdb/mi: add --no-connection to MI -add-inferior command
  2022-03-07 19:40           ` Andrew Burgess
@ 2022-03-07 20:37             ` Pedro Alves
  2022-03-08 12:55               ` Andrew Burgess
  2022-03-08 12:58               ` [PUSHED 0/2] Fixing regression in mi " Andrew Burgess
  0 siblings, 2 replies; 16+ messages in thread
From: Pedro Alves @ 2022-03-07 20:37 UTC (permalink / raw)
  To: Andrew Burgess, Eli Zaretskii; +Cc: gdb-patches

On 2022-03-07 19:40, Andrew Burgess wrote:
> Pedro Alves <pedro@palves.net> writes:
> 
>> On 2022-03-03 12:44, Andrew Burgess via Gdb-patches wrote:
>>> +* MI changes
>>> +
>>> + ** The '-add-inferior' command now accepts a '--no-connection'
>>> +    option, which causes the new inferior to start without a
>>> +    connection.
>>
>> I think this should also say that -add-inferior with no option now inherits
>> the connection of the current inferior, restoring the behavior of GDB xxx.
> 
> I added this note, and pushed these two patches.
> 

Hmm, I don't see any change in the pushed version.  Forgot to amend the
patch, perhaps?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCHv2 2/2] gdb/mi: add --no-connection to MI -add-inferior command
  2022-03-07 20:37             ` Pedro Alves
@ 2022-03-08 12:55               ` Andrew Burgess
  2022-03-08 20:41                 ` Pedro Alves
  2022-03-08 12:58               ` [PUSHED 0/2] Fixing regression in mi " Andrew Burgess
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Burgess @ 2022-03-08 12:55 UTC (permalink / raw)
  To: Pedro Alves, Eli Zaretskii; +Cc: gdb-patches

Pedro Alves <pedro@palves.net> writes:

> On 2022-03-07 19:40, Andrew Burgess wrote:
>> Pedro Alves <pedro@palves.net> writes:
>> 
>>> On 2022-03-03 12:44, Andrew Burgess via Gdb-patches wrote:
>>>> +* MI changes
>>>> +
>>>> + ** The '-add-inferior' command now accepts a '--no-connection'
>>>> +    option, which causes the new inferior to start without a
>>>> +    connection.
>>>
>>> I think this should also say that -add-inferior with no option now inherits
>>> the connection of the current inferior, restoring the behavior of GDB xxx.
>> 
>> I added this note, and pushed these two patches.
>> 
>
> Hmm, I don't see any change in the pushed version.  Forgot to amend the
> patch, perhaps?

Here's the hunk from the patch I pushed, this is currently in master:

diff --git a/gdb/NEWS b/gdb/NEWS
index ee9eaad63a0..4e12ebfb0bd 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -152,6 +152,12 @@ info win
   of the floating-point type, and the "f" is the marker for floating
   point.
 
+* MI changes
+
+ ** The '-add-inferior' with no option flags now inherits the
+    connection of the current inferior, this restores the behaviour of
+    GDB as it was prior to GDB 10.
+
 * New targets
 
 GNU/Linux/LoongArch    loongarch*-*-linux*

I'll follow up with the complete patches.

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PUSHED 0/2] Fixing regression in mi -add-inferior command
  2022-03-07 20:37             ` Pedro Alves
  2022-03-08 12:55               ` Andrew Burgess
@ 2022-03-08 12:58               ` Andrew Burgess
  2022-03-08 12:58                 ` [PUSHED 1/2] gdb/mi: fix " Andrew Burgess
  2022-03-08 12:58                 ` [PUSHED 2/2] gdb/mi: add --no-connection to MI " Andrew Burgess
  1 sibling, 2 replies; 16+ messages in thread
From: Andrew Burgess @ 2022-03-08 12:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Here's the final version of the patches I pushed.  The only difference
from v2 is an extra entry for gdb/NEWS in patch #1.

Thanks,
Andrew

---

Andrew Burgess (1):
  gdb/mi: add --no-connection to MI -add-inferior command

Umair Sair (1):
  gdb/mi: fix regression in mi -add-inferior command

 gdb/NEWS                                      |  10 ++
 gdb/doc/gdb.texinfo                           |  39 +++++-
 gdb/inferior.c                                |   6 +-
 gdb/inferior.h                                |   7 +
 gdb/mi/mi-main.c                              |  47 ++++++-
 .../gdb.mi/interrupt-thread-group.exp         |  16 +--
 gdb/testsuite/gdb.mi/mi-add-inferior.exp      | 130 ++++++++++++++++++
 7 files changed, 230 insertions(+), 25 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-add-inferior.exp

-- 
2.25.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PUSHED 1/2] gdb/mi: fix regression in mi -add-inferior command
  2022-03-08 12:58               ` [PUSHED 0/2] Fixing regression in mi " Andrew Burgess
@ 2022-03-08 12:58                 ` Andrew Burgess
  2022-03-08 12:58                 ` [PUSHED 2/2] gdb/mi: add --no-connection to MI " Andrew Burgess
  1 sibling, 0 replies; 16+ messages in thread
From: Andrew Burgess @ 2022-03-08 12:58 UTC (permalink / raw)
  To: gdb-patches

From: Umair Sair <umair_sair@hotmail.com>

Prior to the multi-target support commit:

  commit 5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2
  Date:   Fri Jan 10 20:06:08 2020 +0000

      Multi-target support

When a new inferior was added using the MI -add-inferior command, the
new inferior would be using the same target as all the other
inferiors.  This makes sense, GDB only supported a single target stack
at a time.

After the above commit, each inferior has its own target stack.

To maintain backward compatibility, for the CLI add-inferior command,
when a new inferior is added the above commit has the new inferior
inherit a copy of the target stack from the current inferior.

Unfortunately, this same backward compatibility is missing for the MI.

This commit fixes this oversight.

Now, when the -add-inferior MI command is used, the new inferior will
inherit a copy of the target stack from the current inferior.
---
 gdb/NEWS                                 |   6 ++
 gdb/inferior.c                           |   6 +-
 gdb/inferior.h                           |   7 ++
 gdb/mi/mi-main.c                         |   4 +
 gdb/testsuite/gdb.mi/mi-add-inferior.exp | 122 +++++++++++++++++++++++
 5 files changed, 141 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-add-inferior.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index ee9eaad63a0..4e12ebfb0bd 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -152,6 +152,12 @@ info win
   of the floating-point type, and the "f" is the marker for floating
   point.
 
+* MI changes
+
+ ** The '-add-inferior' with no option flags now inherits the
+    connection of the current inferior, this restores the behaviour of
+    GDB as it was prior to GDB 10.
+
 * New targets
 
 GNU/Linux/LoongArch    loongarch*-*-linux*
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 965ae65287e..48d5c8bfd15 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -759,11 +759,9 @@ add_inferior_with_spaces (void)
   return inf;
 }
 
-/* Switch to inferior NEW_INF, a new inferior, and unless
-   NO_CONNECTION is true, push the process_stratum_target of ORG_INF
-   to NEW_INF.  */
+/* See inferior.h.  */
 
-static void
+void
 switch_to_inferior_and_push_target (inferior *new_inf,
 				    bool no_connection, inferior *org_inf)
 {
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 45de3c2d9c8..caabc2ee8d8 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -740,4 +740,11 @@ extern struct inferior *add_inferior_with_spaces (void);
 /* Print the current selected inferior.  */
 extern void print_selected_inferior (struct ui_out *uiout);
 
+/* Switch to inferior NEW_INF, a new inferior, and unless
+   NO_CONNECTION is true, push the process_stratum_target of ORG_INF
+   to NEW_INF.  */
+
+extern void switch_to_inferior_and_push_target
+  (inferior *new_inf, bool no_connection, inferior *org_inf);
+
 #endif /* !defined (INFERIOR_H) */
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 4860da7536a..2fab592d6fb 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1708,8 +1708,12 @@ mi_cmd_add_inferior (const char *command, char **argv, int argc)
   if (argc != 0)
     error (_("-add-inferior should be passed no arguments"));
 
+  scoped_restore_current_pspace_and_thread restore_pspace_thread;
+
   inf = add_inferior_with_spaces ();
 
+  switch_to_inferior_and_push_target (inf, false, current_inferior ());
+
   current_uiout->field_fmt ("inferior", "i%d", inf->num);
 }
 
diff --git a/gdb/testsuite/gdb.mi/mi-add-inferior.exp b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
new file mode 100644
index 00000000000..3f0cd7cc06c
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
@@ -0,0 +1,122 @@
+# Copyright 2022 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 MI '-add-inferior'.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile basics.c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	   executable {debug}] != "" } {
+    untested "failed to compile"
+    return -1
+}
+
+mi_clean_restart ${binfile}
+
+# Start execution to establish a connection.
+mi_runto_main
+
+# Use 'info inferiors' to find the details of the current connection.
+set header_line ""
+set inf_line ""
+gdb_test_multiple "info inferiors" "" {
+
+    -re "^info inferiors\r\n" {
+	exp_continue
+    }
+
+    -re "^&\[^\r\n\]+\r\n" {
+	exp_continue
+    }
+
+    -re "~\"(  Num\\s+Description\\s+Connection\[^\r\n\]+)\r\n" {
+	set header_line $expect_out(1,string)
+	exp_continue
+    }
+
+    -re "^~\"(\\*\\s+1\\s+\[^\r\n\]+)\r\n" {
+	set inf_line $expect_out(1,string)
+	exp_continue
+    }
+
+    -re "^\\^done\r\n" {
+	exp_continue
+    }
+
+    -re "^$mi_gdb_prompt$" {
+	gdb_assert { [string length "$header_line"] > 0 }
+	gdb_assert { [string length "$inf_line"] > 0 }
+	pass $gdb_test_name
+    }
+}
+
+# Now extract the string that represents the connection, and convert
+# it into a regexp.
+set idx [string first "Connection" "${header_line}"]
+gdb_assert { $idx > -1 }
+set inf_line [string range "${inf_line}" $idx end]
+regexp "^(${decimal} \\(\[^)\]+\\))" $inf_line conn_info
+set conn_pattern [string_to_regexp "${conn_info}"]
+
+# Now add a new inferior, this should use the connection of the
+# current inferior.
+mi_gdb_test "-add-inferior" \
+    [multi_line "=thread-group-added,id=\"\[^\"\]+\"" \
+	 "~\"\\\[New inferior 2\\\]\\\\n\"" \
+	 "\~\"Added inferior 2 on connection ${conn_pattern}\\\\n\"" \
+	 "\\^done,inferior=\"\[^\"\]+\"" ] \
+    "mi add inferior"
+
+# Now run 'info inferiors' again to check that the currently selected
+# inferior has not changed.
+set saw_current_inferior false
+set saw_new_inferior false
+gdb_test_multiple "info inferiors" \
+    "info inferiors, after new inferior was created" {
+
+	-re "^info inferiors\r\n" {
+	    exp_continue
+	}
+
+	-re "^&\[^\r\n\]+\r\n" {
+	    exp_continue
+	}
+
+	-re "~\"\\s+Num\\s+Description\\s+Connection\[^\r\n\]+\r\n" {
+	    exp_continue
+	}
+
+	-re "^~\"\\*\\s+1\\s+\[^\r\n\]+\\s+${conn_pattern}\\s+\[^\r\n\]+\r\n" {
+	    set saw_current_inferior true
+	    exp_continue
+	}
+
+	-re "^~\"\\s+2\\s+\[^\r\n\]+\\s+${conn_pattern}\\s+\[^\r\n\]+\r\n" {
+	    set saw_new_inferior true
+	    exp_continue
+	}
+
+	-re "^\\^done\r\n" {
+	    exp_continue
+	}
+
+	-re "^$mi_gdb_prompt$" {
+	    gdb_assert { $saw_current_inferior && $saw_new_inferior }
+	    pass $gdb_test_name
+	}
+    }
-- 
2.25.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PUSHED 2/2] gdb/mi: add --no-connection to MI -add-inferior command
  2022-03-08 12:58               ` [PUSHED 0/2] Fixing regression in mi " Andrew Burgess
  2022-03-08 12:58                 ` [PUSHED 1/2] gdb/mi: fix " Andrew Burgess
@ 2022-03-08 12:58                 ` Andrew Burgess
  1 sibling, 0 replies; 16+ messages in thread
From: Andrew Burgess @ 2022-03-08 12:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Following on from the previous commit, where the -add-inferior command
now uses the same connection as the current inferior, this commit adds
a --no-connection option to -add-inferior.

This new option matches the existing option of the same name for the
CLI version of add-inferior; the new inferior is created with no
connection.

I've added a new 'connection' field to the MI output of -add-inferior,
which includes the connection number and short name.  I haven't
included the longer description field, this is the MI after all.  My
expectation would be that if the frontend wanted to display all the
connection details then this would be looked up from 'info
connection' (or the MI equivalent if/when such a command is added).

The existing -add-inferior tests are updated, as are the docs.
---
 gdb/NEWS                                      |  4 ++
 gdb/doc/gdb.texinfo                           | 39 ++++++++++++++--
 gdb/mi/mi-main.c                              | 45 ++++++++++++++++---
 .../gdb.mi/interrupt-thread-group.exp         | 16 ++-----
 gdb/testsuite/gdb.mi/mi-add-inferior.exp      | 10 ++++-
 5 files changed, 91 insertions(+), 23 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 4e12ebfb0bd..e0c55e8274f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -158,6 +158,10 @@ info win
     connection of the current inferior, this restores the behaviour of
     GDB as it was prior to GDB 10.
 
+ ** The '-add-inferior' command now accepts a '--no-connection'
+    option, which causes the new inferior to start without a
+    connection.
+
 * New targets
 
 GNU/Linux/LoongArch    loongarch*-*-linux*
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 063e3a1cc9b..132b94c6487 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3310,6 +3310,7 @@
 @w{@code{remove-inferiors}} command.
 
 @table @code
+@anchor{add_inferior_cli}
 @kindex add-inferior
 @item add-inferior [ -copies @var{n} ] [ -exec @var{executable} ] [-no-connection ]
 Adds @var{n} inferiors to be run using @var{executable} as the
@@ -37141,15 +37142,45 @@
 @subheading Synopsis
 
 @smallexample
--add-inferior
+-add-inferior [ --no-connection ]
 @end smallexample
 
 Creates a new inferior (@pxref{Inferiors Connections and Programs}).  The created
 inferior is not associated with any executable.  Such association may
 be established with the @samp{-file-exec-and-symbols} command
-(@pxref{GDB/MI File Commands}).  The command response has a single
-field, @samp{inferior}, whose value is the identifier of the
-thread group corresponding to the new inferior.
+(@pxref{GDB/MI File Commands}).
+
+By default, the new inferior begins connected to the same target
+connection as the current inferior.  For example, if the current
+inferior was connected to @code{gdbserver} with @code{target remote},
+then the new inferior will be connected to the same @code{gdbserver}
+instance.  The @samp{--no-connection} option starts the new inferior
+with no connection yet.  You can then for example use the
+@code{-target-select remote} command to connect to some other
+@code{gdbserver} instance, use @code{-exec-run} to spawn a local
+program, etc.
+
+The command response always has a field, @var{inferior}, whose value
+is the identifier of the thread group corresponding to the new
+inferior.
+
+An additional section field, @var{connection}, is optional.  This
+field will only exist if the new inferior has a target connection.  If
+this field exists, then its value will be a tuple containing the
+following fields:
+
+@table @samp
+@item number
+The number of the connection used for the new inferior.
+
+@item name
+The name of the connection type used for the new inferior.
+@end table
+
+@subheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{add-inferior}
+(@pxref{add_inferior_cli,,@samp{add-inferior}}).
 
 @subheading Example
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 2fab592d6fb..b3592964e3d 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1703,18 +1703,53 @@ mi_cmd_list_target_features (const char *command, char **argv, int argc)
 void
 mi_cmd_add_inferior (const char *command, char **argv, int argc)
 {
-  struct inferior *inf;
+  bool no_connection = false;
 
-  if (argc != 0)
-    error (_("-add-inferior should be passed no arguments"));
+  /* Parse the command options.  */
+  enum opt
+    {
+      NO_CONNECTION_OPT,
+    };
+  static const struct mi_opt opts[] =
+    {
+	{"-no-connection", NO_CONNECTION_OPT, 0},
+	{NULL, 0, 0},
+    };
+
+  int oind = 0;
+  char *oarg;
+
+  while (1)
+    {
+      int opt = mi_getopt ("-add-inferior", argc, argv, opts, &oind, &oarg);
+
+      if (opt < 0)
+	break;
+      switch ((enum opt) opt)
+	{
+	case NO_CONNECTION_OPT:
+	  no_connection = true;
+	  break;
+	}
+    }
 
   scoped_restore_current_pspace_and_thread restore_pspace_thread;
 
-  inf = add_inferior_with_spaces ();
+  inferior *inf = add_inferior_with_spaces ();
 
-  switch_to_inferior_and_push_target (inf, false, current_inferior ());
+  switch_to_inferior_and_push_target (inf, no_connection,
+				      current_inferior ());
 
   current_uiout->field_fmt ("inferior", "i%d", inf->num);
+
+  process_stratum_target *proc_target = inf->process_target ();
+
+  if (proc_target != nullptr)
+    {
+      ui_out_emit_tuple tuple_emitter (current_uiout, "connection");
+      current_uiout->field_unsigned ("number", proc_target->connection_number);
+      current_uiout->field_string ("name", proc_target->shortname ());
+    }
 }
 
 void
diff --git a/gdb/testsuite/gdb.mi/interrupt-thread-group.exp b/gdb/testsuite/gdb.mi/interrupt-thread-group.exp
index 8661d573cfe..19ccbe85e04 100644
--- a/gdb/testsuite/gdb.mi/interrupt-thread-group.exp
+++ b/gdb/testsuite/gdb.mi/interrupt-thread-group.exp
@@ -57,19 +57,9 @@ mi_send_resuming_command "exec-continue --thread-group i1" \
 set use_second_inferior [expr {![use_gdb_stub]}]
 
 if { $use_second_inferior } {
-    # The inferior created by the -add-inferior MI command does not inherit the
-    # target connection of the first inferior.  If debugging through an
-    # extended-remote connection, that means we can't run that second inferior
-    # on the remote connection.  Use the add-inferior CLI command as a stop-gap.
-    if { [mi_is_target_remote] } {
-	mi_gdb_test "add-inferior" \
-	    "\\^done" \
-	    "add inferior 2"
-    } else {
-	mi_gdb_test "-add-inferior" \
-	    "\\^done,inferior=\"i2\"" \
-	    "add inferior 2"
-    }
+    mi_gdb_test "-add-inferior" \
+	"\\^done,inferior=\"i2\",connection=\\{\[^\}\]+\\}" \
+	"add inferior 2"
     mi_gdb_test "-file-exec-and-symbols --thread-group i2 $::binfile" \
 	"\\^done" \
 	"set executable of inferior 2"
diff --git a/gdb/testsuite/gdb.mi/mi-add-inferior.exp b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
index 3f0cd7cc06c..85cd6a5271d 100644
--- a/gdb/testsuite/gdb.mi/mi-add-inferior.exp
+++ b/gdb/testsuite/gdb.mi/mi-add-inferior.exp
@@ -79,7 +79,7 @@ mi_gdb_test "-add-inferior" \
     [multi_line "=thread-group-added,id=\"\[^\"\]+\"" \
 	 "~\"\\\[New inferior 2\\\]\\\\n\"" \
 	 "\~\"Added inferior 2 on connection ${conn_pattern}\\\\n\"" \
-	 "\\^done,inferior=\"\[^\"\]+\"" ] \
+	 "\\^done,inferior=\"\[^\"\]+\",connection=\{number=\"$decimal\",name=\"\[^\"\]+\"\}" ] \
     "mi add inferior"
 
 # Now run 'info inferiors' again to check that the currently selected
@@ -120,3 +120,11 @@ gdb_test_multiple "info inferiors" \
 	    pass $gdb_test_name
 	}
     }
+
+# Add a third inferior, but this time, use --no-connection.
+mi_gdb_test "-add-inferior --no-connection" \
+    [multi_line "=thread-group-added,id=\"\[^\"\]+\"" \
+	 "~\"\\\[New inferior 3\\\]\\\\n\"" \
+	 "\~\"Added inferior 3\\\\n\"" \
+	 "\\^done,inferior=\"\[^\"\]+\"" ] \
+    "mi add inferior with no connection"
-- 
2.25.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCHv2 2/2] gdb/mi: add --no-connection to MI -add-inferior command
  2022-03-08 12:55               ` Andrew Burgess
@ 2022-03-08 20:41                 ` Pedro Alves
  0 siblings, 0 replies; 16+ messages in thread
From: Pedro Alves @ 2022-03-08 20:41 UTC (permalink / raw)
  To: Andrew Burgess, Eli Zaretskii; +Cc: gdb-patches

On 2022-03-08 12:55, Andrew Burgess wrote:
> Pedro Alves <pedro@palves.net> writes:
> 
>> On 2022-03-07 19:40, Andrew Burgess wrote:
>>> Pedro Alves <pedro@palves.net> writes:
>>>
>>>> On 2022-03-03 12:44, Andrew Burgess via Gdb-patches wrote:
>>>>> +* MI changes
>>>>> +
>>>>> + ** The '-add-inferior' command now accepts a '--no-connection'
>>>>> +    option, which causes the new inferior to start without a
>>>>> +    connection.
>>>>
>>>> I think this should also say that -add-inferior with no option now inherits
>>>> the connection of the current inferior, restoring the behavior of GDB xxx.
>>>
>>> I added this note, and pushed these two patches.
>>>
>>
>> Hmm, I don't see any change in the pushed version.  Forgot to amend the
>> patch, perhaps?
> 
> Here's the hunk from the patch I pushed, this is currently in master:

My bad, I didn't realize there were two NEWS entries, and only looked at the
one in the last patch, which stayed the same.  Sorry about that.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-03-08 20:42 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-24 17:25 [PATCH] Fixing regression in mi -add-inferior command Muhammad Umair Sair
2022-03-02 11:57 ` [PATCHv2 0/2] " Andrew Burgess
2022-03-02 11:57   ` [PATCHv2 1/2] gdb/mi: fix " Andrew Burgess
2022-03-02 11:57   ` [PATCHv2 2/2] gdb/mi: add --no-connection to MI " Andrew Burgess
2022-03-02 20:01     ` Eli Zaretskii
2022-03-03 12:44       ` Andrew Burgess
2022-03-03 14:02         ` Eli Zaretskii
2022-03-03 14:58         ` Pedro Alves
2022-03-07 19:40           ` Andrew Burgess
2022-03-07 20:37             ` Pedro Alves
2022-03-08 12:55               ` Andrew Burgess
2022-03-08 20:41                 ` Pedro Alves
2022-03-08 12:58               ` [PUSHED 0/2] Fixing regression in mi " Andrew Burgess
2022-03-08 12:58                 ` [PUSHED 1/2] gdb/mi: fix " Andrew Burgess
2022-03-08 12:58                 ` [PUSHED 2/2] gdb/mi: add --no-connection to MI " Andrew Burgess
2022-03-03 14:59   ` [PATCHv2 0/2] Fixing regression in mi " Pedro Alves

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).