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

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

[Yao Qi]

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

[PATCH 1/2] [aarch64] Remove argument pc from disas_aarch64_insn

[Yao Qi]

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

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

[Yao Qi]

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

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

[Marcus Shawcroft]

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

[PATCH 0/3 V2] [aarch64] Use opcodes to decode instructions in GDB

[Yao Qi]

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

[PATCH 2/3] [aarch64] expose disas_aarch64_insn and rename it to aarch64_decode_insn

[Yao Qi]

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

Re: [PATCH 2/3] [aarch64] expose disas_aarch64_insn and rename it to aarch64_decode_insn

[Marcus Shawcroft]

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

Re: [PATCH 2/3] [aarch64] expose disas_aarch64_insn and rename it to aarch64_decode_insn

[Yao Qi]

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)
     {

[PATCH 1/3] [aarch64] Remove argument pc from disas_aarch64_insn

[Yao Qi]

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

Re: [PATCH 1/3] [aarch64] Remove argument pc from disas_aarch64_insn

[Marcus Shawcroft]

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

[PATCH 3/3] [aarch64] use aarch64_decode_insn to decode instructions in GDB

[Yao Qi]

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

Re: [PATCH 3/3] [aarch64] use aarch64_decode_insn to decode instructions in GDB

[Yao Qi]

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 (齐尧)