[binutils-gdb] Implement breakpoint_kind_from_pc and sw_breakpoint_from_kind for ARM in GDBServer.

[Antoine Tremblay <hexa at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8689682cc375f6f30d65f1583b3aaa3e6a1e4d63

commit 8689682cc375f6f30d65f1583b3aaa3e6a1e4d63
Author: Antoine Tremblay <antoine.tremblay@ericsson.com>
Date:   Wed Oct 21 11:13:41 2015 -0400

    Implement breakpoint_kind_from_pc and sw_breakpoint_from_kind for ARM in GDBServer.
    
    ARM can have multiple breakpoint types based on the instruction set
    it's currently in: arm, thumb or thumb2.
    
    GDBServer needs to know what breakpoint is to be inserted at location
    when inserting a breakpoint.
    
    This is handled by the breakpoint_kind_from_pc and sw_breakpoint_from_kind
    target ops introduced in a previous patch, this patch adds the
    arm_breakpoint_kind_from_pc and arm_sw_breakpoint_from_kind implementation so
    that the proper breakpoint type is returned based on the pc.
    
    Also in order to share some code with GDB a new file called arm.c have been
    introduced in arch/.
    
    While this file does not contain much yet future patches will add more
    to it thus the inclusion at this stage.
    
    No regressions on Ubuntu 14.04 on ARMv7 and x86.
    With gdbserver-{native,extended} / { -marm -mthumb }
    
    gdb/ChangeLog:
    
    	* Makefile.in: Add arm.c/o.
    	* arch/arm.c: New file.
    	* arch/arm.h: (IS_THUMB_ADDR): Move macro from arm-tdep.c.
    	(MAKE_THUMB_ADDR): Likewise.
    	(UNMAKE_THUMB_ADDR): Likewise.
    	* arm-tdep.c (int thumb_insn_size): Move to arm.c.
    	(IS_THUMB_ADDR): Move to arm.h.
    	(MAKE_THUMB_ADDR): Likewise.
    	(UNMAKE_THUMB_ADDR): Likewise.
    	* configure.tgt: Add arm.o to all ARM configs.
    
    gdb/gdbserver/ChangeLog:
    
    	* Makefile.in: Add arm.c/o.
    	* configure.srv: Likewise.
    	* linux-arm-low.c (arm_breakpoint_kinds): New enum.
    	(arm_breakpoint_kind_from_pc): New function.
    	(arm_sw_breakpoint_from_kind): Return proper kind.
    	(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize.

Diff:
---
 gdb/ChangeLog                 | 13 +++++++++
 gdb/Makefile.in               | 13 ++++++++-
 gdb/arch/arm.c                | 33 ++++++++++++++++++++++
 gdb/arch/arm.h                | 12 +++++++-
 gdb/arm-tdep.c                | 21 +-------------
 gdb/configure.tgt             | 16 +++++------
 gdb/gdbserver/ChangeLog       |  9 ++++++
 gdb/gdbserver/Makefile.in     |  9 +++++-
 gdb/gdbserver/configure.srv   |  1 +
 gdb/gdbserver/linux-arm-low.c | 66 ++++++++++++++++++++++++++++++++++++++++---
 10 files changed, 158 insertions(+), 35 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 37e2bb8..3ca3d6b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2015-10-21  Antoine Tremblay  <antoine.tremblay@ericsson.com>
+
+	* Makefile.in: Add arm.c/o.
+	* arch/arm.c: New file.
+	* arch/arm.h: (IS_THUMB_ADDR): Move macro from arm-tdep.c.
+	(MAKE_THUMB_ADDR): Likewise.
+	(UNMAKE_THUMB_ADDR): Likewise.
+	* arm-tdep.c (int thumb_insn_size): Move to arm.c.
+	(IS_THUMB_ADDR): Move to arm.h.
+	(MAKE_THUMB_ADDR): Likewise.
+	(UNMAKE_THUMB_ADDR): Likewise.
+	* configure.tgt: Add arm.o to all ARM configs.
+
 2015-10-21  Yao Qi  <yao.qi@linaro.org>
 
 	* lib/range-stepping-support.exp (exec_cmd_expect_vCont_count):
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index d5ca2ee..14ad405 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -657,7 +657,7 @@ ALL_64_TARGET_OBS = \
 
 # All other target-dependent objects files (used with --enable-targets=all).
 ALL_TARGET_OBS = \
-	armbsd-tdep.o arm-linux-tdep.o arm-symbian-tdep.o \
+	armbsd-tdep.o arm.o arm-linux-tdep.o arm-symbian-tdep.o \
 	armnbsd-tdep.o armobsd-tdep.o \
 	arm-tdep.o arm-wince-tdep.o \
 	avr-tdep.o \
@@ -1660,6 +1660,7 @@ ALLDEPFILES = \
 	amd64-dicos-tdep.c \
 	amd64-linux-nat.c amd64-linux-tdep.c \
 	amd64-sol2-tdep.c \
+	arm.c \
 	arm-linux-nat.c arm-linux-tdep.c arm-symbian-tdep.c arm-tdep.c \
 	armnbsd-nat.c armbsd-tdep.c armnbsd-tdep.c armobsd-tdep.c \
 	avr-tdep.c \
@@ -2275,6 +2276,16 @@ waitstatus.o: ${srcdir}/target/waitstatus.c
 	$(COMPILE) $(srcdir)/target/waitstatus.c
 	$(POSTCOMPILE)
 
+#
+# gdb/arch/ dependencies
+#
+# Need to explicitly specify the compile rule as make will do nothing
+# or try to compile the object file into the sub-directory.
+
+arm.o: ${srcdir}/arch/arm.c
+	$(COMPILE) $(srcdir)/arch/arm.c
+	$(POSTCOMPILE)
+
 # gdb/nat/ dependencies
 #
 # Need to explicitly specify the compile rule as make will do nothing
diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
new file mode 100644
index 0000000..b11c684
--- /dev/null
+++ b/gdb/arch/arm.c
@@ -0,0 +1,33 @@
+/* Common target dependent code for GDB on ARM systems.
+
+   Copyright (C) 1988-2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "common-defs.h"
+#include "arm.h"
+
+/* Return the size in bytes of the complete Thumb instruction whose
+   first halfword is INST1.  */
+
+int
+thumb_insn_size (unsigned short inst1)
+{
+  if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0)
+    return 4;
+  else
+    return 2;
+}
diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
index e0eed60..a054776 100644
--- a/gdb/arch/arm.h
+++ b/gdb/arch/arm.h
@@ -1,5 +1,5 @@
 /* Common target dependent code for GDB on ARM systems.
-   Copyright (C) 2002-2015 Free Software Foundation, Inc.
+   Copyright (C) 1988-2015 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -58,4 +58,14 @@ enum gdb_regnum {
   ARM_LAST_FP_ARG_REGNUM = ARM_F3_REGNUM
 };
 
+/* Addresses for calling Thumb functions have the bit 0 set.
+   Here are some macros to test, set, or clear bit 0 of addresses.  */
+#define IS_THUMB_ADDR(addr)    ((addr) & 1)
+#define MAKE_THUMB_ADDR(addr)  ((addr) | 1)
+#define UNMAKE_THUMB_ADDR(addr) ((addr) & ~1)
+
+/* Return the size in bytes of the complete Thumb instruction whose
+   first halfword is INST1.  */
+int thumb_insn_size (unsigned short inst1);
+
 #endif
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 249e1d1..3a6c6d8 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -45,6 +45,7 @@
 #include "user-regs.h"
 #include "observer.h"
 
+#include "arch/arm.h"
 #include "arm-tdep.h"
 #include "gdb/sim-arm.h"
 
@@ -235,8 +236,6 @@ static void arm_neon_quad_write (struct gdbarch *gdbarch,
 				 struct regcache *regcache,
 				 int regnum, const gdb_byte *buf);
 
-static int thumb_insn_size (unsigned short inst1);
-
 struct arm_prologue_cache
 {
   /* The stack pointer at the time this frame was created; i.e. the
@@ -267,12 +266,6 @@ static CORE_ADDR arm_analyze_prologue (struct gdbarch *gdbarch,
 
 #define DISPLACED_STEPPING_ARCH_VERSION		5
 
-/* Addresses for calling Thumb functions have the bit 0 set.
-   Here are some macros to test, set, or clear bit 0 of addresses.  */
-#define IS_THUMB_ADDR(addr)	((addr) & 1)
-#define MAKE_THUMB_ADDR(addr)	((addr) | 1)
-#define UNMAKE_THUMB_ADDR(addr) ((addr) & ~1)
-
 /* Set to true if the 32-bit mode is in use.  */
 
 int arm_apcs_32 = 1;
@@ -4364,18 +4357,6 @@ bitcount (unsigned long val)
   return nbits;
 }
 
-/* Return the size in bytes of the complete Thumb instruction whose
-   first halfword is INST1.  */
-
-static int
-thumb_insn_size (unsigned short inst1)
-{
-  if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0)
-    return 4;
-  else
-    return 2;
-}
-
 static int
 thumb_advance_itstate (unsigned int itstate)
 {
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 33d4cfc..2e824ad 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -44,7 +44,7 @@ aarch64*-*-elf)
 aarch64*-*-linux*)
 	# Target: AArch64 linux
 	gdb_target_obs="aarch64-tdep.o aarch64-linux-tdep.o aarch64-insn.o \
-			arm-tdep.o arm-linux-tdep.o \
+			arm.o arm-tdep.o arm-linux-tdep.o \
 			glibc-tdep.o linux-tdep.o solib-svr4.o \
 			symfile-mem.o linux-record.o"
 	build_gdbserver=yes
