Re: lin-lwp.c question

[Mark Kettenis <kettenis at wins dot uva dot nl>]

   Date: Sun, 17 Sep 2000 19:00:09 -0700
   From: Kevin Buettner <kevinb@cygnus.com>

   On Sep 17, 11:04pm, Mark Kettenis wrote:

   > Anyway, I believe it is pretty safe to remove the assertion.  Feel
   > free to check in a patch that does just that.

   Something is still wrong.  I've removed the assertion with the following
   patch:

[Patch deleted]

   But now I'm seeing assertion failures on the following statement:

	     gdb_assert (in_thread_list (lp->pid));

   I've taken a look at lp->pid on one of these failures and it looks
   like a valid LWP.  (Its value is 0x6f6c6f57.) I've taken a look at the
   thread list and lp->pid isn't there.  However, in this case, I don't
   think it's supposed to be there since the pid values in the thread
   list represent threads instead of LWPs.  (They have values like
   0xd0156f57 and 0x88016f57.  The thing to note about these values is
   that bit 31 is set which indicates that the value is a thread id as
   opposed to an LWP id.)

Correct.  I should have taken a closer look last night.

   It seems to me that the assert should say something like

	     gdb_assert (in_thread_list (thread_from_lwp (lp->pid)));

   Also, a few lines down the delete_thread() call would have to become

	       delete_thread (thread_from_lwp (lp->pid));

   Except that this won't work because thread_from_lwp is a static
   function in thread-db.c and is therefore inaccessible to lin-lwp.c. 
   Also, I still don't understand your work well enough yet to know if
   this is a sensible suggestion or not.

Not really.  The code for deleting threads from the thread list is
supposed to be used in the case that the lin-lwp stuff is used without
a user-level threads library, i.e. when the threads in GDB's threads
list are the LWP's themselves.  The attached patch (checked in) makes
fixes things such that it works with LinuxThreads too.

   Stepping back, I think one of the problems with all of this (not your
   fault) is that we're trying to overload an poor lowly little int with
   far too much information.  As currently structured, our pid values can
   either represent plain pids, a pid + an lwp, or a pid + a thread id,
   with no provision for representing a pid + an lwp + a thread id.  Not
   only is it all very confusing and error prone, but we're very limited
   in the range of values that we can use for thread ids, pids, and lwp
   ids.

I totally agree with you.  Although it's possible to work around most
of the problems.  The idea is to have a clear seperation between the
threads, lwp and pid layers.  The threads layer converts its pid +
thread id into a pid + lwp, and manipulates the user-level threads in
the thread list.  The LWP layer converts its pid + lwp to a simple pid
and only manipulates the LWP information in the thread list.  It's
just that I didn't implement that last bit correctly :-(.

   In the next day or so, I'm going to post some WIP patches to the list
   which attempt to address these problems.  (Warning: the patches are
   quite substantial and will potentially break many builds.  I'm trying
   to test as well as I can, but there are some platforms and environments
   which I simply don't have access to.)

Great!


2000-09-18  Mark Kettenis  <kettenis@gnu.org>

	* lin-lwp.c (stop_wait_callback): Remove bogus assertions in the
	code that deals with exiting/signalled threads.  Replace with
	code similar to what's done in lin_lwp_wait.


Index: lin-lwp.c
===================================================================
RCS file: /cvs/src/src/gdb/lin-lwp.c,v
retrieving revision 1.2
diff -u -p -r1.2 lin-lwp.c
--- lin-lwp.c	2000/09/09 07:54:20	1.2
+++ lin-lwp.c	2000/09/18 13:04:15
@@ -506,14 +506,19 @@ stop_wait_callback (struct lwp_info *lp,
       if (WIFEXITED (status) || WIFSIGNALED (status))
 	{
 	  gdb_assert (num_lwps > 1);
-	  gdb_assert (! is_cloned (lp->pid));
 
-	  gdb_assert (in_thread_list (lp->pid));
-	  if (lp->pid != inferior_pid)
-	    delete_thread (lp->pid);
-	  printf_unfiltered ("[%s exited]\n",
-			     target_pid_to_str (lp->pid));
-
+	  if (in_thread_list (lp->pid))
+	    {
+	      /* Core GDB cannot deal with us deleting the current
+		 thread.  */
+	      if (lp->pid != inferior_pid)
+		delete_thread (lp->pid);
+	      printf_unfiltered ("[%s exited]\n",
+				 target_pid_to_str (lp->pid));
+	    }
+#if DEBUG
+	  printf ("%s exited.\n", target_pid_to_str (lp->pid));
+#endif
 	  delete_lwp (lp->pid);
 	  return 0;
 	}
@@ -708,7 +713,7 @@ lin_lwp_wait (int pid, struct target_wai
 	    {
 	      if (in_thread_list (lp->pid))
 		{
-		  /* Core GDB cannot deal with us deeting the current
+		  /* Core GDB cannot deal with us deleting the current
                      thread.  */
 		  if (lp->pid != inferior_pid)
 		    delete_thread (lp->pid);