[PATCH 0/2][GDBServer][AArch64] Enable support for Z0 packets

[PATCH 0/2][GDBServer][AArch64] Enable support for Z0 packets

[Pierre Langlois]

Hi all,

This series of patches lets GDBServer handle software breakpoints for the
AArch64 target.  The main reason for doing this is that it will allow us to
enable support for tracepoints later.

Simply enabling support for the Z0 packet uncovered an issue which is what the
first patch addresses: We need GDB and GDBServer using the same breakpoint
instruction.  The second patch finally enables Z0 packets.

I ran the regression tests in a remote configuration on aarch64 which
activated the following previously unsupported test:

- PASS: gdb.base/dprintf.exp: 1st dprintf, agent
- PASS: gdb.base/dprintf.exp: 2nd dprintf, agent
- PASS: gdb.base/dprintf.exp: dprintf info 2
- PASS: gdb.mi/mi-dprintf.exp: mi expect stop
- PASS: gdb.mi/mi-dprintf.exp: mi 1st dprintf, agent
- PASS: gdb.mi/mi-dprintf.exp: mi info dprintf second time
- PASS: gdb.mi/mi-dprintf.exp: mi 2nd dprintf, agent

Thanks,
Pierre

Pierre Langlois (2):
  [GDBServer][AArch64] Use the same break instruction as GDB
  [GDBServer][AArch64] Enable support for Z0 packets

 gdb/gdbserver/ChangeLog           | 12 ++++++++++++
 gdb/gdbserver/linux-aarch64-low.c | 16 +++++++++-------
 2 files changed, 21 insertions(+), 7 deletions(-)

-- 
2.1.0

[PATCH 2/2] [GDBServer][AArch64] Enable support for Z0 packets

[Pierre Langlois]

This patch lets GDBServer handle software breakpoints instead of relying
on GDB.

gdb/gdbserver/ChangeLog:

	* linux-aarch64-low.c (aarch64_supports_z_point_type): Enable for
	Z_PACKET_SW_BP.
---
 gdb/gdbserver/ChangeLog           | 5 +++++
 gdb/gdbserver/linux-aarch64-low.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index dde7e9e..6873b0c 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,10 @@
 2015-06-26  Pierre Langlois  <pierre.langlois@arm.com>
 
+	* linux-aarch64-low.c (aarch64_supports_z_point_type): Enable for
+	Z_PACKET_SW_BP.
+
+2015-06-26  Pierre Langlois  <pierre.langlois@arm.com>
+
 	* linux-aarch64-low.c: Remove comment about endianness.
 	(aarch64_breakpoint): Change type to gdb_byte[].  Set to "brk #0".
 	(aarch64_breakpoint_at): Change type of insn to gdb_byte[].  Use
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index b0a2775..8a30b00 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -952,13 +952,13 @@ aarch64_supports_z_point_type (char z_type)
 {
   switch (z_type)
     {
+    case Z_PACKET_SW_BP:
     case Z_PACKET_HW_BP:
     case Z_PACKET_WRITE_WP:
     case Z_PACKET_READ_WP:
     case Z_PACKET_ACCESS_WP:
       return 1;
     default:
-      /* Leave the handling of sw breakpoints with the gdb client.  */
       return 0;
     }
 }
-- 
2.1.0

Re: [PATCH 2/2] [GDBServer][AArch64] Enable support for Z0 packets

[Yao Qi]

Pierre Langlois <pierre.langlois@arm.com> writes:

> This patch lets GDBServer handle software breakpoints instead of relying
> on GDB.
>
> gdb/gdbserver/ChangeLog:
>
> 	* linux-aarch64-low.c (aarch64_supports_z_point_type): Enable for
> 	Z_PACKET_SW_BP.

This patch is OK to me.

-- 
Yao (齐尧)

[PATCH 1/2] [GDBServer][AArch64] Use the same break instruction as GDB

[Pierre Langlois]

GDB uses a "brk #0" instruction to perform a software breakpoint while
GDBServer uses an illegal instruction.  Both instructions should match.

