From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28660 invoked by alias); 14 May 2013 18:31:36 -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 28645 invoked by uid 89); 14 May 2013 18:31:36 -0000 X-Spam-SWARE-Status: No, score=-8.0 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS,TW_XS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 14 May 2013 18:31:35 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4EIVXMH032315 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 14 May 2013 14:31:33 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r4EIVVWb026872; Tue, 14 May 2013 14:31:32 -0400 Message-ID: <51928303.3050407@redhat.com> Date: Tue, 14 May 2013 18:31:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH 4/7] range stepping: gdb References: <1363006291-13334-1-git-send-email-yao@codesourcery.com> <1363006291-13334-5-git-send-email-yao@codesourcery.com> In-Reply-To: <1363006291-13334-5-git-send-email-yao@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg00479.txt.bz2 On 03/11/2013 12:51 PM, Yao Qi wrote: > @@ -4689,7 +4694,36 @@ append_resumption (char *p, char *endp, > if (step && siggnal != GDB_SIGNAL_0) > p += xsnprintf (p, endp - p, ";S%02x", siggnal); > else if (step) > - p += xsnprintf (p, endp - p, ";s"); > + { > + struct thread_info *tp = NULL; > + CORE_ADDR pc; > + > + gdb_assert (!ptid_equal (ptid, minus_one_ptid)); I think you'll trip on this in case the target supports vCont but not actually listing thread ids (magic_null_ptid). > + > + if (ptid_is_pid (ptid)) > + tp = find_thread_ptid (inferior_ptid); > + else > + tp = find_thread_ptid (ptid); > + gdb_assert (tp != NULL); > + > + pc = regcache_read_pc (get_thread_regcache (ptid)); > + if (rs->support_vCont.r /* Target supports step range. */ > + /* Can't do range stepping for all threads of a process > + 'pPID.-1'. */ > + && !(remote_multi_process_p (rs) && ptid_is_pid (ptid)) > + /* Not step over a breakpoint. */ > + && !tp->control.trap_expected > + /* We have a range to single step. */ > + && THREAD_WITHIN_SINGLE_STEP_RANGE (tp, pc)) This is problematic. It's better to _not_ have target itself decide when to range step or to single step, and peeking at "infrun-owned" variables. For example, with software watchpoints, GDB needs to have control of single-steps, in order to evaluate the watchpoints at after each instruction is executed, so trap_expected isn't enough. (My v3 adds a test for that, that v2 fails.) I dislike the design of using PC checks here too :-/. That seems fragile, and potentially inefficient (considering GDB ever sending more than one range action per packet, that might end up fetching registers for threads unnecessarily). IMO, it's better to have the core run control (infrun.c/infcmd.c) decide when to range step or single-step. > + { > + p += xsnprintf (p, endp - p, ";r"); > + p += hexnumstr (p, tp->control.step_range_start); > + p += xsnprintf (p, endp - p, ","); > + p += hexnumstr (p, tp->control.step_range_end); > + } > + else > + p += xsnprintf (p, endp - p, ";s"); > + } > else if (siggnal != GDB_SIGNAL_0) > p += xsnprintf (p, endp - p, ";C%02x", siggnal); > else > Thanks, -- Pedro Alves