[commit] Various improvements to gdbserver + GNU/Linux threads

[Daniel Jacobowitz <drow at false dot org>]

Most of this patch is just comment improvements and cleanups.  One
notable fix is that we were not removing breakpoints before detaching
from the program; this only applies to breakpoints managed by
gdbserver, i.e. the thread event breakpoint.  So the next time the
detached program created a thread it was likely to crash.  And we
weren't flushing out any updated registers.  I also made the first
thread handling for libthread_db a bit more efficient.

Tested on x86_64-linux with glibc 2.5 and HEAD.

-- 
Daniel Jacobowitz
CodeSourcery

2007-07-02  Daniel Jacobowitz  <dan@codesourcery.com>

	* inferiors.c (change_inferior_id): Add comment.
	* linux-low.c (check_removed_breakpoint): Add an early
	prototype.  Improve debug output.
	(linux_attach): Doc update.
	(linux_detach_one_process, linux_detach): Clean up before releasing
	each process.
	(send_sigstop, wait_for_sigstop): Improve comments and debug output.
	* linux-low.h (struct process_info): Doc improvement.
	* mem-break.c (delete_all_breakpoints): New.
	* mem-break.h (delete_all_breakpoints): New prototype.
	* thread-db.c (find_first_thread): New.
	(thread_db_create_event): Call it instead of
	thread_db_find_new_threads.  Clean up unused variables.
	(maybe_attach_thread): Remove first thread handling.
	(thread_db_find_new_threads): Use find_first_thread.
	(thread_db_get_tls_address): Likewise.

Index: inferiors.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/inferiors.c,v
retrieving revision 1.9
diff -u -p -r1.9 inferiors.c
--- inferiors.c	9 Jan 2007 17:59:08 -0000	1.9
+++ inferiors.c	2 Jul 2007 14:56:25 -0000
@@ -64,6 +64,11 @@ for_each_inferior (struct inferior_list 
     }
 }
 
+/* When debugging a single-threaded program, the threads list (such as
+   it is) is indexed by PID.  When debugging a multi-threaded program,
+   we index by TID.  This ugly routine replaces the
+   first-debugged-thread's PID with its TID.  */
+
 void
 change_inferior_id (struct inferior_list *list,
 		    unsigned long new_id)
