From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3572 invoked by alias); 24 Aug 2011 10:19:31 -0000 Received: (qmail 3563 invoked by uid 22791); 24 Aug 2011 10:19:30 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 24 Aug 2011 10:19:07 +0000 Received: (qmail 3363 invoked from network); 24 Aug 2011 10:19:06 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 24 Aug 2011 10:19:06 -0000 From: Pedro Alves To: Jan Kratochvil Subject: Re: [patch 2/2] Do not bpstat_clear_actions on throw_exception #3 Date: Wed, 24 Aug 2011 10:19:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-11-generic; KDE/4.7.0; x86_64; ; ) Cc: gdb-patches@sourceware.org References: <20110822145150.GB11817@host1.jankratochvil.net> <201108221806.15757.pedro@codesourcery.com> <20110823203257.GA4325@host1.jankratochvil.net> In-Reply-To: <20110823203257.GA4325@host1.jankratochvil.net> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201108241119.03993.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-08/txt/msg00434.txt.bz2 On Tuesday 23 August 2011 21:32:58, Jan Kratochvil wrote: > On Mon, 22 Aug 2011 19:06:15 +0200, Pedro Alves wrote: > > Not all cases. In async mode, handle_inferior_event is called > > _outside_ of execute_command, directly by the event loop (well, almost > > directly). Thus any exception thrown between bpstat_stop_status is called, > > and the bpstat_do_actions call in inf-loop.c, will leave the thread > > with a dangling bpstat too. Might be good enough to wrap > > handle_inferior_event with a similar cleanup? > > Done, thanks. On top of the same patch 1/2 as before. > > Testing is more difficult, going to post now patch 3/2 but in fact the real > continuation is not tested there as the testcase gets caught by > execute_command. I haven't done a proper testcase for the async mode. I think a hook-stop that errors would be sufficient to leave breakpoint commands dangling (normal_stop is called after handle_inferior_event, from fetch_inferior_event) > No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu. Almost there, but not yet, sorry. :-( The cleanup is installed before fetch_inferior_event (non-stop) switches inferior_ptid to the event thread with a cleanup that restores back to the previous selected thread, which means, this happens: inferior_event_handler -> install cleanup to clear the bpstat of the current thread -> fetch_inferior_event -> install cleanup to restore the selected thread -> switch to the event thread -> handle_inferior_event -> bpstat_stop_status on the now current event thread -> some error is thrown, e.g., while unwinding for step/next. -> the cleanup to restore the previous thread is ran -> the cleanup to clear the bpstat of the current thread is ran, but the current thread is not the event thread. The bpstat of the event thread is left handling. Installing the cleanup in fetch_inferior_event _after_ make_cleanup_restore_current_thread would fix this. All-stop doesn't switch to the event thread until within handle_inferior_event, so that still leaves a window where an exception would run the cleanup with the old thread selected, but I'm not seeing a scenario where that'd be a problem, and plus, the current code (clearing bpstat actions from throw_exception) gets that wrong too. So the patch looks okay to me with that change. Thanks. -- Pedro Alves