This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/8] Break at each iteration for breakpoints placed on a while statement
- From: Pedro Alves <palves at redhat dot com>
- To: Kevin Buettner <kevinb at redhat dot com>, gdb-patches at sourceware dot org
- Date: Wed, 30 Sep 2015 13:16:55 +0100
- Subject: Re: [PATCH 3/8] Break at each iteration for breakpoints placed on a while statement
- Authentication-results: sourceware.org; auth=none
- References: <20150818235334 dot 1afb0c85 at pinnacle dot lan> <20150819000334 dot 62f7a867 at pinnacle dot lan> <55DC5B13 dot 6030501 at redhat dot com> <20150917185751 dot 22fe2937 at pinnacle dot lan>
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 <loop_test+14>: jmp 0x4005af <loop_test+31>
> 0x00000000004005a0 <loop_test+16>: mov 0x200a8a(%rip),%eax # 0x601030 <v>
> 0x00000000004005a6 <loop_test+22>: add $0x1,%eax
> 0x00000000004005a9 <loop_test+25>: mov %eax,0x200a81(%rip) # 0x601030 <v>
> 0x00000000004005af <loop_test+31>: mov 0x200a7b(%rip),%eax # 0x601030 <v>
> 0x00000000004005b5 <loop_test+37>: cmp $0x2,%eax
> 0x00000000004005b8 <loop_test+40>: jle 0x4005a0 <loop_test+16>
>
> 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 <loop_test+14>: jmp 0x4005af <loop_test+31>
> + 0x4005a0 <loop_test+16>: mov 0x200a8a(%rip),%eax
> + 0x4005a6 <loop_test+22>: add $0x1,%eax
> + 0x4005a9 <loop_test+25>: mov %eax,0x200a81(%rip)
> + 0x4005af <loop_test+31>: mov 0x200a7b(%rip),%eax
> + 0x4005b5 <loop_test+37>: cmp $0x2,%eax
> + 0x4005b8 <loop_test+40>: jle 0x4005a0 <loop_test+16>
> +
> + 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