* [PATCH 0/2] [aarch64] Use opcodes to decode instructions in GDB @ 2015-10-01 16:35 Yao Qi 2015-10-01 16:35 ` [PATCH 1/2] [aarch64] Remove argument pc from disas_aarch64_insn Yao Qi 2015-10-01 16:36 ` [PATCH 2/2] [aarch64] Use opcodes to decode instructions in GDB Yao Qi 0 siblings, 2 replies; 12+ messages in thread From: Yao Qi @ 2015-10-01 16:35 UTC (permalink / raw) To: gdb-patches, binutils This patch series is the first try to use opcodes to decode instructions in GDB aarch64 backend. GDB should use existing macros and functions to decode and encode instructions, not only in aarch64 backend, but also in other arch backends. Patch #1 is a cleanup that removing unused argument in disas_aarch64_insn, and patch #2 is the major part of this series. See more details in it. *** BLURB HERE *** Yao Qi (2): [aarch64] Remove argument pc from disas_aarch64_insn [aarch64] Use opcodes to decode instructions in GDB gdb/aarch64-tdep.c | 29 ++++++++++++++++++----------- opcodes/aarch64-dis.c | 9 ++++----- opcodes/aarch64-dis.h | 5 +++++ 3 files changed, 27 insertions(+), 16 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] [aarch64] Remove argument pc from disas_aarch64_insn 2015-10-01 16:35 [PATCH 0/2] [aarch64] Use opcodes to decode instructions in GDB Yao Qi @ 2015-10-01 16:35 ` Yao Qi 2015-10-01 16:36 ` [PATCH 2/2] [aarch64] Use opcodes to decode instructions in GDB Yao Qi 1 sibling, 0 replies; 12+ messages in thread From: Yao Qi @ 2015-10-01 16:35 UTC (permalink / raw) To: gdb-patches, binutils I happen to see that argument pc is not used inside disas_aarch64_insn at all. This patch is to remove it. OK to apply? opcodes: 2015-10-01 Yao Qi <yao.qi@linaro.org> * aarch64-dis.c (disas_aarch64_insn): Remove argument PC. (print_insn_aarch64_word): Caller updated. --- opcodes/aarch64-dis.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/opcodes/aarch64-dis.c b/opcodes/aarch64-dis.c index 5fec4ee..e0faeb5 100644 --- a/opcodes/aarch64-dis.c +++ b/opcodes/aarch64-dis.c @@ -2032,8 +2032,7 @@ user_friendly_fixup (aarch64_inst *inst) /* Decode INSN and fill in *INST the instruction information. */ static int -disas_aarch64_insn (uint64_t pc ATTRIBUTE_UNUSED, uint32_t insn, - aarch64_inst *inst) +disas_aarch64_insn (uint32_t insn, aarch64_inst *inst) { const aarch64_opcode *opcode = aarch64_opcode_lookup (insn); @@ -2172,7 +2171,7 @@ print_insn_aarch64_word (bfd_vma pc, addresses, since the addend is not currently pc-relative. */ pc = 0; - ret = disas_aarch64_insn (pc, word, &inst); + ret = disas_aarch64_insn (word, &inst); if (((word >> 21) & 0x3ff) == 1) { -- 1.9.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] [aarch64] Use opcodes to decode instructions in GDB 2015-10-01 16:35 [PATCH 0/2] [aarch64] Use opcodes to decode instructions in GDB Yao Qi 2015-10-01 16:35 ` [PATCH 1/2] [aarch64] Remove argument pc from disas_aarch64_insn Yao Qi @ 2015-10-01 16:36 ` Yao Qi 2015-10-02 7:51 ` Marcus Shawcroft 1 sibling, 1 reply; 12+ messages in thread From: Yao Qi @ 2015-10-01 16:36 UTC (permalink / raw) To: gdb-patches, binutils I just noticed that opcodes, at least in aarch64, exposes some good interface to decode instructions. It is good for GDB to use them rather than reinventing the wheel. 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. gdb: 2015-10-01 Yao Qi <yao.qi@linaro.org> * aarch64-tdep.c: Include opcodes/aarch64-dis.h. (submask): Move it above. (bit): Likewise. (bits): Likewise. (aarch64_software_single_step): Call disas_aarch64_insn. Decode instruction by aarch64_inst instead of using aarch64_decode_bcond. --- gdb/aarch64-tdep.c | 29 ++++++++++++++++++----------- opcodes/aarch64-dis.c | 4 ++-- opcodes/aarch64-dis.h | 5 +++++ 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 5b5e1ad..25a8446 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -59,6 +59,12 @@ #include "arch/aarch64-insn.h" +#include "opcodes/aarch64-dis.h" + +#define submask(x) ((1L << ((x) + 1)) - 1) +#define bit(obj,st) (((obj) >> (st)) & 1) +#define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st))) + /* Pseudo register base numbers. */ #define AARCH64_Q0_REGNUM 0 #define AARCH64_D0_REGNUM (AARCH64_Q0_REGNUM + 32) @@ -2491,35 +2497,40 @@ aarch64_software_single_step (struct frame_info *frame) int insn_count; int bc_insn_count = 0; /* Conditional branch instruction count. */ int last_breakpoint = 0; /* Defaults to 0 (no breakpoints placed). */ + aarch64_inst inst; + + if (disas_aarch64_insn (insn, &inst) != 0) + return 0; /* Look for a Load Exclusive instruction which begins the sequence. */ - if (!decode_masked_match (insn, 0x3fc00000, 0x08400000)) + if (inst.opcode->iclass != ldstexcl || bit (insn, 22) == 0) return 0; for (insn_count = 0; insn_count < atomic_sequence_length; ++insn_count) { - int32_t offset; - unsigned cond; - loc += insn_size; insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code); + if (disas_aarch64_insn (insn, &inst) != 0) + return 0; /* Check if the instruction is a conditional branch. */ - if (aarch64_decode_bcond (loc, insn, &cond, &offset)) + if (inst.opcode->iclass == condbranch) { + gdb_assert (inst.operands[0].type == AARCH64_OPND_ADDR_PCREL19); + if (bc_insn_count >= 1) return 0; /* It is, so we'll try to set a breakpoint at the destination. */ - breaks[1] = loc + offset; + breaks[1] = loc + inst.operands[0].imm.value; bc_insn_count++; last_breakpoint++; } /* Look for the Store Exclusive which closes the atomic sequence. */ - if (decode_masked_match (insn, 0x3fc00000, 0x08000000)) + if (inst.opcode->iclass == ldstexcl && bit (insn, 22) == 0) { closing_insn = loc; break; @@ -2771,10 +2782,6 @@ When on, AArch64 specific debugging is enabled."), /* AArch64 process record-replay related structures, defines etc. */ -#define submask(x) ((1L << ((x) + 1)) - 1) -#define bit(obj,st) (((obj) >> (st)) & 1) -#define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st))) - #define REG_ALLOC(REGS, LENGTH, RECORD_BUF) \ do \ { \ diff --git a/opcodes/aarch64-dis.c b/opcodes/aarch64-dis.c index e0faeb5..1c9bc7c 100644 --- a/opcodes/aarch64-dis.c +++ b/opcodes/aarch64-dis.c @@ -2029,9 +2029,9 @@ user_friendly_fixup (aarch64_inst *inst) } } -/* Decode INSN and fill in *INST the instruction information. */ +/* See aarch64-dis.h. */ -static int +int disas_aarch64_insn (uint32_t insn, aarch64_inst *inst) { const aarch64_opcode *opcode = aarch64_opcode_lookup (insn); diff --git a/opcodes/aarch64-dis.h b/opcodes/aarch64-dis.h index 767191c..241100e 100644 --- a/opcodes/aarch64-dis.h +++ b/opcodes/aarch64-dis.h @@ -36,6 +36,11 @@ const aarch64_opcode* aarch64_opcode_lookup (uint32_t); const aarch64_opcode* aarch64_find_next_opcode (const aarch64_opcode *); +/* Decode INSN and fill in *INST the instruction information. Return zero + on success. */ + +int disas_aarch64_insn (uint32_t insn, aarch64_inst *inst); + /* Given OPCODE, return its alias, e.g. given UBFM, return LSL. In the case of multiple alias candidates, the one of the highest priority -- 1.9.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] [aarch64] Use opcodes to decode instructions in GDB 2015-10-01 16:36 ` [PATCH 2/2] [aarch64] Use opcodes to decode instructions in GDB Yao Qi @ 2015-10-02 7:51 ` Marcus Shawcroft 2015-10-02 11:24 ` [PATCH 0/3 V2] " Yao Qi 0 siblings, 1 reply; 12+ messages in thread From: Marcus Shawcroft @ 2015-10-02 7:51 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches, binutils 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 0/3 V2] [aarch64] Use opcodes to decode instructions in GDB 2015-10-02 7:51 ` Marcus Shawcroft @ 2015-10-02 11:24 ` Yao Qi 2015-10-02 11:24 ` [PATCH 3/3] [aarch64] use aarch64_decode_insn " Yao Qi ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Yao Qi @ 2015-10-02 11:24 UTC (permalink / raw) To: marcus.shawcroft; +Cc: gdb-patches, binutils Hi Marcus, On 02/10/15 08:51, Marcus Shawcroft wrote: > 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. OK, I move the declaration to includes/opcode/aarch64.h in V2. > > 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 Patch #2 in V2 does this, but renames it to aarch64_decode_insn, because aarch64_decode_insn can be used in disassemble, but also in something else. > take the patch to drop the PC argument first, then rename and expose > the function, then the gdb patch to use the interface. OK, in V2, there are three patches. Patch #1 remove the PC argument, patch #2 exposes and renames disas_aarch64_insn, and patch #3 use it GDB. > >> >+/* 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. Fixed in V2. *** BLURB HERE *** Yao Qi (3): [aarch64] Remove argument pc from disas_aarch64_insn [aarch64] expose disas_aarch64_insn and rename it to aarch64_decode_insn [aarch64] use aarch64_decode_insn to decode instructions in GDB gdb/aarch64-tdep.c | 29 ++++++++++++++++++----------- include/opcode/aarch64.h | 3 +++ opcodes/aarch64-dis.c | 10 +++++----- 3 files changed, 26 insertions(+), 16 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] [aarch64] use aarch64_decode_insn to decode instructions in GDB 2015-10-02 11:24 ` [PATCH 0/3 V2] " Yao Qi @ 2015-10-02 11:24 ` Yao Qi 2015-10-07 8:56 ` Yao Qi 2015-10-02 11:24 ` [PATCH 2/3] [aarch64] expose disas_aarch64_insn and rename it to aarch64_decode_insn Yao Qi 2015-10-02 11:24 ` [PATCH 1/3] [aarch64] Remove argument pc from disas_aarch64_insn Yao Qi 2 siblings, 1 reply; 12+ messages in thread From: Yao Qi @ 2015-10-02 11:24 UTC (permalink / raw) To: marcus.shawcroft; +Cc: gdb-patches, binutils In this patch, we start to use aarch64_decode_insn to decode instructions in aarch64_software_single_step. gdb: 2015-10-02 Yao Qi <yao.qi@linaro.org> * aarch64-tdep.c: Include opcode/aarch64.h. (submask): Move it above. (bit): Likewise. (bits): Likewise. (aarch64_software_single_step): Call aarch64_decode_insn. Decode instruction by aarch64_inst instead of using aarch64_decode_bcond and decode_masked_match. --- gdb/aarch64-tdep.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 5b5e1ad..df67e12 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -59,6 +59,12 @@ #include "arch/aarch64-insn.h" +#include "opcode/aarch64.h" + +#define submask(x) ((1L << ((x) + 1)) - 1) +#define bit(obj,st) (((obj) >> (st)) & 1) +#define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st))) + /* Pseudo register base numbers. */ #define AARCH64_Q0_REGNUM 0 #define AARCH64_D0_REGNUM (AARCH64_Q0_REGNUM + 32) @@ -2491,35 +2497,40 @@ aarch64_software_single_step (struct frame_info *frame) int insn_count; int bc_insn_count = 0; /* Conditional branch instruction count. */ int last_breakpoint = 0; /* Defaults to 0 (no breakpoints placed). */ + aarch64_inst inst; + + if (aarch64_decode_insn (insn, &inst) != 0) + return 0; /* Look for a Load Exclusive instruction which begins the sequence. */ - if (!decode_masked_match (insn, 0x3fc00000, 0x08400000)) + if (inst.opcode->iclass != ldstexcl || bit (insn, 22) == 0) return 0; for (insn_count = 0; insn_count < atomic_sequence_length; ++insn_count) { - int32_t offset; - unsigned cond; - loc += insn_size; insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code); + if (aarch64_decode_insn (insn, &inst) != 0) + return 0; /* Check if the instruction is a conditional branch. */ - if (aarch64_decode_bcond (loc, insn, &cond, &offset)) + if (inst.opcode->iclass == condbranch) { + gdb_assert (inst.operands[0].type == AARCH64_OPND_ADDR_PCREL19); + if (bc_insn_count >= 1) return 0; /* It is, so we'll try to set a breakpoint at the destination. */ - breaks[1] = loc + offset; + breaks[1] = loc + inst.operands[0].imm.value; bc_insn_count++; last_breakpoint++; } /* Look for the Store Exclusive which closes the atomic sequence. */ - if (decode_masked_match (insn, 0x3fc00000, 0x08000000)) + if (inst.opcode->iclass == ldstexcl && bit (insn, 22) == 0) { closing_insn = loc; break; @@ -2771,10 +2782,6 @@ When on, AArch64 specific debugging is enabled."), /* AArch64 process record-replay related structures, defines etc. */ -#define submask(x) ((1L << ((x) + 1)) - 1) -#define bit(obj,st) (((obj) >> (st)) & 1) -#define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st))) - #define REG_ALLOC(REGS, LENGTH, RECORD_BUF) \ do \ { \ -- 1.9.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] [aarch64] use aarch64_decode_insn to decode instructions in GDB 2015-10-02 11:24 ` [PATCH 3/3] [aarch64] use aarch64_decode_insn " Yao Qi @ 2015-10-07 8:56 ` Yao Qi 0 siblings, 0 replies; 12+ messages in thread From: Yao Qi @ 2015-10-07 8:56 UTC (permalink / raw) To: Yao Qi; +Cc: marcus.shawcroft, gdb-patches, binutils Yao Qi <qiyaoltc@gmail.com> writes: > gdb: > > 2015-10-02 Yao Qi <yao.qi@linaro.org> > > * aarch64-tdep.c: Include opcode/aarch64.h. > (submask): Move it above. > (bit): Likewise. > (bits): Likewise. > (aarch64_software_single_step): Call aarch64_decode_insn. > Decode instruction by aarch64_inst instead of using > aarch64_decode_bcond and decode_masked_match. Patch is pushed in. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] [aarch64] expose disas_aarch64_insn and rename it to aarch64_decode_insn 2015-10-02 11:24 ` [PATCH 0/3 V2] " Yao Qi 2015-10-02 11:24 ` [PATCH 3/3] [aarch64] use aarch64_decode_insn " Yao Qi @ 2015-10-02 11:24 ` Yao Qi 2015-10-02 12:35 ` Marcus Shawcroft 2015-10-02 11:24 ` [PATCH 1/3] [aarch64] Remove argument pc from disas_aarch64_insn Yao Qi 2 siblings, 1 reply; 12+ messages in thread From: Yao Qi @ 2015-10-02 11:24 UTC (permalink / raw) To: marcus.shawcroft; +Cc: gdb-patches, binutils We want to use disas_aarch64_insn inside GDB to decode instructions, so this patch exposes it and rename it to aarch64_decode_insn to follow the conventions of other interfaces. This patch also change argument insn type from uint32_t to aarch64_insn. include/opcode: 2015-10-02 Yao Qi <yao.qi@linaro.org> * aarch64.h (aarch64_decode_insn): Declare it. opcodes: 2015-10-02 Yao Qi <yao.qi@linaro.org> * aarch64-dis.c (disas_aarch64_insn): Remove static. Change argument insn type to aarch64_insn. Rename to ... (aarch64_decode_insn): ... it. (print_insn_aarch64_word): Caller updated. --- include/opcode/aarch64.h | 3 +++ opcodes/aarch64-dis.c | 9 +++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h index dcf0fef..d0c7489 100644 --- a/include/opcode/aarch64.h +++ b/include/opcode/aarch64.h @@ -925,6 +925,9 @@ aarch64_stack_pointer_p (const aarch64_opnd_info *); extern int aarch64_zero_register_p (const aarch64_opnd_info *); +extern +int aarch64_decode_insn (aarch64_insn, aarch64_inst *); + /* Given an operand qualifier, return the expected data element size of a qualified operand. */ extern unsigned char diff --git a/opcodes/aarch64-dis.c b/opcodes/aarch64-dis.c index e0faeb5..fe3caac 100644 --- a/opcodes/aarch64-dis.c +++ b/opcodes/aarch64-dis.c @@ -2029,10 +2029,11 @@ user_friendly_fixup (aarch64_inst *inst) } } -/* Decode INSN and fill in *INST the instruction information. */ +/* Decode INSN and fill in *INST the instruction information. Return zero + on success. */ -static int -disas_aarch64_insn (uint32_t insn, aarch64_inst *inst) +int +aarch64_decode_insn (aarch64_insn insn, aarch64_inst *inst) { const aarch64_opcode *opcode = aarch64_opcode_lookup (insn); @@ -2171,7 +2172,7 @@ print_insn_aarch64_word (bfd_vma pc, addresses, since the addend is not currently pc-relative. */ pc = 0; - ret = disas_aarch64_insn (word, &inst); + ret = aarch64_decode_insn (word, &inst); if (((word >> 21) & 0x3ff) == 1) { -- 1.9.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] [aarch64] expose disas_aarch64_insn and rename it to aarch64_decode_insn 2015-10-02 11:24 ` [PATCH 2/3] [aarch64] expose disas_aarch64_insn and rename it to aarch64_decode_insn Yao Qi @ 2015-10-02 12:35 ` Marcus Shawcroft 2015-10-02 14:32 ` Yao Qi 0 siblings, 1 reply; 12+ messages in thread From: Marcus Shawcroft @ 2015-10-02 12:35 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches, binutils On 2 October 2015 at 12:23, Yao Qi <qiyaoltc@gmail.com> wrote: > include/opcode: > > 2015-10-02 Yao Qi <yao.qi@linaro.org> > > * aarch64.h (aarch64_decode_insn): Declare it. > > opcodes: > > 2015-10-02 Yao Qi <yao.qi@linaro.org> > > * aarch64-dis.c (disas_aarch64_insn): Remove static. Change > argument insn type to aarch64_insn. Rename to ... > (aarch64_decode_insn): ... it. > (print_insn_aarch64_word): Caller updated. > --- > include/opcode/aarch64.h | 3 +++ > opcodes/aarch64-dis.c | 9 +++++---- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h > index dcf0fef..d0c7489 100644 > --- a/include/opcode/aarch64.h > +++ b/include/opcode/aarch64.h > @@ -925,6 +925,9 @@ aarch64_stack_pointer_p (const aarch64_opnd_info *); > extern > int aarch64_zero_register_p (const aarch64_opnd_info *); > > +extern > +int aarch64_decode_insn (aarch64_insn, aarch64_inst *); The usual layout would be: extern int aarch64_decode_insn (.... OK with that change. /Marcus ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] [aarch64] expose disas_aarch64_insn and rename it to aarch64_decode_insn 2015-10-02 12:35 ` Marcus Shawcroft @ 2015-10-02 14:32 ` Yao Qi 0 siblings, 0 replies; 12+ messages in thread From: Yao Qi @ 2015-10-02 14:32 UTC (permalink / raw) To: Marcus Shawcroft; +Cc: gdb-patches, binutils On 02/10/15 13:35, Marcus Shawcroft wrote: > The usual layout would be: > > extern int > aarch64_decode_insn (.... Sigh, I must be misled by aarch64_zero_register_p above... > > OK with that change. Patch below is what I pushed in. Thanks for the review. -- Yao (é½å°§) include/opcode: 2015-10-02 Yao Qi <yao.qi@linaro.org> * aarch64.h (aarch64_decode_insn): Declare it. opcodes: 2015-10-02 Yao Qi <yao.qi@linaro.org> * aarch64-dis.c (disas_aarch64_insn): Remove static. Change argument insn type to aarch64_insn. Rename to ... (aarch64_decode_insn): ... it. (print_insn_aarch64_word): Caller updated. diff --git a/include/opcode/ChangeLog b/include/opcode/ChangeLog index aa5ea1c..a93d964 100644 --- a/include/opcode/ChangeLog +++ b/include/opcode/ChangeLog @@ -1,3 +1,7 @@ +2015-10-02 Yao Qi <yao.qi@linaro.org> + + * aarch64.h (aarch64_decode_insn): Declare it. + 2015-09-29 Dominik Vogt <vogt@linux.vnet.ibm.com> * s390.h (S390_INSTR_FLAG_HTM): New flag. diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h index dcf0fef..ac4da28 100644 --- a/include/opcode/aarch64.h +++ b/include/opcode/aarch64.h @@ -925,6 +925,9 @@ aarch64_stack_pointer_p (const aarch64_opnd_info *); extern int aarch64_zero_register_p (const aarch64_opnd_info *); +extern int +aarch64_decode_insn (aarch64_insn, aarch64_inst *); + /* Given an operand qualifier, return the expected data element size of a qualified operand. */ extern unsigned char diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog index 051c42b..124ead7 100644 --- a/opcodes/ChangeLog +++ b/opcodes/ChangeLog @@ -1,5 +1,12 @@ 2015-10-02 Yao Qi <yao.qi@linaro.org> + * aarch64-dis.c (disas_aarch64_insn): Remove static. Change + argument insn type to aarch64_insn. Rename to ... + (aarch64_decode_insn): ... it. + (print_insn_aarch64_word): Caller updated. + +2015-10-02 Yao Qi <yao.qi@linaro.org> + * aarch64-dis.c (disas_aarch64_insn): Remove argument PC. (print_insn_aarch64_word): Caller updated. diff --git a/opcodes/aarch64-dis.c b/opcodes/aarch64-dis.c index e0faeb5..fe3caac 100644 --- a/opcodes/aarch64-dis.c +++ b/opcodes/aarch64-dis.c @@ -2029,10 +2029,11 @@ user_friendly_fixup (aarch64_inst *inst) } } -/* Decode INSN and fill in *INST the instruction information. */ +/* Decode INSN and fill in *INST the instruction information. Return zero + on success. */ -static int -disas_aarch64_insn (uint32_t insn, aarch64_inst *inst) +int +aarch64_decode_insn (aarch64_insn insn, aarch64_inst *inst) { const aarch64_opcode *opcode = aarch64_opcode_lookup (insn); @@ -2171,7 +2172,7 @@ print_insn_aarch64_word (bfd_vma pc, addresses, since the addend is not currently pc-relative. */ pc = 0; - ret = disas_aarch64_insn (word, &inst); + ret = aarch64_decode_insn (word, &inst); if (((word >> 21) & 0x3ff) == 1) { ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] [aarch64] Remove argument pc from disas_aarch64_insn 2015-10-02 11:24 ` [PATCH 0/3 V2] " Yao Qi 2015-10-02 11:24 ` [PATCH 3/3] [aarch64] use aarch64_decode_insn " Yao Qi 2015-10-02 11:24 ` [PATCH 2/3] [aarch64] expose disas_aarch64_insn and rename it to aarch64_decode_insn Yao Qi @ 2015-10-02 11:24 ` Yao Qi 2015-10-02 12:30 ` Marcus Shawcroft 2 siblings, 1 reply; 12+ messages in thread From: Yao Qi @ 2015-10-02 11:24 UTC (permalink / raw) To: marcus.shawcroft; +Cc: gdb-patches, binutils I happen to see that argument pc is not used inside disas_aarch64_insn at all. This patch is to remove it. OK to apply? opcodes: 2015-10-01 Yao Qi <yao.qi@linaro.org> * aarch64-dis.c (disas_aarch64_insn): Remove argument PC. (print_insn_aarch64_word): Caller updated. --- opcodes/aarch64-dis.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/opcodes/aarch64-dis.c b/opcodes/aarch64-dis.c index 5fec4ee..e0faeb5 100644 --- a/opcodes/aarch64-dis.c +++ b/opcodes/aarch64-dis.c @@ -2032,8 +2032,7 @@ user_friendly_fixup (aarch64_inst *inst) /* Decode INSN and fill in *INST the instruction information. */ static int -disas_aarch64_insn (uint64_t pc ATTRIBUTE_UNUSED, uint32_t insn, - aarch64_inst *inst) +disas_aarch64_insn (uint32_t insn, aarch64_inst *inst) { const aarch64_opcode *opcode = aarch64_opcode_lookup (insn); @@ -2172,7 +2171,7 @@ print_insn_aarch64_word (bfd_vma pc, addresses, since the addend is not currently pc-relative. */ pc = 0; - ret = disas_aarch64_insn (pc, word, &inst); + ret = disas_aarch64_insn (word, &inst); if (((word >> 21) & 0x3ff) == 1) { -- 1.9.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] [aarch64] Remove argument pc from disas_aarch64_insn 2015-10-02 11:24 ` [PATCH 1/3] [aarch64] Remove argument pc from disas_aarch64_insn Yao Qi @ 2015-10-02 12:30 ` Marcus Shawcroft 0 siblings, 0 replies; 12+ messages in thread From: Marcus Shawcroft @ 2015-10-02 12:30 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches, binutils On 2 October 2015 at 12:23, Yao Qi <qiyaoltc@gmail.com> wrote: > I happen to see that argument pc is not used inside disas_aarch64_insn > at all. This patch is to remove it. > > OK to apply? > > opcodes: > > 2015-10-01 Yao Qi <yao.qi@linaro.org> > > * aarch64-dis.c (disas_aarch64_insn): Remove argument PC. > (print_insn_aarch64_word): Caller updated. Ok /Marcus ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-10-07 8:56 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-01 16:35 [PATCH 0/2] [aarch64] Use opcodes to decode instructions in GDB Yao Qi 2015-10-01 16:35 ` [PATCH 1/2] [aarch64] Remove argument pc from disas_aarch64_insn Yao Qi 2015-10-01 16:36 ` [PATCH 2/2] [aarch64] Use opcodes to decode instructions in GDB Yao Qi 2015-10-02 7:51 ` Marcus Shawcroft 2015-10-02 11:24 ` [PATCH 0/3 V2] " Yao Qi 2015-10-02 11:24 ` [PATCH 3/3] [aarch64] use aarch64_decode_insn " Yao Qi 2015-10-07 8:56 ` Yao Qi 2015-10-02 11:24 ` [PATCH 2/3] [aarch64] expose disas_aarch64_insn and rename it to aarch64_decode_insn Yao Qi 2015-10-02 12:35 ` Marcus Shawcroft 2015-10-02 14:32 ` Yao Qi 2015-10-02 11:24 ` [PATCH 1/3] [aarch64] Remove argument pc from disas_aarch64_insn Yao Qi 2015-10-02 12:30 ` Marcus Shawcroft
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox