Make the remote target always register a thread

[Pedro Alves <pedro@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 1699 bytes --]

Hi,

This patch makes the remote target always put a thread in the
thread list.  This is needed so we can get rid of context
switching on the core side.

This is also the fix I suggested for the crash reported in this thread:
"How to catch GDB crash"
http://sourceware.org/ml/gdb/2008-06/msg00242.html

(The crash will still be there for other targets that
don't always register the main thread)

The remote target is special in its handling of threads,
as depending on the optional thread related packets supported,
inferior_ptid can "upgrade" from magic_null_ptid to a
real ptid, hence care must be taken to not leave an old
thread in the thread list with magic_null_ptid ptid, if
threads are reported.

I've tested those special cases manually by hacking GDB,
so I could test combinations like,

target remote, no qC support, but qfThreadInfo supported,
target remote, no qC support, but T AA thread:TID is reported,

target extended-remote, attaching, with no threads support (no
qC, or qfThreadInfo or T AA thread:TID, or the old thread listing
protocol)

target extended-remote, running, again with no threads support.

All the above, but after connecting, enabling T AA thread:TID
All the above, but after connecting, enabling qC

GDB behaved correctly, in those cases, that is, the <main>
thread is upgraded to a real ptid correctly.

The start_remote change is needed, because we're
adding the thread to the list before calling it, so we can't
call init_thread_list there.  Instead the call is moved to the
start_remote called.  There's another caller in monitor.c,
so that also gets the obvious change.

Regtested with a native gdbserver on x86_64-unknown-linux-gnu.

OK?

-- 
Pedro Alves

[-- Attachment #2: 002-remote_always_a_thread.diff --]
[-- Type: text/x-diff, Size: 6028 bytes --]

2008-06-25  Pedro Alves  <pedro@codesourcery.com>

	* infrun.c (start_remote): Don't clear thread list here.
	* monitor.c (monitor_open): Clear thread list here.
	
	* remote.c (record_currthread): Upgrade the main thread and its
	entry in the thread list if this is the first time we hear about
	threads.
	(remote_thread_alive): Consider magic_null_ptid or a ptid without
	a tid member always alive.
	(remote_find_new_threads): Don't update the main thread here.
	(remote_start_remote): Clear thread list here.  Always add the
	main thread.
	(extended_remote_attach_1): Add the main thread here.
	(extended_remote_mourn_1): Re-add the main thread here.
	(extended_remote_create_inferior_1): Add a main thread.

---
 gdb/infrun.c  |    1 -
 gdb/monitor.c |    3 +++
 gdb/remote.c  |   54 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 52 insertions(+), 6 deletions(-)

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-06-25 20:09:25.000000000 +0100
+++ src/gdb/infrun.c	2008-06-25 21:13:35.000000000 +0100
@@ -1281,7 +1281,6 @@ proceed (CORE_ADDR addr, enum target_sig
 void
 start_remote (int from_tty)
 {
-  init_thread_list ();
   init_wait_for_inferior ();
   stop_soon = STOP_QUIETLY_REMOTE;
   stepping_over_breakpoint = 0;
Index: src/gdb/monitor.c
===================================================================
--- src.orig/gdb/monitor.c	2008-06-25 20:09:25.000000000 +0100
+++ src/gdb/monitor.c	2008-06-25 21:13:35.000000000 +0100
@@ -804,6 +804,9 @@ monitor_open (char *args, struct monitor
 
   push_target (targ_ops);
 
+  /* Start afresh.  */
+  init_thread_list ();
+
   inferior_ptid = pid_to_ptid (42000);	/* Make run command think we are busy... */
 
   /* Give monitor_wait something to read */
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2008-06-25 21:13:34.000000000 +0100
+++ src/gdb/remote.c	2008-06-25 22:34:48.000000000 +0100
@@ -1089,7 +1089,31 @@ record_currthread (ptid_t currthread)
   /* If this is a new thread, add it to GDB's thread list.
      If we leave it up to WFI to do this, bad things will happen.  */
   if (!in_thread_list (currthread))
-    add_thread (currthread);
+    {
+      if (ptid_equal (pid_to_ptid (ptid_get_pid (currthread)), inferior_ptid))
+	{
+	  /* inferior_ptid has no thread member yet.  This can happen
+	     with the vAttach -> remote_wait,"TAAthread:" path if the
+	     stub doesn't support qC.  This is the first stop reported
+	     after an attach, so this is the main thread.  Update the
+	     ptid in the thread list.  */
+	  struct thread_info *th = find_thread_pid (inferior_ptid);
+	  inferior_ptid = th->ptid = currthread;
+	}
+      else if (ptid_equal (magic_null_ptid, inferior_ptid))
+	{
+	  /* inferior_ptid is not set yet.  This can happen with the
+	     vRun -> remote_wait,"TAAthread:" path if the stub
+	     doesn't support qC.  This is the first stop reported
+	     after an attach, so this is the main thread.  Update the
+	     ptid in the thread list.  */
+	  struct thread_info *th = find_thread_pid (inferior_ptid);
+	  inferior_ptid = th->ptid = currthread;
+	}
+      else
+	/* This is really a new thread.  Add it.  */
+	add_thread (currthread);
+    }
 }
 
 static char *last_pass_packet;
@@ -1212,6 +1236,16 @@ remote_thread_alive (ptid_t ptid)
   struct remote_state *rs = get_remote_state ();
   int tid = ptid_get_tid (ptid);
 
+  if (ptid_equal (ptid, magic_null_ptid))
+    /* The main thread is always alive.  */
+    return 1;
+
+  if (ptid_get_pid (ptid) != 0 && ptid_get_tid (ptid) == 0)
+    /* The main thread is always alive.  This can happen after a
+       vAttach, if the remote side doesn't support
+       multi-threading.  */
+    return 1;
+
   if (tid < 0)
     xsnprintf (rs->buf, get_remote_packet_size (), "T-%08x", -tid);
   else
@@ -1925,9 +1959,6 @@ remote_find_new_threads (void)
 {
   remote_threadlist_iterator (remote_newthread_step, 0,
 			      CRAZY_MAX_THREADS);
-  if (ptid_equal (inferior_ptid, magic_null_ptid))
-    /* We don't know the current thread yet.  Query it.  */
-    inferior_ptid = remote_current_thread (inferior_ptid);
 }
 
 /*
@@ -2289,6 +2320,9 @@ remote_start_remote (struct ui_out *uiou
       strcpy (wait_status, rs->buf);
     }
 
+  /* Start afresh.  */
+  init_thread_list ();
+
   /* Let the stub know that we want it to return the thread.  */
   set_continue_thread (minus_one_ptid);
 
@@ -2304,6 +2338,9 @@ remote_start_remote (struct ui_out *uiou
   /* Now, if we have thread information, update inferior_ptid.  */
   inferior_ptid = remote_current_thread (inferior_ptid);
 
+  /* Always add the main thread.  */
+  add_thread_silent (inferior_ptid);
+
   get_offsets ();		/* Get text, data & bss offsets.  */
 
   /* Use the previously fetched status.  */
@@ -2934,6 +2971,9 @@ extended_remote_attach_1 (struct target_
   /* Now, if we have thread information, update inferior_ptid.  */
   inferior_ptid = remote_current_thread (inferior_ptid);
 
+  /* Now, add the main thread to the thread list.  */
+  add_thread_silent (inferior_ptid);
+
   attach_flag = 1;
 
   /* Next, if the target can specify a description, read it.  We do
@@ -5152,7 +5192,8 @@ extended_remote_mourn_1 (struct target_o
       /* Assume that the target has been restarted.  Set inferior_ptid
 	 so that bits of core GDB realizes there's something here, e.g.,
 	 so that the user can say "kill" again.  */
-      inferior_ptid = magic_null_ptid;
+      inferior_ptid = remote_current_thread (magic_null_ptid);
+      add_thread_silent (inferior_ptid);
     }
   else
     {
@@ -5267,6 +5308,9 @@ extended_remote_create_inferior_1 (char 
   /* Now mark the inferior as running before we do anything else.  */
   attach_flag = 0;
   inferior_ptid = magic_null_ptid;
+
+  add_thread_silent (inferior_ptid);
+
   target_mark_running (&extended_remote_ops);
 
   /* Get updated offsets, if the stub uses qOffsets.  */