From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26528 invoked by alias); 29 Jul 2013 23:33:27 -0000 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 Received: (qmail 26517 invoked by uid 89); 29 Jul 2013 23:33:27 -0000 X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL,BAYES_05,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RDNS_NONE,SPF_PASS autolearn=ham version=3.3.1 Received: from Unknown (HELO mail-vb0-f73.google.com) (209.85.212.73) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 29 Jul 2013 23:33:26 +0000 Received: by mail-vb0-f73.google.com with SMTP id e12so186036vbg.2 for ; Mon, 29 Jul 2013 16:33:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:mime-version:content-type:content-transfer-encoding:message-id :date:to:cc:subject:in-reply-to:references:x-mailer :x-gm-message-state; bh=DtwBKvyUD8PqG+0js8jdQ32Vr1Q0uznMOhJ6/xEH+yI=; b=N7MZBy6Zjjha/mA5Pvsbd10JJ401nLd797qbEz1ZuY1Q169z8SWKqPxo9skF6i1Pl7 9mDwTLxCffEBbtdM1md1XdimaMcZKBkBdI3P/eDGdrDCs7qNBF7bDSaZFLG7QoLN7U2c 50WeEta1mi2eIlbJsQeXrspgHa+l0AFWgPgGPm2L3fgGy2PHGtgZufON+ssJQhqRt1iE hVXO4pp7SiLmX/YqRELnJrRS+y3MCjcwCUSRtlq+Br40j6bzJbxsJMdCVcUtUpPS1vdm yA4o0UnNi9TJSd6JqBt78PJ1nLIDqHEImrxFBsNcK2xuQCc9vZJ/15cVmFsiaajaRqUc xzlg== X-Received: by 10.236.54.135 with SMTP id i7mr32624900yhc.33.1375140798016; Mon, 29 Jul 2013 16:33:18 -0700 (PDT) Received: from corp2gmr1-1.hot.corp.google.com (corp2gmr1-1.hot.corp.google.com [172.24.189.92]) by gmr-mx.google.com with ESMTPS id v70si4893992yhv.3.2013.07.29.16.33.18 for (version=TLSv1.1 cipher=AES128-SHA bits=128/128); Mon, 29 Jul 2013 16:33:18 -0700 (PDT) Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.17.128.44]) by corp2gmr1-1.hot.corp.google.com (Postfix) with ESMTP id 7A44731C1EE; Mon, 29 Jul 2013 16:33:17 -0700 (PDT) From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <20982.64444.939564.557857@ruffy.mtv.corp.google.com> Date: Mon, 29 Jul 2013 23:33:00 -0000 To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 7/8] fix py-finish-breakpoint.exp with always-async In-Reply-To: <1375116324-32092-8-git-send-email-tromey@redhat.com> References: <1375116324-32092-1-git-send-email-tromey@redhat.com> <1375116324-32092-8-git-send-email-tromey@redhat.com> X-Gm-Message-State: ALoCoQnQaBXwxRqoAbkTY6Q+bCXaMFpy/IVXW5X1qetXHOvRaDSdbuhwl0TOwjL03/4TedorFzivNfuG3kB32v4e6M/IsUIc0Zi6MT3LMINy8eIUAbkGildQKvxxQ+zd3oEeVc0CPiMASp4wrj2stoLEtgVIDDfS6W/LVwhEh15jxkf7pMaOy6gkC0+1tJZonRBQ1wK5xLScIzL55zAG7oQdHjDB9HXSSg== X-SW-Source: 2013-07/txt/msg00750.txt.bz2 Tom Tromey writes: > With target async enabled, py-finish-breakpoint.exp will trigger an > assertion failure. > > The failure occurs because execute_command re-enters the event loop in > some circumstances, and in this case resets the sync_execution flag. > Then later gdb reaches this assertion in normal_stop: > > gdb_assert (sync_execution || !target_can_async_p ()); > > execute_command has a comment explaining why it dispatches events: > > /* If the interpreter is in sync mode (we're running a user > command's list, running command hooks or similars), and we > just ran a synchronous command that started the target, wait > for that command to end. */ > > However, the code did not follow this comment -- it didn't check to > see if the command started the target. > > This patch fixes the problem by noting whether the target was > executing in sync_execution mode before running the command, and then > augmenting the condition to test this as well. > > Built and regtested on x86-64 Fedora 18. > > * top.c (execute_command): Only dispatch events if command > started target. > --- > gdb/top.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/gdb/top.c b/gdb/top.c > index 467e6a2..6af0fad 100644 > --- a/gdb/top.c > +++ b/gdb/top.c > @@ -426,6 +426,8 @@ execute_command (char *p, int from_tty) > { > const char *cmd = p; > char *arg; > + int was_executing = sync_execution && target_has_execution; > + > line = p; > > /* If trace-commands is set then this will print this command. */ > @@ -481,7 +483,8 @@ execute_command (char *p, int from_tty) > command's list, running command hooks or similars), and we > just ran a synchronous command that started the target, wait > for that command to end. */ > - if (!interpreter_async && sync_execution) > + if (!interpreter_async && !was_executing > + && sync_execution && target_has_execution) > { > while (gdb_do_one_event () >= 0) > if (!sync_execution) Hi. The patch is a bit confusing. Assignment of was_execution tests for sync_execution, and later we test sync_execution && !was_executing. It may be correct to do it that way (I'm not sure ATM), but the phrase "was_executing" doesn't really convey any notion of what "sync_execution" means. IOW, it'd be clearer if the patch did: int was_executing = target_has_execution; and then augment the patch with whatever else is necessary to make it correct. [Assuming I understand correctly what the patch is trying to do.] ---- Also, is target_has_execution what you want here? If I'm stopped at a breakpoint target_has_execution still returns non-zero. E.g. do we want that while loop for a "next" command? [where the target "has execution" before and after the command] If that's *not* the case then that code needs a lot more commenting. :-) Maybe I'm missing something of course.