[PATCH 15/23] Fix reconnecting to a gdbserver already debugging multiple processes, I

[Pedro Alves <palves@redhat.com>]

The multi-target patch will change the remote target's behavior when:

- the current inferior is connected to an extended-remote target.
- the current inferior is attached to any process.
- some other inferior than than the current one is live.

In current master, we get:

 (gdb) tar extended-remote :9999
 A program is being debugged already.  Kill it? (y or n)

While after multi-target, since each inferior may have its own target
connection, we'll get:

 (gdb) tar extended-remote :9999
 Already connected to a remote target.  Disconnect? (y or n)

That change made gdb.server/extended-remote-restart.exp expose a gdb
bug, because it made "target remote", via gdb_reconnect, just
disconnect from the previous connection, while in current master that
command would kill the inferior before disconnecting.  In turn, that
would make a multi-target gdb find processes already running under
control of gdbserver as soon as it reconnects, while in current master
there is never any process around when gdb reconnects, since they'd
all been killed prior to disconnection.

The bug this exposed is that remote_target::remote_add_inferior was
always reusing current_inferior() for the new process, even if the
current inferior was already bound to a process.  In the testcase's
case, when we reconnect, the remote is debugging two processes.  So
we'd bind the first remote process to the empty current inferior the
first time, and then bind the second remote process to the same
inferior again, essencially losing track of the first process.  That
resulted in failed assertions when we look up the inferior for the
first process by PID.  The fix is to still prefer binding to the
current inferior (so that plain "target remote" keeps doing what you'd
expect), but not reuse the current inferior if it is already bound to
a process.

This patch tweaks the test to explicitly disconnect before
reconnecting, to avoid GDB killing processes, thus making current GDB
behave the same as it will behave when the multi-target work lands.
That change alone without the GDB fix exposes the bug like so:

 (gdb) PASS: gdb.server/extended-remote-restart.exp: kill: 0, follow-child 0: disconnect
 target extended-remote localhost:2350
 Remote debugging using localhost:2350
 src/gdb/thread.c:93: internal-error: thread_info* inferior_thread(): Assertion `tp' failed.
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 Quit this debugging session? (y or n)

The original bug that the testcase was written for was related to
killing, (git 9d4a934ce604 ("gdb: Fix assert for extended-remote
target (PR gdb/18050)")), but since the testcase tries reconnecting
with both explicitly killing and not explicitly killing, I think we're
covering the original bug with this testcase change.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* remote.c (remote_target::remote_add_inferior): Don't bind a
	process to the current inferior if the current inferior is already
	bound to a process.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.server/extended-remote-restart.exp (test_reload): Explicitly
	disconnect before reconnecting.
---
 gdb/remote.c                                         | 20 ++++++++++++++++++++
 gdb/testsuite/gdb.server/extended-remote-restart.exp |  4 +++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index eacaf11976..16ef9dbdfd 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2371,6 +2371,26 @@ remote_target::remote_add_inferior (bool fake_pid_p, int pid, int attached,
 	 between program/address spaces.  We simply bind the inferior
 	 to the program space's address space.  */
       inf = current_inferior ();
+
+      /* However, if the current inferior is already bound to a
+	 process, find some other empty inferior.  */
+      if (inf->pid != 0)
+	{
+	  inf = nullptr;
+	  for (inferior *it : all_inferiors ())
+	    if (it->pid == 0)
+	      {
+		inf = it;
+		break;
+	      }
+	}
+      if (inf == nullptr)
+	{
+	  /* Since all inferiors were already bound to a process, add
+	     a new inferior.  */
+	  inf = add_inferior_with_spaces ();
+	}
+      switch_to_inferior_no_thread (inf);
       inferior_appeared (inf, pid);
     }
 
diff --git a/gdb/testsuite/gdb.server/extended-remote-restart.exp b/gdb/testsuite/gdb.server/extended-remote-restart.exp
index 1fa46f2073..c78342c010 100644
--- a/gdb/testsuite/gdb.server/extended-remote-restart.exp
+++ b/gdb/testsuite/gdb.server/extended-remote-restart.exp
@@ -113,7 +113,9 @@ proc test_reload { do_kill_p follow_child_p } {
 	    "Check inferior was killed"
     }
 
-    # Reconnect to the target.
+    # Disconnect, and reconnect to the target.
+    gdb_test "disconnect" ".*"
+
     if { [gdb_reconnect] == 0 } {
 	pass "reconnect after fork"
     } else {
-- 
2.14.5