Re: Make the remote target always register a thread

[Pedro Alves <pedro@codesourcery.com>]

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

A Thursday 26 June 2008 03:48:14, Daniel Jacobowitz wrote:
> On Thu, Jun 26, 2008 at 12:08:08AM +0100, Pedro Alves wrote:
> > (The crash will still be there for other targets that
> > don't always register the main thread)
>
> Be nice to get rid of the crash, one way or another.

It will take a while to make all targets consistent,
so although I'd prefer to fix them all, practically speaking, the
assertion should be removed for now, and perhaps leave a comment
there so when we do have all targets consistent, the assert will be
catching a real bug.

The fix seems to me to be a one or two liner,
but, as I've said at gdb@, I don't know if we should output
thread-id=0 or not output thread-id at all, or maybe even
output thread-id=all, as if there are no threads in the list,
this is the special case of there being only one "task" really,
but the target is not registering it.

(OTOH, having an assert at least makes it easy to see
which target needs to be changed.)

> OK.

Thanks, I've checked it in, with an obvious change as attached.  I needed
to include gdbthread.h in monitor.c.

-- 
Pedro Alves

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

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

	* infrun.c (start_remote): Don't clear thread list here.
	* monitor.c (monitor_open): Include "gdbthread.h".  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.

	* Makefile.in (monitor.o): Depend on $(gdbthread_h).

---
 gdb/Makefile.in |    2 +-
 gdb/infrun.c    |    1 -
 gdb/monitor.c   |    4 ++++
 gdb/remote.c    |   54 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 54 insertions(+), 7 deletions(-)

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-06-27 12:30:13.000000000 +0100
+++ src/gdb/infrun.c	2008-06-27 12:33:21.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-27 12:30:13.000000000 +0100
+++ src/gdb/monitor.c	2008-06-27 12:37:31.000000000 +0100
@@ -54,6 +54,7 @@
 #include "gdb_regex.h"
 #include "srec.h"
 #include "regcache.h"
+#include "gdbthread.h"
 
 static char *dev_name;
 static struct target_ops *targ_ops;
@@ -804,6 +805,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-27 12:33:04.000000000 +0100
+++ src/gdb/remote.c	2008-06-27 12:33:21.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.  */
Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2008-06-27 12:37:12.000000000 +0100
+++ src/gdb/Makefile.in	2008-06-27 12:38:20.000000000 +0100
@@ -2540,7 +2540,7 @@ mn10300-tdep.o: mn10300-tdep.c $(defs_h)
 	$(symtab_h) $(dwarf2_frame_h) $(osabi_h) $(target_h) $(mn10300_tdep_h)
 monitor.o: monitor.c $(defs_h) $(gdbcore_h) $(target_h) $(exceptions_h) \
 	$(gdb_string_h) $(command_h) $(serial_h) $(monitor_h) $(gdbcmd_h) \
-	$(inferior_h) $(gdb_regex_h) $(srec_h) $(regcache_h)
+	$(inferior_h) $(gdb_regex_h) $(srec_h) $(regcache_h) $(gdbthread_h)
 mt-tdep.o: mt-tdep.c $(defs_h) $(frame_h) $(frame_unwind_h) $(frame_base_h) \
 	$(symtab_h) $(dis_asm_h) $(arch_utils_h) $(gdbtypes_h) \
 	$(gdb_string_h) $(regcache_h) $(reggroups_h) $(gdbcore_h) \