@@ -84,31 +84,31 @@ am33_2.0*-*-linux*)
 
 arm*-wince-pe | arm*-*-mingw32ce*)
 	# Target: ARM based machine running Windows CE (win32)
-	gdb_target_obs="arm-tdep.o arm-wince-tdep.o windows-tdep.o"
+	gdb_target_obs="arm.o arm-tdep.o arm-wince-tdep.o windows-tdep.o"
 	build_gdbserver=yes
 	;;
 arm*-*-linux*)
 	# Target: ARM based machine running GNU/Linux
-	gdb_target_obs="arm-tdep.o arm-linux-tdep.o glibc-tdep.o \
+	gdb_target_obs="arm.o arm-tdep.o arm-linux-tdep.o glibc-tdep.o \
 			solib-svr4.o symfile-mem.o linux-tdep.o linux-record.o"
 	build_gdbserver=yes
 	;;
 arm*-*-netbsd* | arm*-*-knetbsd*-gnu)
 	# Target: NetBSD/arm
-	gdb_target_obs="arm-tdep.o armnbsd-tdep.o solib-svr4.o"
+	gdb_target_obs="arm.o arm-tdep.o armnbsd-tdep.o solib-svr4.o"
 	;;
 arm*-*-openbsd*)
 	# Target: OpenBSD/arm
