Re: [PATCH 3/8] Break at each iteration for breakpoints placed on a while statement

[Pedro Alves <palves at redhat dot com>]

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