Re: [patch, nios2] Nios II R2 support for GDB

[Sandra Loosemore <sandra at codesourcery dot com>]

On 08/03/2015 08:00 AM, Yao Qi wrote:
> Sandra Loosemore <sandra@codesourcery.com> writes:
>
> Hi Sandra,
> The patch looks good to me.  Some comments below...

Thanks for the review!  You have sharp eyes to pick up those extra 
trailing whitespace problems.

> >   static CORE_ADDR
> > -nios2_linux_syscall_next_pc (struct frame_info *frame)
> > +nios2_linux_syscall_next_pc (struct frame_info *frame,
> > +			     const struct nios2_opcode *op)
> >   {
> >     CORE_ADDR pc = get_frame_pc (frame);
> >     ULONGEST syscall_nr = get_frame_register_unsigned (frame, NIOS2_R2_REGNUM);
> > @@ -182,7 +200,7 @@ nios2_linux_syscall_next_pc (struct frame_info *frame)
> >     if (syscall_nr == 139 /* rt_sigreturn */)
> >       return frame_unwind_caller_pc (frame);
> >
> > -  return pc + NIOS2_OPCODE_SIZE;
> > +  return pc + op->size;
>
> Nit: Can we get the size of opcode without OP here?  We can get the
> gdbarch of FRAME by frame_unwind_arch, we can tell it is R1 or R2, and
> get to know the size of the instruction?

The arch alone is not enough to set the size of the break instruction; 
R2 has both 16-bit and 32-bit variants.  I suppose it might be that the 
future R2 Linux support will be implemented in such a way as to always 
use the 32-bit variant, but in the general case for R2 you have to 
disassemble an instruction to find out how long it is.  And the only 
caller of this hook has just done that already and put the result in the 
OP parameter; it seems better not to duplicate that code in another file 
when the information is already right there.

-Sandra