-	gdb_target_obs="arm-tdep.o armbsd-tdep.o armobsd-tdep.o obsd-tdep.o \
-			solib-svr4.o"
+	gdb_target_obs="arm.o arm-tdep.o armbsd-tdep.o armobsd-tdep.o \
+			obsd-tdep.o solib-svr4.o"
 	;;
 arm*-*-symbianelf*)
 	# Target: SymbianOS/arm
-	gdb_target_obs="arm-tdep.o arm-symbian-tdep.o"
+	gdb_target_obs="arm.o arm-tdep.o arm-symbian-tdep.o"
 	;;
 arm*-*-*)
 	# Target: ARM embedded system
-	gdb_target_obs="arm-tdep.o"
+	gdb_target_obs="arm.o arm-tdep.o"
 	gdb_sim=../sim/arm/libsim.a
 	;;
 
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index d789cfc..d542262 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,14 @@
 2015-10-21  Antoine Tremblay  <antoine.tremblay@ericsson.com>
 
+	* Makefile.in: Add arm.c/o.
+	* configure.srv: Likewise.
+	* linux-arm-low.c (arm_breakpoint_kinds): New enum.
+	(arm_breakpoint_kind_from_pc): New function.
+	(arm_sw_breakpoint_from_kind): Return proper kind.
+	(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize.
+
+2015-10-21  Antoine Tremblay  <antoine.tremblay@ericsson.com>
+
 	* linux-low.c (initialize_low): Ajdust for breakpoint global variables
 	removal.
 	* mem-break.c : Remove breakpoint_data/breakpoint_len global variables.
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index cd146f4..97e1e62 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -180,7 +180,8 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/common-debug.c $(srcdir)/common/cleanups.c \
 	$(srcdir)/common/common-exceptions.c $(srcdir)/symbol.c \
 	$(srcdir)/common/btrace-common.c \
-	$(srcdir)/common/fileio.c $(srcdir)/nat/linux-namespaces.c
+	$(srcdir)/common/fileio.c $(srcdir)/nat/linux-namespaces.c \
+	$(srcdir)/arch/arm.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -583,6 +584,12 @@ fileio.o: ../common/fileio.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
 
+# Arch object files rules form ../arch
+
+arm.o: ../arch/arm.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
+
 # Native object files rules from ../nat
 
 x86-dregs.o: ../nat/x86-dregs.c
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index f187c9d..e854110 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -70,6 +70,7 @@ case "${target}" in
 			srv_regobj="${srv_regobj} arm-with-neon.o"
 			srv_tgtobj="$srv_linux_obj linux-arm-low.o"
 			srv_tgtobj="$srv_tgtobj linux-aarch32-low.o"
+			srv_tgtobj="${srv_tgtobj} arm.o"
 			srv_xmlfiles="arm-with-iwmmxt.xml"
 			srv_xmlfiles="${srv_xmlfiles} arm-with-vfpv2.xml"
 			srv_xmlfiles="${srv_xmlfiles} arm-with-vfpv3.xml"
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 3a56620..d5af8c8 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -30,6 +30,8 @@
 #include "nat/gdb_ptrace.h"
 #include <signal.h>
 
+#include "arch/arm.h"
+
 /* Defined in auto-generated files.  */
 void init_registers_arm (void);
 extern const struct target_desc *tdesc_arm;
@@ -80,6 +82,14 @@ typedef enum
   arm_hwbp_access = 3
 } arm_hwbp_type;
 
