Re: [PATCH 2/2] [aarch64] Use opcodes to decode instructions in GDB

[Marcus Shawcroft <marcus.shawcroft@gmail.com>]

Hi Yao

On 1 October 2015 at 17:35, Yao Qi <qiyaoltc@gmail.com> wrote:

> In this patch, I expose disas_aarch64_insn in opcodes, and use it in
> aarch64_software_single_step to decode instructions.  If this is a
> good way to go, I'll continue using disas_aarch64_insn in other
> places such as prologue analysis and even fast tracepoint in GDBserver.
>
> Regression tested GDB for target aarch64-linux-gnu.  Is opcodes change OK?
>
> opcodes:
>
> 2015-10-01  Yao Qi  <yao.qi@linaro.org>
>
>         * aarch64-dis.c (disas_aarch64_insn): Make it external.  Update
>         comments.
>         * aarch64-dis.h (disas_aarch64_insn): Declare it.

I'll let others comment on the gdb aspect of this patch, w.r.t
opcodes, the aarch64-dis.h header is internal to opcodes.  The public
interface to opcodes is exposed via  include/opcode/aarch64.h or
include/dis-asm.h.  The latter exposes just the cross architecture
disassembler interface so I think  includes/opcode/aarch64.h is the
right choice in this case.

Before we expose this function, can we put in a patch to rename it to
following the name space convention used by the other exposed
functions, something like aarch64_disassemble_insn.? I think we should
take the patch to drop the PC argument first, then rename and expose
the function, then the gdb patch to use the interface.

> +/* Decode INSN and fill in *INST the instruction information.  Return zero
> +   on success.  */
> +
> +int disas_aarch64_insn (uint32_t insn, aarch64_inst *inst);

The prototype should drop the formal argument names, irrespective of
which .h file it lands in.

Cheers
/Marcus