From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15180 invoked by alias); 4 Jun 2003 23:39:34 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 15126 invoked from network); 4 Jun 2003 23:39:33 -0000 Received: from unknown (HELO touchme.toronto.redhat.com) (207.219.125.105) by sources.redhat.com with SMTP; 4 Jun 2003 23:39:33 -0000 Received: from redhat.com (toocool.toronto.redhat.com [172.16.14.72]) by touchme.toronto.redhat.com (Postfix) with ESMTP id 2CC72800021; Wed, 4 Jun 2003 19:39:33 -0400 (EDT) Message-ID: <3EDE8335.5060708@redhat.com> Date: Wed, 04 Jun 2003 23:39:00 -0000 From: "J. Johnston" Organization: Red Hat Inc. User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Joel Brobecker Cc: gdb-patches@sources.redhat.com Subject: Re: RFA: fix for thread events in thread-db.c References: <3EA6EDD8.8000100@redhat.com> <20030604215811.GA3088@gnat.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-06/txt/msg00190.txt.bz2 Joel Brobecker wrote: >>Ok to commit? >> >>-- Jeff J. >> >>2003-04-23 Jeff Johnston >> >> * 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 > >