+/* Enum describing the different kinds of breakpoints.  */
+enum arm_breakpoint_kinds
+{
+   ARM_BP_KIND_THUMB = 2,
+   ARM_BP_KIND_THUMB2 = 3,
+   ARM_BP_KIND_ARM = 4,
+};
+
 /* Type describing an ARM Hardware Breakpoint Control register value.  */
 typedef unsigned int arm_hwbp_control_t;
 
@@ -237,7 +247,9 @@ arm_set_pc (struct regcache *regcache, CORE_ADDR pc)
 static const unsigned long arm_breakpoint = 0xef9f0001;
 #define arm_breakpoint_len 4
 static const unsigned short thumb_breakpoint = 0xde01;
+#define thumb_breakpoint_len 2
 static const unsigned short thumb2_breakpoint[] = { 0xf7f0, 0xa000 };
+#define thumb2_breakpoint_len 4
 
 /* For new EABI binaries.  We recognize it regardless of which ABI
    is used for gdbserver, so single threaded debugging should work
@@ -913,7 +925,39 @@ arm_regs_info (void)
     return &regs_info_arm;
 }
 
-/* Implementation of linux_target_ops method "sw_breakpoint_from_kind".  */
+/* Implementation of linux_target_ops method "breakpoint_kind_from_pc".
+
+   Determine the type and size of breakpoint to insert at PCPTR.  Uses the
+   program counter value to determine whether a 16-bit or 32-bit breakpoint
+   should be used.  It returns the breakpoint's kind, and adjusts the program
+   counter (if necessary) to point to the actual memory location where the
+   breakpoint should be inserted.  */
+
+static int
+arm_breakpoint_kind_from_pc (CORE_ADDR *pcptr)
+{
+  if (IS_THUMB_ADDR (*pcptr))
+    {
+      gdb_byte buf[2];
+
+      *pcptr = UNMAKE_THUMB_ADDR (*pcptr);
+
+      /* Check whether we are replacing a thumb2 32-bit instruction.  */
+      if ((*the_target->read_memory) (*pcptr, buf, 2) == 0)
+	{
+	  unsigned short inst1 = 0;
+
+	  (*the_target->read_memory) (*pcptr, (gdb_byte *) &inst1, 2);
+	  if (thumb_insn_size (inst1) == 4)
+	    return ARM_BP_KIND_THUMB2;
+	}
+      return ARM_BP_KIND_THUMB;
+    }
+  else
+    return ARM_BP_KIND_ARM;
+}
+
+/*  Implementation of the linux_target_ops method "sw_breakpoint_from_kind".  */
 
 static const gdb_byte *
 arm_sw_breakpoint_from_kind (int kind , int *size)
@@ -924,11 +968,25 @@ arm_sw_breakpoint_from_kind (int kind , int *size)
      clone events, we will never insert a breakpoint, so even a Thumb
      C library will work; so will mixing EABI/non-EABI gdbserver and
      application.  */
+  switch (kind)
+    {
+      case ARM_BP_KIND_THUMB:
+	*size = thumb_breakpoint_len;
+	return (gdb_byte *) &thumb_breakpoint;
+      case ARM_BP_KIND_THUMB2:
+	*size = thumb2_breakpoint_len;
+	return (gdb_byte *) &thumb2_breakpoint;
+      case ARM_BP_KIND_ARM:
+	*size = arm_breakpoint_len;
 #ifndef __ARM_EABI__
-  return (const gdb_byte *) &arm_breakpoint;
+	return (const gdb_byte *) &arm_breakpoint;
 #else
-  return (const gdb_byte *) &arm_eabi_breakpoint;
+	return (const gdb_byte *) &arm_eabi_breakpoint;
 #endif
+      default:
+       return NULL;
+    }
+  return NULL;
 }
 
 struct linux_target_ops the_low_target = {
@@ -939,7 +997,7 @@ struct linux_target_ops the_low_target = {
   NULL, /* fetch_register */
   arm_get_pc,
   arm_set_pc,
-  NULL, /* breakpoint_kind_from_pc */
+  arm_breakpoint_kind_from_pc,
   arm_sw_breakpoint_from_kind,
   arm_reinsert_addr,
   0,