From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23174 invoked by alias); 30 Sep 2015 12:17: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 23162 invoked by uid 89); 30 Sep 2015 12:17:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 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; Wed, 30 Sep 2015 12:16:58 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id B276DC0A15E1 for ; Wed, 30 Sep 2015 12:16:57 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8UCGug1027696; Wed, 30 Sep 2015 08:16:56 -0400 Message-ID: <560BD2B7.9010706@redhat.com> Date: Wed, 30 Sep 2015 12:17: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> <55DC5B13.6030501@redhat.com> <20150917185751.22fe2937@pinnacle.lan> In-Reply-To: <20150917185751.22fe2937@pinnacle.lan> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-09/txt/msg00650.txt.bz2 Hi Kevin, Sorry for the delay. On 09/18/2015 02:57 AM, Kevin Buettner wrote: > If I place a breakpoint on the Loop 5 line, it ends up on on 0x4005cc, > which is a nop. It appears that gcc has rewritten this code so that > the the comparison comes first, which exactly the opposite of what > happens when one writes a while loop. > > The presence of the nop means that things work as expected when we > place a breakpoint on that line. It will stop just once on the > statement instead of once for each time through the loop. > > This was an interesting exercise, but it doesn't really answer the > question of what would happen if gcc would instead place an > actual branch at the beginning. I would expect my patch > to cause the breakpoint to be placed within the loop instead > of at the beginning of the statement. I don't think this is > desirable, but I can't think of a way (within the current work) to > stop it from happening. Yeah, me neither. This is always going to be brittle. :-/ Really the best would be DWARF is_stmt. > > Here's the new patch: > > 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 pratical effect of this is that a breakpoint placed on a while "practical" > 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 | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 76 insertions(+) > > diff --git a/gdb/linespec.c b/gdb/linespec.c > index 4c29c12..4b5cde9 100644 > --- a/gdb/linespec.c > +++ b/gdb/linespec.c > @@ -1808,6 +1808,29 @@ 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, int nelts, struct symtab_and_line *sals) > +{ > + int i; > + > + for (i = 0; i < nelts; i++) > + { > + struct symtab_and_line sal; > + > + 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 > @@ -1933,6 +1956,59 @@ 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 pratical effect of this is that a breakpoint placed "practical". > + 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 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? > + { > + intermediate_results.sals[i].pc = branch_dest; > + } Unnecessary braces. > > if (self->funfirstline) > skip_prologue_sal (&intermediate_results.sals[i]); > Thanks, Pedro Alves