When enabling support for the 'Z0' packet, we let GDBServer insert the
breakpoint instruction instead of GDB.  And in case of permanent
breakpoints for example, GDB will check if a breakpoint is inserted in the
inferior with `program_breakpoint_here_p (gdbarch, address)', and
compare the instruction read from the inferior with the breakpoint
instruction.

On AArch64, instructions are always little endian so we need to
represent it as an array of bytes, as done in aarch64-tdep.c.

gdb/gdbserver/ChangeLog:

	* linux-aarch64-low.c: Remove comment about endianness.
	(aarch64_breakpoint): Change type to gdb_byte[].  Set to "brk #0".
	(aarch64_breakpoint_at): Change type of insn to gdb_byte[].  Use
	memcmp.
---
 gdb/gdbserver/ChangeLog           |  7 +++++++
 gdb/gdbserver/linux-aarch64-low.c | 14 ++++++++------
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 2528f0f..dde7e9e 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,10 @@
+2015-06-26  Pierre Langlois  <pierre.langlois@arm.com>
+
+	* linux-aarch64-low.c: Remove comment about endianness.
+	(aarch64_breakpoint): Change type to gdb_byte[].  Set to "brk #0".
+	(aarch64_breakpoint_at): Change type of insn to gdb_byte[].  Use
+	memcmp.
+
 2015-06-24  Gary Benson  <gbenson@redhat.com>
 
 	* linux-i386-ipa.c (stdint.h): Do not include.
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 043458d..b0a2775 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -295,19 +295,21 @@ aarch64_set_pc (struct regcache *regcache, CORE_ADDR pc)
   supply_register_by_name (regcache, "pc", &newpc);
 }
 
-/* Correct in either endianness.  */
-
 #define aarch64_breakpoint_len 4
 
-static const unsigned long aarch64_breakpoint = 0x00800011;
+/* AArch64 BRK software debug mode instruction.
+   This instruction needs to match gdb/aarch64-tdep.c
+   (aarch64_default_breakpoint).  */
+static const gdb_byte aarch64_breakpoint[] = {0x00, 0x00, 0x20, 0xd4};
 
 static int
 aarch64_breakpoint_at (CORE_ADDR where)
 {
-  unsigned long insn;
+  gdb_byte insn[aarch64_breakpoint_len];
 
-  (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
-  if (insn == aarch64_breakpoint)
+  (*the_target->read_memory) (where, (unsigned char *) &insn,
+			      aarch64_breakpoint_len);
+  if (memcmp (insn, aarch64_breakpoint, aarch64_breakpoint_len) == 0)
     return 1;
 
   return 0;
-- 
2.1.0

Re: [PATCH 1/2] [GDBServer][AArch64] Use the same break instruction as GDB

[Yao Qi]

Pierre Langlois <pierre.langlois@arm.com> writes:

> gdb/gdbserver/ChangeLog:
>
> 	* linux-aarch64-low.c: Remove comment about endianness.
> 	(aarch64_breakpoint): Change type to gdb_byte[].  Set to "brk #0".
> 	(aarch64_breakpoint_at): Change type of insn to gdb_byte[].  Use
> 	memcmp.

This patch is OK to me.

-- 
Yao (齐尧)

Re: [PATCH 0/2][GDBServer][AArch64] Enable support for Z0 packets

[Pierre Langlois]

On 26/06/15 14:42, Pierre Langlois wrote:
> Hi all,
> 
> This series of patches lets GDBServer handle software breakpoints for the
> AArch64 target.  The main reason for doing this is that it will allow us to
> enable support for tracepoints later.
> 
> Simply enabling support for the Z0 packet uncovered an issue which is what the
> first patch addresses: We need GDB and GDBServer using the same breakpoint
> instruction.  The second patch finally enables Z0 packets.
> 
> I ran the regression tests in a remote configuration on aarch64 which
> activated the following previously unsupported test:
> 
> - PASS: gdb.base/dprintf.exp: 1st dprintf, agent
> - PASS: gdb.base/dprintf.exp: 2nd dprintf, agent
> - PASS: gdb.base/dprintf.exp: dprintf info 2
> - PASS: gdb.mi/mi-dprintf.exp: mi expect stop
> - PASS: gdb.mi/mi-dprintf.exp: mi 1st dprintf, agent
> - PASS: gdb.mi/mi-dprintf.exp: mi info dprintf second time
> - PASS: gdb.mi/mi-dprintf.exp: mi 2nd dprintf, agent
> 
> Thanks,
> Pierre
> 
> Pierre Langlois (2):
>   [GDBServer][AArch64] Use the same break instruction as GDB
>   [GDBServer][AArch64] Enable support for Z0 packets
> 
>  gdb/gdbserver/ChangeLog           | 12 ++++++++++++
>  gdb/gdbserver/linux-aarch64-low.c | 16 +++++++++-------
>  2 files changed, 21 insertions(+), 7 deletions(-)
> 

I've pushed these patches.