Fix reconnecting to a gdbserver already debugging multiple processes, I

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:
2020-01-10  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:
2020-01-10  Pedro Alves  <palves@redhat.com>

	* gdb.server/extended-remote-restart.exp (test_reload): Explicitly
	disconnect before reconnecting.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 78922021681c4646766da4aaee7549def8fc2ab8..4163c869afed4c1f63b140240b8b21ebdb42ddac 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2020-01-10  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.
+
 2020-01-10  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
            Pedro Alves  <palves@redhat.com>
 

diff --git a/gdb/remote.c b/gdb/remote.c
index ffdeede7af5330777f60c3f5136863eb13be7f1f..751769ea7fc517013a3931df7ef0452a321f61d8 100644 (file)
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2367,6 +2367,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/ChangeLog b/gdb/testsuite/ChangeLog
index 52d52d15f6cd48b29be5b5c0ec3e9fe3bb314ffe..0ff75954b1a6fc5dd2275f2ad219abfdc4363e91 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-10  Pedro Alves  <palves@redhat.com>
+
+       * gdb.server/extended-remote-restart.exp (test_reload): Explicitly
+       disconnect before reconnecting.
+
 2020-01-10  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
            Pedro Alves  <palves@redhat.com>
 

diff --git a/gdb/testsuite/gdb.server/extended-remote-restart.exp b/gdb/testsuite/gdb.server/extended-remote-restart.exp
index b34027348efff07db6018ad1d52c936e9500fe07..3ce0f0b076ab8210580ce97d96d5b85a509f3b0f 100644 (file)
--- 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 {