Index: linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.58
diff -u -p -r1.58 linux-low.c
--- linux-low.c	20 Jun 2007 18:54:21 -0000	1.58
+++ linux-low.c	2 Jul 2007 14:56:25 -0000
@@ -68,6 +68,7 @@ static void linux_resume_one_process (st
 static void linux_resume (struct thread_resume *resume_info);
 static void stop_all_processes (void);
 static int linux_wait_for_event (struct thread_info *child);
+static int check_removed_breakpoint (struct process_info *event_child);
 
 struct pending_signals
 {
@@ -224,7 +225,8 @@ linux_attach (unsigned long pid)
 
   linux_attach_lwp (pid, pid);
 
-  /* Don't ignore the initial SIGSTOP if we just attached to this process.  */
+  /* Don't ignore the initial SIGSTOP if we just attached to this process.
+     It will be collected by wait shortly.  */
   process = (struct process_info *) find_inferior_id (&all_processes, pid);
   process->stop_expected = 0;
 
@@ -286,13 +288,36 @@ linux_detach_one_process (struct inferio
   struct thread_info *thread = (struct thread_info *) entry;
   struct process_info *process = get_thread_process (thread);
 
+  /* Make sure the process isn't stopped at a breakpoint that's
+     no longer there.  */
+  check_removed_breakpoint (process);
+
+  /* If this process is stopped but is expecting a SIGSTOP, then make
+     sure we take care of that now.  This isn't absolutely guaranteed
+     to collect the SIGSTOP, but is fairly likely to.  */
+  if (process->stop_expected)
+    {
+      /* Clear stop_expected, so that the SIGSTOP will be reported.  */
+      process->stop_expected = 0;
+      if (process->stopped)
+	linux_resume_one_process (&process->head, 0, 0, NULL);
+      linux_wait_for_event (thread);
+    }
+
+  /* Flush any pending changes to the process's registers.  */
+  regcache_invalidate_one ((struct inferior_list_entry *)
+			   get_process_thread (process));
+
+  /* Finally, let it resume.  */
   ptrace (PTRACE_DETACH, pid_of (process), 0, 0);
 }
 
 static int
 linux_detach (void)
 {
+  delete_all_breakpoints ();
   for_each_inferior (&all_threads, linux_detach_one_process);
+  clear_inferiors ();
   return 0;
 }
 
@@ -332,7 +357,8 @@ check_removed_breakpoint (struct process
     return 0;
 
   if (debug_threads)
-    fprintf (stderr, "Checking for breakpoint.\n");
+    fprintf (stderr, "Checking for breakpoint in process %ld.\n",
+	     event_child->lwpid);
 
   saved_inferior = current_inferior;
   current_inferior = get_process_thread (event_child);
@@ -345,7 +371,8 @@ check_removed_breakpoint (struct process
   if (stop_pc != event_child->pending_stop_pc)
     {
       if (debug_threads)
-	fprintf (stderr, "Ignoring, PC was changed.\n");
+	fprintf (stderr, "Ignoring, PC was changed.  Old PC was 0x%08llx\n",
+		 event_child->pending_stop_pc);
 
       event_child->pending_is_breakpoint = 0;
       current_inferior = saved_inferior;
@@ -817,6 +844,13 @@ send_sigstop (struct inferior_list_entry
      send another.  */
   if (process->stop_expected)
     {
+      if (debug_threads)
+	fprintf (stderr, "Have pending sigstop for process %ld\n",
+		 process->lwpid);
+
+      /* We clear the stop_expected flag so that wait_for_sigstop
+	 will receive the SIGSTOP event (instead of silently resuming and
+	 waiting again).  It'll be reset below.  */
       process->stop_expected = 0;
       return;
     }
@@ -852,7 +886,9 @@ wait_for_sigstop (struct inferior_list_e
       && WSTOPSIG (wstat) != SIGSTOP)
     {
       if (debug_threads)
-	fprintf (stderr, "Stopped with non-sigstop signal\n");
+	fprintf (stderr, "Process %ld (thread %ld) "
+		 "stopped with non-sigstop status %06x\n",
+		 process->lwpid, process->tid, wstat);
       process->status_pending_p = 1;
       process->status_pending = wstat;
       process->stop_expected = 1;
Index: linux-low.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.h,v
retrieving revision 1.16
diff -u -p -r1.16 linux-low.h
--- linux-low.h	9 Jan 2007 22:55:10 -0000	1.16
+++ linux-low.h	2 Jul 2007 14:56:25 -0000
@@ -92,8 +92,12 @@ struct process_info
   unsigned long lwpid;
   unsigned long tid;
 
-  /* If this flag is set, the next SIGSTOP will be ignored (the process will
-     be immediately resumed).  */
+  /* If this flag is set, the next SIGSTOP will be ignored (the
+     process will be immediately resumed).  This means that either we
+     sent the SIGSTOP to it ourselves and got some other pending event
+     (so the SIGSTOP is still pending), or that we stopped the
+     inferior implicitly via PTRACE_ATTACH and have not waited for it
+     yet.  */
   int stop_expected;
 
   /* If this flag is set, the process is known to be stopped right now (stop
Index: mem-break.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/mem-break.c,v
retrieving revision 1.7
diff -u -p -r1.7 mem-break.c
--- mem-break.c	29 Mar 2007 01:06:47 -0000	1.7
+++ mem-break.c	2 Jul 2007 14:56:25 -0000
@@ -283,3 +283,12 @@ check_mem_write (CORE_ADDR mem_addr, uns
 	memcpy (buf + buf_offset, breakpoint_data + copy_offset, copy_len);
     }
 }
+
+/* Delete all breakpoints.  */
+
+void
+delete_all_breakpoints (void)
+{
+  while (breakpoints)
+    delete_breakpoint (breakpoints);
+}
Index: mem-break.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/mem-break.h,v
retrieving revision 1.5
diff -u -p -r1.5 mem-break.h
--- mem-break.h	29 Mar 2007 01:06:47 -0000	1.5
+++ mem-break.h	2 Jul 2007 14:56:25 -0000
@@ -72,4 +72,8 @@ void check_mem_write (CORE_ADDR mem_addr
 
 void set_breakpoint_data (const unsigned char *bp_data, int bp_len);
 
+/* Delete all breakpoints.  */
+
+void delete_all_breakpoints (void);
+
 #endif /* MEM_BREAK_H */
Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/thread-db.c,v
retrieving revision 1.10
diff -u -p -r1.10 thread-db.c
--- thread-db.c	27 Jun 2007 11:52:02 -0000	1.10
+++ thread-db.c	2 Jul 2007 14:56:25 -0000
@@ -41,7 +41,7 @@ static struct ps_prochandle proc_handle;
 /* Connection to the libthread_db library.  */
 static td_thragent_t *thread_agent;
 
-static void thread_db_find_new_threads (void);
+static int find_first_thread (void);
 static int find_new_threads_callback (const td_thrhandle_t *th_p, void *data);
 
 static char *
@@ -135,15 +135,11 @@ thread_db_create_event (CORE_ADDR where)
 {
   td_event_msg_t msg;
   td_err_e err;
-  struct inferior_linux_data *tdata;
-  struct thread_info *inferior;
   struct process_info *process;
 
   if (debug_threads)
     fprintf (stderr, "Thread creation event.\n");
 
-  tdata = inferior_target_data (current_inferior);
-
   /* FIXME: This assumes we don't get another event.
      In the LinuxThreads implementation, this is safe,
      because all events come from the manager thread
@@ -156,10 +152,9 @@ thread_db_create_event (CORE_ADDR where)
   /* If we do not know about the main thread yet, this would be a good time to
      find it.  We need to do this to pick up the main thread before any newly
      created threads.  */
-  inferior = (struct thread_info *) all_threads.head;
-  process = get_thread_process (inferior);
+  process = get_thread_process (current_inferior);
   if (process->thread_known == 0)
-    thread_db_find_new_threads ();
+    find_first_thread ();
 
   /* msg.event == TD_EVENT_CREATE */
 
@@ -230,43 +225,73 @@ thread_db_enable_reporting ()
   return 1;
 }
 
-static void
-maybe_attach_thread (const td_thrhandle_t *th_p, td_thrinfo_t *ti_p)
+static int
+find_first_thread (void)
 {
+  td_thrhandle_t th;
+  td_thrinfo_t ti;
   td_err_e err;
   struct thread_info *inferior;
   struct process_info *process;
 
-  /* If we are attaching to our first thread, things are a little
-     different.  */
-  if (all_threads.head == all_threads.tail)
-    {
-      inferior = (struct thread_info *) all_threads.head;
-      process = get_thread_process (inferior);
+  inferior = (struct thread_info *) all_threads.head;
+  process = get_thread_process (inferior);
+  if (process->thread_known)
+    return 1;
+
+  /* Get information about the one thread we know we have.  */
+  err = td_ta_map_lwp2thr (thread_agent, process->lwpid, &th);
+  if (err != TD_OK)
+    error ("Cannot get first thread handle: %s", thread_db_err_str (err));
+
+  err = td_thr_get_info (&th, &ti);
+  if (err != TD_OK)
+    error ("Cannot get first thread info: %s", thread_db_err_str (err));
 
-      if (process->thread_known == 0)
-	{
-	  /* If the new thread ID is zero, a final thread ID will be
-	     available later.  Do not enable thread debugging yet.  */
-	  if (ti_p->ti_tid == 0)
-	    {
-	      err = td_thr_event_enable (th_p, 1);
-	      if (err != TD_OK)
-		error ("Cannot enable thread event reporting for %d: %s",
-		       ti_p->ti_lid, thread_db_err_str (err));
-	      return;
-	    }
-
-	  if (process->lwpid != ti_p->ti_lid)
-	    fatal ("PID mismatch!  Expected %ld, got %ld",
-		   (long) process->lwpid, (long) ti_p->ti_lid);
-
-	  /* Switch to indexing the threads list by TID.  */
-	  change_inferior_id (&all_threads, ti_p->ti_tid);
-	  goto found;
-	}
+  if (debug_threads)
+    fprintf (stderr, "Found first thread %ld (LWP %d)\n",
+	     ti.ti_tid, ti.ti_lid);
+
+  if (process->lwpid != ti.ti_lid)
+    fatal ("PID mismatch!  Expected %ld, got %ld",
+	   (long) process->lwpid, (long) ti.ti_lid);
+
+  /* If the new thread ID is zero, a final thread ID will be available
+     later.  Do not enable thread debugging yet.  */
+  if (ti.ti_tid == 0)
+    {
+      err = td_thr_event_enable (&th, 1);
+      if (err != TD_OK)
+	error ("Cannot enable thread event reporting for %d: %s",
+	       ti.ti_lid, thread_db_err_str (err));
+      return 0;
     }
-  
+
+  /* Switch to indexing the threads list by TID.  */
+  change_inferior_id (&all_threads, ti.ti_tid);
+
+  new_thread_notify (ti.ti_tid);
+
+  process->tid = ti.ti_tid;
+  process->lwpid = ti.ti_lid;
+  process->thread_known = 1;
+  process->th = th;
+
+  err = td_thr_event_enable (&th, 1);
+  if (err != TD_OK)
+    error ("Cannot enable thread event reporting for %d: %s",
+           ti.ti_lid, thread_db_err_str (err));
+
+  return 1;
+}
+
+static void
+maybe_attach_thread (const td_thrhandle_t *th_p, td_thrinfo_t *ti_p)
+{
+  td_err_e err;
+  struct thread_info *inferior;
+  struct process_info *process;
+
   inferior = (struct thread_info *) find_inferior_id (&all_threads,
 						      ti_p->ti_tid);
   if (inferior != NULL)
@@ -287,7 +312,6 @@ maybe_attach_thread (const td_thrhandle_
 
   process = inferior_target_data (inferior);
 
-found:
   new_thread_notify (ti_p->ti_tid);
 
   process->tid = ti_p->ti_tid;
@@ -325,6 +349,12 @@ thread_db_find_new_threads (void)
 {
   td_err_e err;
 
+  /* This function is only called when we first initialize thread_db.
+     First locate the initial thread.  If it is not ready for
+     debugging yet, then stop.  */
+  if (find_first_thread () == 0)
+    return;
+
   /* Iterate over all user-space threads to discover new threads.  */
   err = td_ta_thr_iter (thread_agent, find_new_threads_callback, NULL,
 			TD_THR_ANY_STATE, TD_THR_LOWEST_PRIORITY,
@@ -359,7 +389,7 @@ thread_db_get_tls_address (struct thread
 
   process = get_thread_process (thread);
   if (!process->thread_known)
-    thread_db_find_new_threads ();
+    find_first_thread ();
   if (!process->thread_known)
     return TD_NOTHR;