Re: [PATCH 2/8] Add new gdbarch method, unconditional_branch_address

[Kevin Buettner <kevinb at redhat dot com>]

On Tue, 22 Sep 2015 17:09:17 +0100
Yao Qi <qiyaoltc@gmail.com> wrote:

> Kevin Buettner <kevinb@redhat.com> writes:
> 
> > Add new gdbarch method, unconditional_branch_address.
> >
> > gdb/ChangeLog:
> >
> >     	* gdbarch.sh (unconditional_branch_address): New gdbarch method.
> >     	* gdbarch.h, gdbarch.c: Regenerate.
> >     	* arch-utils.h (default_unconditional_branch_address): Declare.
> >     	* arch-utils.c (default_unconditional_branch_address): New function.
> 
> Did you consider using existing gdbarch method
> adjust_breakpoint_address when you wrote the patch?  Can we use
> adjust_breakpoint_address here rather than adding a new gdbarch method?

Hi Yao,

The adjust_breakpoint_address method is not suitable for this patch
set.  It was originally added for the FR-V architecture.  FR-V is a
VLIW architecture which places limits on where a breakpoint may be
placed within a bundle.  For FR-V, this method ensures that a breakpoint
is placed at the beginning of the VLIW bundle.  I see now that it has
also been used for MIPS to avoid placing breakpoints on branch delay
slots and also for ARM to avoid placing breakpoints within IT
(if-then) blocks.  In each of these cases, the breakpoint is moved
because, if is not, the breakpoint might not be hit.

The gdbarch method that I'm introducing in this patch set needs to
limit itself to checking to see if the instruction at a given address
is an unconditional branch; if it is, it will return true and also
provide the branch destination address via an output parameter.  An
existing gdbarch method that might be suitable (with some
modification) for this purpose is insn_is_jump, which is presently
used by the btrace code.

The insn_is_jump method is a predicate only; at the moment, it does
not provide the branch destination.  It could be changed to do so,
but I think it makes sense to limit use of the insn_is_{jump,ret,call}
methods to btrace.  This is the justification that I provided in
another reply to this thread:

    I decided that the work I'm doing here should have it's own
    method.  If GCC should someday emit DWARF which sets is_stmt to 0
    for the branch instruction at the beginning of a while loop, this
    work might not be needed any longer.  It'll be easier to rip it
    out and clean things up again if I don't modify gdbarch methods
    that were added for other purposes.

Kevin