From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15772 invoked by alias); 26 Feb 2013 19:53:16 -0000 Received: (qmail 15762 invoked by uid 22791); 26 Feb 2013 19:53:15 -0000 X-SWARE-Spam-Status: No, hits=-8.0 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 26 Feb 2013 19:53:03 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1QJr1vo020243 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 26 Feb 2013 14:53:01 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r1QJqxkZ019943; Tue, 26 Feb 2013 14:53:00 -0500 Message-ID: <512D129B.6040504@redhat.com> Date: Tue, 26 Feb 2013 19:53:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH 2/2] Find the next matched trace file in 'tfile target'. References: <1361761061-9625-1-git-send-email-yao@codesourcery.com> <1361761061-9625-2-git-send-email-yao@codesourcery.com> In-Reply-To: <1361761061-9625-2-git-send-email-yao@codesourcery.com> Content-Type: multipart/mixed; boundary="------------050709020108080904080805" 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: 2013-02/txt/msg00663.txt.bz2 This is a multi-part message in MIME format. --------------050709020108080904080805 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-length: 2557 On 02/25/2013 02:57 AM, Yao Qi wrote: > The previous patch exposes a bug in tfile target when finding a trace > frame. Every time, GDB will scan tfile from the starting offset and > initialize trace frame number to zero. When TYPE is not tfind_number, > it means GDB wants to find the *next* matched trace frame of current > one. So we need to check the tfile trace frame number iterator is > greater than the current trace frame number (which means *next*). > This is mainly what this patch does. Thanks. Good catch. > diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c > index ca104aa..f7a3650 100644 > --- a/gdb/tracepoint.c > +++ b/gdb/tracepoint.c > @@ -4324,22 +4324,34 @@ tfile_trace_find (enum trace_find_type type, int num, > break; > case tfind_pc: > tfaddr = tfile_get_traceframe_address (tframe_offset); > - if (tfaddr == addr1) > + if (tfaddr == addr1 > + /* Looks for the next trace frame if matched. */ > + && (tfnum > traceframe_number > + || (tfnum == traceframe_number && tfnum == 0))) > found = 1; > break; > case tfind_tp: > tp = get_tracepoint (num); > - if (tp && tpnum == tp->number_on_target) > + if (tp && tpnum == tp->number_on_target > + /* Looks for the next trace frame if matched. */ > + && (tfnum > traceframe_number > + || (tfnum == traceframe_number && tfnum == 0))) > found = 1; > break; > case tfind_range: > tfaddr = tfile_get_traceframe_address (tframe_offset); > - if (addr1 <= tfaddr && tfaddr <= addr2) > + if (addr1 <= tfaddr && tfaddr <= addr2 > + /* Looks for the next trace frame if matched. */ > + && (tfnum > traceframe_number > + || (tfnum == traceframe_number && tfnum == 0))) > found = 1; > break; > case tfind_outside: > tfaddr = tfile_get_traceframe_address (tframe_offset); > - if (!(addr1 <= tfaddr && tfaddr <= addr2)) > + if (!(addr1 <= tfaddr && tfaddr <= addr2) > + /* Looks for the next trace frame if matched. */ > + && (tfnum > traceframe_number > + || (tfnum == traceframe_number && tfnum == 0))) I'm confused on why this bit of the predicate (tfnum == traceframe_number && tfnum == 0) is necessary. traceframe_number is -1 when not looking at a traceframe yet, so "tfnum > traceframe_number" should be sufficient, no? I find it clearer to move the frame skipping a bit higher up, even before the specific tfind tp/range/etc. matching. Doing it this way also avoids unnecessary read/lseek system calls done by tfile_get_traceframe_address. WDYT? --------------050709020108080904080805 Content-Type: text/x-patch; name="tfile_find.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="tfile_find.diff" Content-length: 3039 commit f837913c7a153a0e40a15f4a283deea201cbcbe7 Author: Pedro Alves Date: Tue Feb 26 19:43:21 2013 +0000 Find the next matched trace file in 'tfile target'. The previous patch exposes a bug in the tfile target when finding a trace frame. Every time, GDB will scan tfile from the starting offset and look for a matching trace frame starting from frame zero. But, except when TYPE is tfind_number, GDB wants to find the *next* matched trace frame after the current selected frame. So we need to check the tfile trace frame number iterator is greater than the current trace frame number (which means *next*), meaning skip all frames up to and including the current. Regression tested on x86_64 Fedora 17. gdb/ 2013-02-25 Yao Qi Pedro Alves * tracepoint.c (tfile_trace_find): For tfind pc/tp/range/outside, look for the next trace frame instead of always starting from frame 0. diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c index ca104aa..9a80aa3 100644 --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -4316,35 +4316,46 @@ tfile_trace_find (enum trace_find_type type, int num, ((gdb_byte *) &data_size, 4, gdbarch_byte_order (target_gdbarch ())); offset += 4; - switch (type) + + if (type == tfind_number) { - case tfind_number: + /* Looking for a specific trace frame. */ if (tfnum == num) found = 1; - break; - case tfind_pc: - tfaddr = tfile_get_traceframe_address (tframe_offset); - if (tfaddr == addr1) - found = 1; - break; - case tfind_tp: - tp = get_tracepoint (num); - if (tp && tpnum == tp->number_on_target) - found = 1; - break; - case tfind_range: - tfaddr = tfile_get_traceframe_address (tframe_offset); - if (addr1 <= tfaddr && tfaddr <= addr2) - found = 1; - break; - case tfind_outside: - tfaddr = tfile_get_traceframe_address (tframe_offset); - if (!(addr1 <= tfaddr && tfaddr <= addr2)) - found = 1; - break; - default: - internal_error (__FILE__, __LINE__, _("unknown tfind type")); } + else + { + /* Start from the _next_ trace frame. */ + if (tfnum > traceframe_number) + { + switch (type) + { + case tfind_pc: + tfaddr = tfile_get_traceframe_address (tframe_offset); + if (tfaddr == addr1) + found = 1; + break; + case tfind_tp: + tp = get_tracepoint (num); + if (tp && tpnum == tp->number_on_target) + found = 1; + break; + case tfind_range: + tfaddr = tfile_get_traceframe_address (tframe_offset); + if (addr1 <= tfaddr && tfaddr <= addr2) + found = 1; + break; + case tfind_outside: + tfaddr = tfile_get_traceframe_address (tframe_offset); + if (!(addr1 <= tfaddr && tfaddr <= addr2)) + found = 1; + break; + default: + internal_error (__FILE__, __LINE__, _("unknown tfind type")); + } + } + } + if (found) { if (tpp) --------------050709020108080904080805--