From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4992 invoked by alias); 18 Dec 2013 14:09:19 -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 4981 invoked by uid 89); 18 Dec 2013 14:09:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 18 Dec 2013 14:09:15 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rBIE9Dlj017278 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 18 Dec 2013 09:09:13 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id rBIE9BWu015071; Wed, 18 Dec 2013 09:09:12 -0500 Message-ID: <52B1AC87.8070409@redhat.com> Date: Wed, 18 Dec 2013 14:09:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH 0/4 V7] MI notification on trace started/stopped References: <1386856184-16519-1-git-send-email-yao@codesourcery.com> In-Reply-To: <1386856184-16519-1-git-send-email-yao@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-12/txt/msg00699.txt.bz2 On 12/12/2013 01:49 PM, Yao Qi wrote: > Hi, > I examined all previous versions of this series and corresponding > reviews. I find that V2 is the closet one to acceptance. After > v2, I pursued "notification annex" and supported query on it, > which is a blind alley. (I don't recall why I choose this way then). > > I update my patches to follow V2 and address comments I got during > the review. The major comment raised by Pedro is that "GDB and > the remote negotiate the set of supported notifications". Also, I still think ordering issues between trace start/stop and stop events needs to be considered. Let me expand. With trace events sent on a separate channel, we can get things like this, with a single-threaded program being traced: #1 - tstart, resume program #2 - trace stops because buffer full #3 - gdbserver queues trace stop notification. #4 - breakpoint at "next_session" is hit. #5 - gdb happens to process breakpoint notification first. #6 - The "next_session" breakpoint has a breakpoint command that does: "tstop; tsave; tstart; continue" (program iterates, and another collection sample begins) #7 - GDB emits MI =trace-stopped then =trace-started in response to tstop/tstart above. #8 - gdb processes the pending trace stop notification, emits MI =trace-stopped to frontend. #9 - The frontend is confused, thinking the trace session is stopped, when it is in fact running. Now, even if we sent trace stops down the %Stop notification, that wouldn't happen. GDB would always process the trace stop notification before the breakpoint. But, considering multi-threaded programs, a different thread can stop the trace immediately after the breakpoint at next_session hits and triggers a %Stop notification, and so sending trace notifications using %Stop wouldn't fix the frontend confusion in this case, as the stop would still be processed before the trace stop. What I'm just now thinking would fix it would be if the remote _also_ triggered trace _start_ notifications in addition to trace stops: #1 - tstart, resume program #2 - trace stops because buffer full #3 - gdbserver queues trace stop notification. #4 - breakpoint at "next_session" is hit. #5 - gdb happens to process breakpoint notification first. #6 - The "next_session" breakpoint has a breakpoint command that does: "tstop; tsave; tstart; continue" (program iterates, and another collection sample begins) #7 - GDB emits MI =trace-stopped then =trace-started in response to tstop/tstart above. #8 - gdb processes the pending trace stop notification, emits MI =trace-stopped to frontend. #9 - gdb processes a trace start notification, emits MI =trace-start to frontend. #10 - frontend displays the trace session as running. Now, looking at this, we see MI may get this trace event sequence: ... =trace-stopped (caused by tstop in bp command) =trace-start (caused by tstart in bp command) =trace-stopped (triggered by target) =trace-start (triggered by target) even though the tracing only stopped and started once. To fully fix that, MI trace events triggered by GDB actions could be distinguishable from MI trace events triggered by the target (e.g, an attribute), and the frontend could only look at target-triggered events. I'm not sure that's really necessary, and it can always be added later, but it may be nice to have. Another way to fix the ordering issue I just thought would be to have a sequence number associated with each trace session, and send those along trace start/stop notifications, so that a delayed =trace-stopped generated from the target would be older than the current trace session, so it would be ignored. Not sure how I feel about that. Not sure how I feel about the other solution either. :-) Seems to me something needs to be done though. -- Pedro Alves