[3/8] 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
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_address, 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 target is the same line.
---
 gdb/linespec.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
  

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