From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 109472 invoked by alias); 25 Aug 2015 12:10:00 -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 109456 invoked by uid 89); 25 Aug 2015 12:09:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_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 (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 25 Aug 2015 12:09:58 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 2E2618CF5E for ; Tue, 25 Aug 2015 12:09:57 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7PC9tmG028497; Tue, 25 Aug 2015 08:09:56 -0400 Message-ID: <55DC5B13.6030501@redhat.com> Date: Tue, 25 Aug 2015 12:10:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Kevin Buettner , gdb-patches@sourceware.org Subject: Re: [PATCH 3/8] Break at each iteration for breakpoints placed on a while statement References: <20150818235334.1afb0c85@pinnacle.lan> <20150819000334.62f7a867@pinnacle.lan> In-Reply-To: <20150819000334.62f7a867@pinnacle.lan> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-08/txt/msg00719.txt.bz2 On 08/19/2015 08:03 AM, Kevin Buettner wrote: > @@ -1933,6 +1956,18 @@ create_sals_line_offset (struct linespec_state *self, > struct symbol *sym = (blocks[i] > ? block_containing_function (blocks[i]) > : NULL); > + CORE_ADDR branch_addr = gdbarch_unconditional_branch_address > + (get_current_arch (), intermediate_results.sals[i].pc); It'd be nice to have a comment here explaining why we do this. You have it in the commit log, but it'd be useful going forward to have it in the sources. Nit, this is about the branch _destination_ not the branch instruction's address, right? I'd suggest s/gdbarch_unconditional_branch_address/gdbarch_unconditional_branch_destination/ Also, there should be a better gdbarch to use than get_current_arch(). E.g., get_objfile_arch (SYMTAB_OBJFILE (sals[i].symtab)), the sal's pspace's gdbarch, etc. > + > + /* Only use branch if it's in the same block and is also And here "use branch destination". > + within one of the sals from the initial list. */ > + if (branch_addr != 0 && blocks[i]->startaddr <= branch_addr > + && branch_addr < blocks[i]->endaddr > + && addr_in_sals (branch_addr, intermediate_results.nelts, > + intermediate_results.sals)) > + { > + intermediate_results.sals[i].pc = branch_addr; > + } Also, we really shouldn't be adding code that assumes 0 to mean invalid address, as on some ports, it is a valid address. You could e.g., change gdbarch_unconditional_branch_address to return an int and take an output CORE_ADDR * parameter, and/or make it an M gdbarch method, and check gdbarch_unconditional_branch_address_p before use. I wonder whether we have to worry about the case of a goto jumping to the same line? Like: goto label; foo (); label: bar (); Not the most usual or beautiful code to write, though I'd guess code generators could output things like that. Writing it in several lines but behind a #define might be another way to get everything in the same line. Thanks, Pedro Alves