[RFA] add some comments to linux-low.c:linux_attach_lwp

[dje at google dot com (Doug Evans)]

Hi.

Until things are fixed, I'd like to record some knowledge so
I don't forget.  This also does a better job (IMO) of enumerating
all the cases that need to be handled here.

Ok to check in?

2008-12-13  Doug Evans  <dje@google.com>

	* linux-low.c (linux_attach_lwp): Add some comments/fixmes.

Index: linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.84
diff -u -p -r1.84 linux-low.c
--- linux-low.c	12 Dec 2008 23:14:28 -0000	1.84
+++ linux-low.c	13 Dec 2008 21:24:00 -0000
@@ -326,6 +326,8 @@ linux_attach_lwp (unsigned long pid)
 	       strerror (errno), errno);
     }
 
+  /* FIXME: This intermittently fails.
+     We need to wait for SIGSTOP first.  */
   ptrace (PTRACE_SETOPTIONS, pid, 0, PTRACE_O_TRACECLONE);
 
   new_process = (struct process_info *) add_process (pid);
@@ -333,15 +335,36 @@ linux_attach_lwp (unsigned long pid)
   new_thread_notify (thread_id_to_gdb_id (new_process->lwpid));
 
   /* The next time we wait for this LWP we'll see a SIGSTOP as PTRACE_ATTACH
-     brings it to a halt.  We should ignore that SIGSTOP and resume the process
-     (unless this is the first process, in which case the flag will be cleared
-     in linux_attach).
+     brings it to a halt.
+
+     There are several cases to consider here:
+
+     1) gdbserver has already attached to the process and is being notified
+        of a new thread that is being created.
+        In this case we should ignore that SIGSTOP and resume the process.
+        This is handled below by setting stop_expected = 1.
+
+     2) This is the first thread (the process thread), and we're attaching
+        to it via attach_inferior.
+        In this case we want the process thread to stop.
+        This is handled by having linux_attach clear stop_expected after
+        we return.
+        ??? If the process already has several threads we leave the other
+        threads running.
+
+     3) GDB is connecting to gdbserver and is requesting an enumeration of all
+        existing threads.
+        In this case we want the thread to stop.
+        FIXME: This case is currently not properly handled.
+        We should wait for the SIGSTOP but don't.  Things work apparently
+        because enough time passes between when we ptrace (ATTACH) and when
+        gdb makes the next ptrace call on the thread.
 
      On the other hand, if we are currently trying to stop all threads, we
      should treat the new thread as if we had sent it a SIGSTOP.  This works
-     because we are guaranteed that add_process added us to the end of the
-     list, and so the new thread has not yet reached wait_for_sigstop (but
-     will).  */
+     because we are guaranteed that the add_process call above added us to the
+     end of the list, and so the new thread has not yet reached
+     wait_for_sigstop (but will).  */
   if (! stopping_threads)
     new_process->stop_expected = 1;
 }