From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32427 invoked by alias); 7 Dec 2011 04:09:19 -0000 Received: (qmail 32138 invoked by uid 22791); 7 Dec 2011 04:09:18 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 07 Dec 2011 04:09:02 +0000 Received: from nat-jpt.mentorg.com ([192.94.33.2] helo=PR1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1RY8oX-0006jS-QG from Yao_Qi@mentor.com ; Tue, 06 Dec 2011 20:09:02 -0800 Received: from [127.0.0.1] ([172.16.63.104]) by PR1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Wed, 7 Dec 2011 13:08:59 +0900 Message-ID: <4EDEE6D8.8010301@codesourcery.com> Date: Wed, 07 Dec 2011 07:55:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20110930 Thunderbird/7.0.1 MIME-Version: 1.0 To: Hui Zhu CC: gdb-patches ml Subject: Re: [PATCH] Fix tracepoint tstart again get gdb_assert References: In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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-12/txt/msg00219.txt.bz2 On 12/06/2011 11:56 PM, Hui Zhu wrote: > Hi, > > I use a gdb tstart again in a section an got: > > (gdb) tstart > (gdb) tstop > (gdb) tstart > ../../src/gdb/tracepoint.c:1770: internal-error: start_tracing: > Assertion `!loc->inserted' failed. > A problem internal to GDB has been detected, > further debugging may prove unreliable. > Quit this debugging session? (y or n) > Thanks for this patch! It is related to my previous commit [patch 4/8] Download tracepoint locations and track its status http://sourceware.org/ml/gdb-patches/2011-11/msg00337.html > The reason is: > start_tracing: > for (loc = b->loc; loc; loc = loc->next) > { > /* Since tracepoint locations are never duplicated, `inserted' > flag should be zero. */ > gdb_assert (!loc->inserted); > > target_download_tracepoint (loc); > > loc->inserted = 1; > } > > But in stop_tracing and trace_status_command don't have code to set > inserted back to 0. > Right, I agree that loc->inserted should be cleared on stop_tracing. However, I don't know why we have to clear loc->inserted in trace_status_command. A few comments below. > So I make a patch for it. > > Thanks, > Hui > > 2011-12-06 Hui Zhu > > * tracepoint.c (stop_tracing): Set loc->inserted if need. > (trace_status_command): Ditto. > --- > tracepoint.c | 41 ++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 38 insertions(+), 3 deletions(-) > > --- a/tracepoint.c > +++ b/tracepoint.c > @@ -1847,6 +1847,9 @@ void > stop_tracing (char *note) > { > int ret; > + VEC(breakpoint_p) *tp_vec = NULL; > + int ix; > + struct breakpoint *b; > > target_trace_stop (); > > @@ -1859,6 +1862,22 @@ stop_tracing (char *note) > > /* Should change in response to reply? */ > current_trace_status ()->running = 0; > + > + tp_vec = all_tracepoints (); > + for (ix = 0; VEC_iterate (breakpoint_p, tp_vec, ix, b); ix++) > + { > + struct tracepoint *t = (struct tracepoint *) b; > + struct bp_location *loc; > + > + if ((b->type == bp_fast_tracepoint > + ? !may_insert_fast_tracepoints > + : !may_insert_tracepoints)) > + continue; > + I don't think this check above is needed here. In start_tracing, we need may_insert_fast_tracepoints and may_insert_tracepoints to download desired type of tracepoint. However, these two variables may be changed in a trace experiment, so we don't have to check them and clear loc->inserted unconditionally. > + for (loc = b->loc; loc; loc = loc->next) > + loc->inserted = 0; > + } > + VEC_free (breakpoint_p, tp_vec); > } > > /* tstatus command */ > @@ -1868,7 +1887,7 @@ trace_status_command (char *args, int fr > struct trace_status *ts = current_trace_status (); > int status, ix; > VEC(breakpoint_p) *tp_vec = NULL; > - struct breakpoint *t; > + struct breakpoint *b; > > status = target_get_trace_status (ts); > > @@ -2013,8 +2032,24 @@ trace_status_command (char *args, int fr > /* Now report any per-tracepoint status available. */ > tp_vec = all_tracepoints (); > > - for (ix = 0; VEC_iterate (breakpoint_p, tp_vec, ix, t); ix++) > - target_get_tracepoint_status (t, NULL); > + for (ix = 0; VEC_iterate (breakpoint_p, tp_vec, ix, b); ix++) > + { > + struct tracepoint *t = (struct tracepoint *) b; > + struct bp_location *loc; > + > + target_get_tracepoint_status (b, NULL); > + > + if (!ts->running) > + { > + if ((b->type == bp_fast_tracepoint > + ? !may_insert_fast_tracepoints > + : !may_insert_tracepoints)) > + continue; > + > + for (loc = b->loc; loc; loc = loc->next) > + loc->inserted = 0; > + } > + } > > VEC_free (breakpoint_p, tp_vec); > } As I said at the beginning, I don't understand why we have to clear loc->inserted in trace_status_command when ts->running is 0. -- Yao (齐尧)