Re: RFA: fix for thread events in thread-db.c

["J. Johnston" <jjohnstn@redhat.com>]

Joel Brobecker wrote:
>>Ok to commit?
>>
>>-- Jeff J.
>>
>>2003-04-23  Jeff Johnston  <jjohnstn@redhat.com>
>>
>>	* thread-db.c (check_event): For create/death event breakpoints,
>>	loop through all messages to ensure that we read the message
>>	corresponding to the breakpoint we are at.
> 
> 
> Jeff, this patch has been approved but there is something I don't get.
> Is the loop variable really necessary? It doesn't seem to ever be
> changed except when you set it right before looping. And the exit from
> the "do ... while" loop seems to be done via a "return".
> 
> Thanks.
> 

I made it a loop so one can turn on and off the looping externally.
This makes sense if events other than the TD_CREATE event are supported.
For such events, you can use the other libthread_db "get message" interface which allows
you to specify the thread you are interested in and then there is no reason
to loop.  I have, for example, made this work for the TD_DEATH event.

The comment explains this; i.e. the loop is not always necessary.
I could have just as easily made it a for (;;) loop but this is functionally
equivalent and makes it sort of obvious how to change it in the future.
> 
>>Index: thread-db.c
>>===================================================================
>>RCS file: /cvs/src/src/gdb/thread-db.c,v
>>retrieving revision 1.30
>>diff -u -r1.30 thread-db.c
>>--- thread-db.c	17 Apr 2003 17:30:02 -0000	1.30
>>+++ thread-db.c	23 Apr 2003 19:37:47 -0000
>>@@ -775,64 +775,73 @@
>>   td_thrinfo_t ti;
>>   td_err_e err;
>>   CORE_ADDR stop_pc;
>>+  int loop = 0;
>> 
>>   /* Bail out early if we're not at a thread event breakpoint.  */
>>   stop_pc = read_pc_pid (ptid) - DECR_PC_AFTER_BREAK;
>>   if (stop_pc != td_create_bp_addr && stop_pc != td_death_bp_addr)
>>     return;
>> 
>>-  err = td_ta_event_getmsg_p (thread_agent, &msg);
>>-  if (err != TD_OK)
>>+  /* If we are at a create breakpoint, we do not know what new lwp
>>+     was created and cannot specifically locate the event message for it.
>>+     We have to call td_ta_event_getmsg() to get
>>+     the latest message.  Since we have no way of correlating whether
>>+     the event message we get back corresponds to our breakpoint, we must 
>>+     loop and read all event messages, processing them appropriately.
>>+     This guarantees we will process the correct message before continuing 
>>+     from the breakpoint.  
>>+
>>+     Currently, death events are not enabled.  If they are enabled,
>>+     the death event can use the td_thr_event_getmsg() interface to
>>+     get the message specifically for that lwp and avoid looping
>>+     below.  */
>>+
>>+  loop = 1;
>>+
>>+  do
>>     {
>>-      if (err == TD_NOMSG)
>>-	return;
>>+      err = td_ta_event_getmsg_p (thread_agent, &msg);
>>+      if (err != TD_OK)
>>+	{
>>+	  if (err == TD_NOMSG)
>>+	    return;
>> 
>>-      error ("Cannot get thread event message: %s", thread_db_err_str (err));
>>-    }
>>+	  error ("Cannot get thread event message: %s",
>>+		 thread_db_err_str (err));
>>+	}
>> 
>>-  err = td_thr_get_info_p (msg.th_p, &ti);
>>-  if (err != TD_OK)
>>-    error ("check_event: cannot get thread info: %s",
>>-	   thread_db_err_str (err));
>>+      err = td_thr_get_info_p (msg.th_p, &ti);
>>+      if (err != TD_OK)
>>+	error ("Cannot get thread info: %s", thread_db_err_str (err));
>> 
>>-  ptid = BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
>>+      ptid = BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
>> 
>>-  switch (msg.event)
>>-    {
>>-    case TD_CREATE:
>>-#if 0
>>-      /* FIXME: kettenis/2000-08-26: Since we use td_ta_event_getmsg,
>>-         there is no guarantee that the breakpoint will match the
>>-         event.  Should we use td_thr_event_getmsg instead?  */
>>-
>>-      if (stop_pc != td_create_bp_addr)
>>-	error ("Thread creation event doesn't match breakpoint.");
>>-#endif
>>-
>>-      /* We may already know about this thread, for instance when the
>>-         user has issued the `info threads' command before the SIGTRAP
>>-         for hitting the thread creation breakpoint was reported.  */
>>-      if (!in_thread_list (ptid))
>>-	attach_thread (ptid, msg.th_p, &ti, 1);
>>-      return;
>>-
>>-    case TD_DEATH:
>>-#if 0
>>-      /* FIXME: See TD_CREATE.  */
>>-
>>-      if (stop_pc != td_death_bp_addr)
>>-	error ("Thread death event doesn't match breakpoint.");
>>-#endif
>>+      switch (msg.event)
>>+	{
>>+	case TD_CREATE:
>>+
>>+	  /* We may already know about this thread, for instance when the
>>+	     user has issued the `info threads' command before the SIGTRAP
>>+	     for hitting the thread creation breakpoint was reported.  */
>>+	  if (!in_thread_list (ptid))
>>+	    attach_thread (ptid, msg.th_p, &ti, 1);
>>+
>>+	  break;
>>+
>>+	case TD_DEATH:
>>+
>>+	  if (!in_thread_list (ptid))
>>+	    error ("Spurious thread death event.");
>> 
>>-      if (!in_thread_list (ptid))
>>-	error ("Spurious thread death event.");
>>+	  detach_thread (ptid, 1);
>> 
>>-      detach_thread (ptid, 1);
>>-      return;
>>+	  break;
>> 
>>-    default:
>>-      error ("Spurious thread event.");
>>+	default:
>>+	  error ("Spurious thread event.");
>>+	}
>>     }
>>+  while (loop);
>> }
>> 
>> static ptid_t
> 
>