From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21565 invoked by alias); 15 Jul 2008 16:34:56 -0000 Received: (qmail 21554 invoked by uid 22791); 15 Jul 2008 16:34:55 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 15 Jul 2008 16:34:34 +0000 Received: (qmail 15262 invoked from network); 15 Jul 2008 16:34:32 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 15 Jul 2008 16:34:32 -0000 From: Pedro Alves To: gdb@sourceware.org, luisgpm@linux.vnet.ibm.com Subject: Re: (maybe) Async mode failures on PPC Date: Tue, 15 Jul 2008 16:34:00 -0000 User-Agent: KMail/1.9.9 References: <1216050287.2607.10.camel@gargoyle> <1216055220.2607.18.camel@gargoyle> <1216133200.7260.4.camel@gargoyle> In-Reply-To: <1216133200.7260.4.camel@gargoyle> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200807151734.30838.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2008-07/txt/msg00177.txt.bz2 Hi Luis, Thanks for taking care of this. > 2008-07-15 Luis Machado > > * infrun.c (handle_inferior_event): reinit_frame_cache before > handling events on threads. As I noted on IRC, this is a bit incomplete, but I know you're already handling it. :-) On Tuesday 15 July 2008 15:46:40, Luis Machado wrote: > On Mon, 2008-07-14 at 14:06 -0300, Luis Machado wrote: > Here is a patch i discussed with Pedro to fix this regression. Tested > and nothing showed up in the testsuite. For everyone else not following the discussion on IRC: The main issue was the insert_breakpoints call being done before the stopped thread being tagged as stopped, as Andreas also noticed. Inside insert_breakpoints, when updating watchpoints, there are calls into frame routines, which bail out if the thread is considered to be running. It isn't at this point, but we were only tagging it as stopped a bit afterwards. handle_inferior_event() { ... switch (infwait_state) { ... case infwait_nonstep_watch_state: if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: infwait_nonstep_watch_state\n"); insert_breakpoints (); ... break; ... } ... reinit_frame_cache (); /* If it's a new process, add it to the thread database */ ecs->new_thread_event = (!ptid_equal (ecs->ptid, inferior_ptid) && !ptid_equal (ecs->ptid, minus_one_ptid) && !in_thread_list (ecs->ptid)); if (ecs->ws.kind != TARGET_WAITKIND_EXITED && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED && ecs->new_thread_event) add_thread (ecs->ptid); if (ecs->ws.kind != TARGET_WAITKIND_IGNORE) { /* Mark the non-executing threads accordingly. */ if (!non_stop || ecs->ws.kind == TARGET_WAITKIND_EXITED || ecs->ws.kind == TARGET_WAITKIND_SIGNALLED) set_executing (pid_to_ptid (-1), 0); else set_executing (ecs->ptid, 0); } ... > Index: gdb/infrun.c > =================================================================== > --- gdb.orig/infrun.c 2008-07-15 09:36:19.000000000 -0500 > +++ gdb/infrun.c 2008-07-15 09:37:20.000000000 -0500 > @@ -1828,6 +1828,29 @@ > > adjust_pc_after_break (ecs); > > + reinit_frame_cache (); As seen above, this was being done only after the insert_breakpoints call. Inserting breakpoints ends up updating the watchpoints, which triggers calls into frame handling (evaluating if the frame is in scope, temporalily changing the selected frame, etc.). There's a registers_changed call just before resuming and switching to infwait_nonstep_watch_state (which calls reinit_frame_cache), and we tell the target to step only one thread, so there should be no practical problem in leaving the call where it was, but, might as well move up this call too while we're at it, in case something else changes in the future, and uncovers this issue. > + > + /* If it's a new process, add it to the thread database */ > + > + ecs->new_thread_event = (!ptid_equal (ecs->ptid, inferior_ptid) > + && !ptid_equal (ecs->ptid, minus_one_ptid) > + && !in_thread_list (ecs->ptid)); > + > + if (ecs->ws.kind != TARGET_WAITKIND_EXITED > + && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED && > ecs->new_thread_event) + add_thread (ecs->ptid); This moves up along, so any new thread that ends up being added by this is also tagged as stopped. It's the default thread state, when one adds a new thread, but we shouldn't rely on that. > + > + if (ecs->ws.kind != TARGET_WAITKIND_IGNORE) > + { > + /* Mark the non-executing threads accordingly. */ > + if (!non_stop > + || ecs->ws.kind == TARGET_WAITKIND_EXITED > + || ecs->ws.kind == TARGET_WAITKIND_SIGNALLED) > + set_executing (pid_to_ptid (-1), 0); > + else > + set_executing (ecs->ptid, 0); > + } > + This moves up, because doing this later is what was creating the problem in the first place, due to insert_breakpoints ending up calling get_current_frame. -- Pedro Alves