From patchwork Thu Oct 1 01:13:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Buettner X-Patchwork-Id: 8898 Received: (qmail 75729 invoked by alias); 1 Oct 2015 01:13:20 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 75688 invoked by uid 89); 1 Oct 2015 01:13:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, T_RP_MATCHES_RCVD 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; Thu, 01 Oct 2015 01:13:18 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 897C580B27 for ; Thu, 1 Oct 2015 01:13:16 +0000 (UTC) Received: from pinnacle.lan ([10.3.113.5]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t911DFXt008169 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA256 bits=256 verify=NO) for ; Wed, 30 Sep 2015 21:13:16 -0400 Date: Wed, 30 Sep 2015 18:13:14 -0700 From: Kevin Buettner To: gdb-patches@sourceware.org Subject: Re: [PATCH 3/8] Break at each iteration for breakpoints placed on a while statement Message-ID: <20150930181314.0552d954@pinnacle.lan> In-Reply-To: <560BD2B7.9010706@redhat.com> References: <20150818235334.1afb0c85@pinnacle.lan> <20150819000334.62f7a867@pinnacle.lan> <55DC5B13.6030501@redhat.com> <20150917185751.22fe2937@pinnacle.lan> <560BD2B7.9010706@redhat.com> MIME-Version: 1.0 X-IsSubscribed: yes Hi Pedro, Thanks again for the review. On Wed, 30 Sep 2015 13:16:55 +0100 Pedro Alves wrote: > > The pratical effect of this is that a breakpoint placed on a while > > "practical" > > + The pratical effect of this is that a breakpoint placed > > "practical". Both fixed in new patch. > > + /* Only use branch destination if it's in the same block and > > + is also within one of the sals from the initial list. */ > > + if (is_branch && blocks[i]->startaddr <= branch_dest > > + && branch_dest < blocks[i]->endaddr > > + && addr_in_sals (branch_dest, intermediate_results.nelts, > > + intermediate_results.sals)) > > I believe intermediate_results can contain sals of different > pspaces / inferiors. Shouldn't we take that into account somehow? Good catch. I had not considered this. In the new patch, I've added a `pspace' paramater to addr_in_sals() and have added the necessary code to that function. The end result is that only sals in the program space of the sal that we might potentially fix up will be considered. (I hope that makes sense.) > > > + { > > + intermediate_results.sals[i].pc = branch_dest; > > + } > > Unnecessary braces. I took them out. Here's the revised patch: commit 6018c9ae5ea2bc5fcf723ce9a9beeb315d3fb3c0 Author: Kevin Buettner Date: Tue Aug 18 22:06:40 2015 -0700 Break at each iteration for breakpoints placed on a while statement. This patch changes create_sals_line_offset() in linespec.c so that, for a given SAL, if that SAL's address (pc) refers to an unconditional branch instruction whose branch target also refers to the same SAL, then the branch target is used for the SAL instead. The practical effect of this is that a breakpoint placed on a while loop will break at the evaluation of the condition instead of at the unconditional branch which transfers control to the starting address for the evaluation of the condition. Consider the following code snippet (which is taken from one of the new tests for this patch set): 9 while (v < 3) /* Loop 1 condition */ 10 { 11 v++; /* Loop 1 increment */ 12 } This is compiled as the following x86_64 code: 0x000000000040059e : jmp 0x4005af 0x00000000004005a0 : mov 0x200a8a(%rip),%eax # 0x601030 0x00000000004005a6 : add $0x1,%eax 0x00000000004005a9 : mov %eax,0x200a81(%rip) # 0x601030 0x00000000004005af : mov 0x200a7b(%rip),%eax # 0x601030 0x00000000004005b5 : cmp $0x2,%eax 0x00000000004005b8 : jle 0x4005a0 If a breakpoint is placed on line 9, which begins at loop_test+14, this change/patch causes the breakpoint to be placed on loop_test+31, which is the starting address for the evaluation of the condition. In order for this to work, an architecture specific method, unconditional_branch_destination, was introduced in an earlier patch in the set. I've implemented this method for x86_64, i386, arm, thumb, powerpc, rx, and rl78. I've tested on each of these architectures and see no regressions. gdb/ChangeLog: * linespec.c (addr_in_sals): New function. (create_sals_line_offset): Adjust SAL whose pc refers to an unconditional branch whose destination is the same line. --- gdb/linespec.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/gdb/linespec.c b/gdb/linespec.c index b2233b9..1ba0e67 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -1809,6 +1809,37 @@ canonicalize_linespec (struct linespec_state *state, const linespec_p ls) } } +/* Return 1 if one of the SALS between 0 and NELTS contains ADDR. + Return 0 otherwise. */ + +static int +addr_in_sals (CORE_ADDR addr, struct program_space *pspace, int nelts, + struct symtab_and_line *sals) +{ + int i; + + /* Make sure we're in the correct program space for a potential + call to find_pc_sect_line, below. */ + set_current_program_space (pspace); + + for (i = 0; i < nelts; i++) + { + struct symtab_and_line sal; + + if (pspace != sals[i].pspace) + continue; + + if (sals[i].end == 0) + sal = find_pc_sect_line (sals[i].pc, sals[i].section, 0); + else + sal = sals[i]; + + if (sal.pc <= addr && addr < sal.end) + return 1; + } + return 0; +} + /* Given a line offset in LS, construct the relevant SALs. */ static struct symtabs_and_lines @@ -1934,6 +1965,60 @@ create_sals_line_offset (struct linespec_state *self, struct symbol *sym = (blocks[i] ? block_containing_function (blocks[i]) : NULL); + int is_branch; + CORE_ADDR branch_dest; + + /* This next bit of code examines the instruction at the + SAL's address (pc). If the instruction is an + unconditional branch whose branch destination also + refers to the same SAL, then the branch destination is + used for the SAL's pc value instead. + + The practical effect of this is that a breakpoint placed + on a while loop will break at the evaluation of the + condition instead of at an unconditional branch which + transfers control to the starting address for the + evaluation of the condition. + + Consider the following code snippet (which is taken + from the test case for gdb.base/loop-break.exp.) + + while (v < 3) + { + v++; + } + + On x86_64, this might be compiled as follows: + + 0x40059e : jmp 0x4005af + 0x4005a0 : mov 0x200a8a(%rip),%eax + 0x4005a6 : add $0x1,%eax + 0x4005a9 : mov %eax,0x200a81(%rip) + 0x4005af : mov 0x200a7b(%rip),%eax + 0x4005b5 : cmp $0x2,%eax + 0x4005b8 : jle 0x4005a0 + + If a breakpoint is placed on the while statement line + which begins at loop_test+14, the following code causes + the breakpoint to be (instead) placed on loop_test+31, which + is the starting address for the evaluation of the condition. */ + + is_branch = gdbarch_unconditional_branch_destination + (get_objfile_arch + (SYMTAB_OBJFILE (intermediate_results.sals[i].symtab)), + intermediate_results.sals[i].pc, + &branch_dest); + + /* Only use branch destination if it's in the same block and + is also within one of the sals (within the same program + space) from the initial list. */ + if (is_branch && blocks[i]->startaddr <= branch_dest + && branch_dest < blocks[i]->endaddr + && addr_in_sals (branch_dest, + intermediate_results.sals[i].pspace, + intermediate_results.nelts, + intermediate_results.sals)) + intermediate_results.sals[i].pc = branch_dest; if (self->funfirstline) skip_prologue_sal (&intermediate_results.sals[i]);