From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6081 invoked by alias); 20 Jan 2006 22:41:23 -0000 Received: (qmail 6064 invoked by uid 22791); 20 Jan 2006 22:41:21 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Fri, 20 Jan 2006 22:41:18 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1F04wV-0005Mm-Sn; Fri, 20 Jan 2006 17:41:15 -0500 Date: Fri, 20 Jan 2006 22:41:00 -0000 From: Daniel Jacobowitz To: Andrew STUBBS Cc: gdb-patches@sources.redhat.com Subject: Re: [PATCH] Disable thread specific breakpoints when thread dies Message-ID: <20060120224115.GB16076@nevyn.them.org> Mail-Followup-To: Andrew STUBBS , gdb-patches@sources.redhat.com References: <20051114155659.GA25717@nevyn.them.org> <437A19DE.6040905@st.com> <437B47A1.4040705@st.com> <20051117034811.GB3057@nevyn.them.org> <437CA66B.9060201@st.com> <20060112162659.GA16141@nevyn.them.org> <43C7E466.9080703@st.com> <20060114160611.GA12603@nevyn.them.org> <43CB97A2.20205@st.com> <43CBC6EB.9080904@st.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <43CBC6EB.9080904@st.com> User-Agent: Mutt/1.5.8i X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-01/txt/msg00255.txt.bz2 On Mon, Jan 16, 2006 at 04:16:43PM +0000, Andrew STUBBS wrote: > Andrew STUBBS wrote: > >Now onto figuring out why it doesn't work.... > > The smoking gun seems to be here: > > [From linux-thread-db.c] > > static void > detach_thread (ptid_t ptid, int verbose) > { > struct thread_info *thread_info; > > if (verbose) > printf_unfiltered (_("[%s exited]\n"), target_pid_to_str (ptid)); > > /* Don't delete the thread now, because it still reports as active > until it has executed a few instructions after the event > breakpoint - if we deleted it now, "info threads" would cause us > to re-attach to it. Just mark it as having had a TD_DEATH > event. This means that we won't delete it from our thread list > until we notice that it's dead (via prune_threads), or until > something re-uses its thread ID. */ > thread_info = find_thread_pid (ptid); > gdb_assert (thread_info != NULL); > thread_info->private->dying = 1; > } Nice catch - although there's a second smoking gun besides this one, which is that linux-nat.c won't delete threads that are the current inferior_ptid. With good reason. GDB absolutely falls apart when you do that. I think the long term fix for the latter is going to involve some serious surgery on the notion of a current global inferior_ptid. But until that happens, here's a workaround confined to the Linux native code. The big advantage over calling prune_threads is we don't need to make a lot of expensive syscalls. Any comments? Does this work for you? It does seem to avoid inserting the breakpoint unnecessarily, as I'd expect. -- Daniel Jacobowitz CodeSourcery 2006-01-20 Daniel Jacobowitz * linux-nat.c (struct saved_ptids, threads_to_delete) (record_dead_thread, prune_lwps, find_thread_from_lwp) (exit_lwp): New. (linux_nat_resume): Call prune_lwps. (wait_lwp, linux_nat_wait): Call exit_lwp. Index: linux-nat.c =================================================================== RCS file: /cvs/src/src/gdb/linux-nat.c,v retrieving revision 1.36 diff -u -p -r1.36 linux-nat.c --- linux-nat.c 4 Jan 2006 19:34:58 -0000 1.36 +++ linux-nat.c 20 Jan 2006 22:36:46 -0000 @@ -868,6 +868,92 @@ iterate_over_lwps (int (*callback) (stru return NULL; } +/* Record a PTID for later deletion. */ + +struct saved_ptids +{ + ptid_t ptid; + struct saved_ptids *next; +}; +static struct saved_ptids *threads_to_delete; + +static void +record_dead_thread (ptid_t ptid) +{ + struct saved_ptids *p = xmalloc (sizeof (struct saved_ptids)); + p->ptid = ptid; + p->next = threads_to_delete; + threads_to_delete = p; +} + +/* Delete any dead threads which are not the current thread. */ + +static void +prune_lwps (void) +{ + struct saved_ptids **p = &threads_to_delete; + + while (*p) + if (! ptid_equal ((*p)->ptid, inferior_ptid)) + { + struct saved_ptids *tmp = *p; + delete_thread (tmp->ptid); + *p = tmp->next; + xfree (tmp); + } + else + p = &(*p)->next; +} + +/* Callback for iterate_over_threads that finds a thread corresponding + to the given LWP. */ + +static int +find_thread_from_lwp (struct thread_info *thr, void *dummy) +{ + ptid_t *ptid_p = dummy; + + if (GET_LWP (thr->ptid) && GET_LWP (thr->ptid) == GET_LWP (*ptid_p)) + return 1; + else + return 0; +} + +/* Handle the exit of a single thread LP. */ + +static void +exit_lwp (struct lwp_info *lp) +{ + if (in_thread_list (lp->ptid)) + { + /* Core GDB cannot deal with us deleting the current thread. */ + if (!ptid_equal (lp->ptid, inferior_ptid)) + delete_thread (lp->ptid); + else + record_dead_thread (lp->ptid); + printf_unfiltered (_("[%s exited]\n"), + target_pid_to_str (lp->ptid)); + } + else + { + /* Even if LP->PTID is not in the global GDB thread list, the + LWP may be - with an additional thread ID. We don't need + to print anything in this case; thread_db is in use and + already took care of that. But it didn't delete the thread + in order to handle zombies correctly. */ + + struct thread_info *thr; + + thr = iterate_over_threads (find_thread_from_lwp, &lp->ptid); + if (thr && !ptid_equal (thr->ptid, inferior_ptid)) + delete_thread (thr->ptid); + else + record_dead_thread (thr->ptid); + } + + delete_lwp (lp->ptid); +} + /* Attach to the LWP specified by PID. If VERBOSE is non-zero, print a message telling the user that a new LWP has been added to the process. */ @@ -1121,6 +1207,8 @@ linux_nat_resume (ptid_t ptid, int step, signo ? strsignal (signo) : "0", target_pid_to_str (inferior_ptid)); + prune_lwps (); + /* A specific PTID means `step only this process id'. */ resume_all = (PIDGET (ptid) == -1); @@ -1321,16 +1409,7 @@ wait_lwp (struct lwp_info *lp) if (thread_dead) { - if (in_thread_list (lp->ptid)) - { - /* Core GDB cannot deal with us deleting the current thread. */ - if (!ptid_equal (lp->ptid, inferior_ptid)) - delete_thread (lp->ptid); - printf_unfiltered (_("[%s exited]\n"), - target_pid_to_str (lp->ptid)); - } - - delete_lwp (lp->ptid); + exit_lwp (lp); return 0; } @@ -2117,16 +2196,6 @@ retry: /* Check if the thread has exited. */ if ((WIFEXITED (status) || WIFSIGNALED (status)) && num_lwps > 1) { - if (in_thread_list (lp->ptid)) - { - /* Core GDB cannot deal with us deleting the current - thread. */ - if (!ptid_equal (lp->ptid, inferior_ptid)) - delete_thread (lp->ptid); - printf_unfiltered (_("[%s exited]\n"), - target_pid_to_str (lp->ptid)); - } - /* If this is the main thread, we must stop all threads and verify if they are still alive. This is because in the nptl thread model, there is no signal issued for exiting LWPs @@ -2148,7 +2217,7 @@ retry: "LLW: %s exited.\n", target_pid_to_str (lp->ptid)); - delete_lwp (lp->ptid); + exit_lwp (lp); /* If there is at least one more LWP, then the exit signal was not the end of the debugged application and should be @@ -2170,21 +2239,12 @@ retry: has stopped. A similar check is made in stop_wait_callback(). */ if (num_lwps > 1 && !linux_nat_thread_alive (lp->ptid)) { - if (in_thread_list (lp->ptid)) - { - /* Core GDB cannot deal with us deleting the current - thread. */ - if (!ptid_equal (lp->ptid, inferior_ptid)) - delete_thread (lp->ptid); - printf_unfiltered (_("[%s exited]\n"), - target_pid_to_str (lp->ptid)); - } if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, "LLW: %s exited.\n", target_pid_to_str (lp->ptid)); - delete_lwp (lp->ptid); + exit_lwp (lp); /* Make sure there is at least one thread running. */ gdb_assert (iterate_over_lwps (running_callback, NULL));