[PATCHv2] gdb/remote: Restore support for 'S' stop reply packet

[Andrew Burgess]

Pedro,

Thanks for your feedback.  I agree with all your points, this version
of the patch:

  1. Doesn't query the target for the current thread, but just uses
  the first non-exited thread.

  2. Gives a warning if the target has more than one non-exited
  thread.

  3. Commit message is revised to reflect new implementation.

How's this?

Thanks,
Andrew

---

With this commit:

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

      Multi-target support

There was a regression in GDB's support for older aspects of the
remote protocol.  Specifically, when a target sends the 'S' stop reply
packet, which doesn't include a thread-id, then GDB has to figure out
which thread actually stopped.

Before the above commit GDB figured this out by using inferior_ptid in
process_stop_reply, which contained the ptid of the current
process/thread.  With the above commit the inferior_ptid now has the
value null_ptid inside process_stop_reply, this can be seen in
do_target_wait, where we call switch_to_inferior_no_thread before
calling do_target_wait_1.

The solution I propose in this commit is to use the first non exited
thread of the inferior.  Additionally, GDB will give a warning if a
multi-threaded target stops without passing a thread-id.

There's no test included with this commit, if anyone has any ideas for
how we could test this aspect of the remote protocol (short of writing
a new gdbserver) then I'd love to know.

It is possible to trigger this bug by attaching GDB to a running GDB,
place a breakpoint on remote_parse_stop_reply, and manually change the
contents of buf - when we get a 'T' based stop packet, replace it with
an 'S' based packet, like this:

  (gdb) call memset (buf, "S05\0", 4)

After this the GDB that is performing the remote debugging will crash
with this error:

  inferior.c:279: internal-error: inferior* find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0' failed.

gdb/ChangeLog:

	* remote.c (remote_target::process_stop_reply): Use the first
	non-exited thread if the target didn't pass a thread-id.
---
 gdb/ChangeLog |  5 +++++
 gdb/remote.c  | 18 +++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 4a70ab3fb0d..4ac359ad5d9 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7668,10 +7668,22 @@ remote_target::process_stop_reply (struct stop_reply *stop_reply,
   *status = stop_reply->ws;
   ptid = stop_reply->ptid;
 
-  /* If no thread/process was reported by the stub, assume the current
-     inferior.  */
+  /* If no thread/process was reported by the stub then use the first
+     non-exited thread in the current inferior.  */
   if (ptid == null_ptid)
-    ptid = inferior_ptid;
+    {
+      for (thread_info *thr : all_non_exited_threads (this))
+	{
+	  if (ptid != null_ptid)
+	    {
+	      warning (_("multi-threaded target stopped without sending "
+			 "a thread-id, using first non-exited thread"));
+	      break;
+	    }
+	  ptid = thr->ptid;
+	}
+      gdb_assert (ptid != null_ptid);
+    }
 
   if (status->kind != TARGET_WAITKIND_EXITED
       && status->kind != TARGET_WAITKIND_SIGNALLED
-- 
2.14.5