This is the mail archive of the gdb-cvs@sourceware.org mailing list for the GDB project.
Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]
[binutils-gdb] [ARM] Make thumb2_breakpoint static again

From: Yao Qi <qiyao at sourceware dot org>
To: gdb-cvs at sourceware dot org
Date: 14 Jan 2016 09:37:25 -0000
Subject: [binutils-gdb] [ARM] Make thumb2_breakpoint static again
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1b451dda5f8905b26bafafe00423335d4fffe8dd

commit 1b451dda5f8905b26bafafe00423335d4fffe8dd
Author: Yao Qi <yao.qi@linaro.org>
Date:   Thu Jan 14 09:36:43 2016 +0000

    [ARM] Make thumb2_breakpoint static again
    
    This patch makes thumb2_breakpoint static.  When writing this patch,
    I find the only reason we keep thumb2_breakpoint extern is that it
    is used as an argument passed to arm_gdbserver_get_next_pcs.  However,
    field arm_thumb2_breakpoint is only used in a null check in
    thumb_get_next_pcs_raw, so I wonder why do need to pass thumb2_breakpoint
    to arm_gdbserver_get_next_pcs.
    
    thumb2_breakpoint was added by Daniel Jacobowitz in order to support
    single-step IT block
    https://sourceware.org/ml/gdb-patches/2010-01/msg00624.html  the logic
    there was if we have 32-bit thumb-2 breakpoint defined, we can safely
    single-step IT block, otherwise, we can't.  Daniel didn't want to use
    16-bit thumb BKPT instruction, because it triggers even on instruction
    which should be executed.  Secondly, using 16-bit thumb illegal
    instruction on top of 32-bit thumb instruction may break the meaning of
    original IT blocks, because the other 16-bit can be regarded as an
    instruction.  See more explanations from Daniel's kernel patch
    http://www.spinics.net/lists/arm-kernel/msg80476.html
    
    Let us back to this patch, GDB/GDBserver can safely single step
    IT block if thumb2_breakpoint is defined, but the single step logic
    doesn't have to know the thumb-2 breakpoint instruction.  Only
    breakpoint insertion mechanism decides to use which breakpoint
    instruction.  In the software single step code, instead of pass
    thumb2_breakpoint, we can pass a boolean variable
    has_thumb2_breakpoint indicate whether the target has thumb-2
    breakpoint defined, which is equivalent to the original code.
    
    Regression tested on arm-linux.  No regression.
    
    gdb:
    
    2016-01-14  Yao Qi  <yao.qi@linaro.org>
    
    	* arch/arm-get-next-pcs.c (arm_get_next_pcs_ctor): Change
    	argument arm_thumb2_breakpoint to has_thumb2_breakpoint.
    	(thumb_get_next_pcs_raw): Check has_thumb2_breakpoint
    	instead.
    	* arch/arm-get-next-pcs.h (struct arm_get_next_pcs)
    	<arm_thumb2_breakpoint>: Remove.
    	<has_thumb2_breakpoint>: New field.
    	(arm_get_next_pcs_ctor): Update declaration.
    	* arm-linux-tdep.c (arm_linux_software_single_step): Pass
    	1 to arm_get_next_pcs_ctor.
    	* arm-tdep.c (arm_software_single_step): Pass 0 to
    	arm_get_next_pcs_ctor.
    
    gdb/gdbserver:
    
    2016-01-14  Yao Qi  <yao.qi@linaro.org>
    
    	* linux-aarch32-low.c (thumb2_breakpoint): Make it static.
    	* linux-aarch32-low.h (thumb2_breakpoint): Remove declaration.
    	* linux-arm-low.c (arm_gdbserver_get_next_pcs): Pass 1 to
    	arm_get_next_pcs_ctor.

Diff:
---
 gdb/ChangeLog                     | 15 +++++++++++++++
 gdb/arch/arm-get-next-pcs.c       |  6 +++---
 gdb/arch/arm-get-next-pcs.h       |  7 ++++---
 gdb/arm-linux-tdep.c              |  2 +-
 gdb/arm-tdep.c                    |  2 +-
 gdb/gdbserver/ChangeLog           |  7 +++++++
 gdb/gdbserver/linux-aarch32-low.c |  2 +-
 gdb/gdbserver/linux-aarch32-low.h |  2 --
 gdb/gdbserver/linux-arm-low.c     |  2 +-
 9 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8d35f5f..048ac3e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2016-01-14  Yao Qi  <yao.qi@linaro.org>
+
+	* arch/arm-get-next-pcs.c (arm_get_next_pcs_ctor): Change
+	argument arm_thumb2_breakpoint to has_thumb2_breakpoint.
+	(thumb_get_next_pcs_raw): Check has_thumb2_breakpoint
+	instead.
+	* arch/arm-get-next-pcs.h (struct arm_get_next_pcs)
+	<arm_thumb2_breakpoint>: Remove.
+	<has_thumb2_breakpoint>: New field.
+	(arm_get_next_pcs_ctor): Update declaration.
+	* arm-linux-tdep.c (arm_linux_software_single_step): Pass
+	1 to arm_get_next_pcs_ctor.
+	* arm-tdep.c (arm_software_single_step): Pass 0 to
+	arm_get_next_pcs_ctor.
+
 2016-01-13  Ulrich Weigand  <uweigand@de.ibm.com>
 
 	* MAINTAINERS: Add Andreas Arnez as s390 target maintainer.
diff --git a/gdb/arch/arm-get-next-pcs.c b/gdb/arch/arm-get-next-pcs.c
index 137df1d..6b8f38a 100644
--- a/gdb/arch/arm-get-next-pcs.c
+++ b/gdb/arch/arm-get-next-pcs.c
@@ -30,13 +30,13 @@ arm_get_next_pcs_ctor (struct arm_get_next_pcs *self,
 		       struct arm_get_next_pcs_ops *ops,
 		       int byte_order,
 		       int byte_order_for_code,
-		       const gdb_byte *arm_thumb2_breakpoint,
+		       int has_thumb2_breakpoint,
 		       struct regcache *regcache)
 {
   self->ops = ops;
   self->byte_order = byte_order;
   self->byte_order_for_code = byte_order_for_code;
-  self->arm_thumb2_breakpoint = arm_thumb2_breakpoint;
+  self->has_thumb2_breakpoint = has_thumb2_breakpoint;
   self->regcache = regcache;
 }
 
@@ -297,7 +297,7 @@ thumb_get_next_pcs_raw (struct arm_get_next_pcs *self, CORE_ADDR pc)
      flags, affecting the execution of further instructions, we may
      need to set two breakpoints.  */
 
-  if (self->arm_thumb2_breakpoint != NULL)
+  if (self->has_thumb2_breakpoint)
     {
       if ((inst1 & 0xff00) == 0xbf00 && (inst1 & 0x000f) != 0)
 	{
diff --git a/gdb/arch/arm-get-next-pcs.h b/gdb/arch/arm-get-next-pcs.h
index 895e866..4a0fc16 100644
--- a/gdb/arch/arm-get-next-pcs.h
+++ b/gdb/arch/arm-get-next-pcs.h
@@ -41,8 +41,9 @@ struct arm_get_next_pcs
   int byte_order;
   /* Byte order for code.  */
   int byte_order_for_code;
-  /* Thumb2 breakpoint instruction.  */
-  const gdb_byte *arm_thumb2_breakpoint;
+  /* Whether the target has 32-bit thumb-2 breakpoint defined or
+     not.  */
+  int has_thumb2_breakpoint;
   /* Registry cache.  */
   struct regcache *regcache;
 };
@@ -52,7 +53,7 @@ void arm_get_next_pcs_ctor (struct arm_get_next_pcs *self,
 			    struct arm_get_next_pcs_ops *ops,
 			    int byte_order,
 			    int byte_order_for_code,
-			    const gdb_byte *arm_thumb2_breakpoint,
+			    int has_thumb2_breakpoint,
 			    struct regcache *regcache);
 
 /* Find the next possible PCs after the current instruction executes.  */
diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 2870bd1..6050bdf 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -931,7 +931,7 @@ arm_linux_software_single_step (struct frame_info *frame)
 			 &arm_linux_get_next_pcs_ops,
 			 gdbarch_byte_order (gdbarch),
 			 gdbarch_byte_order_for_code (gdbarch),
-			 gdbarch_tdep (gdbarch)->thumb2_breakpoint,
+			 1,
 			 regcache);
 
   next_pcs = arm_get_next_pcs (&next_pcs_ctx, regcache_read_pc (regcache));
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 05d60bb..8874ec8 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -6183,7 +6183,7 @@ arm_software_single_step (struct frame_info *frame)
 			 &arm_get_next_pcs_ops,
 			 gdbarch_byte_order (gdbarch),
 			 gdbarch_byte_order_for_code (gdbarch),
-			 gdbarch_tdep (gdbarch)->thumb2_breakpoint,
+			 0,
 			 regcache);
 
   next_pcs = arm_get_next_pcs (&next_pcs_ctx, regcache_read_pc (regcache));
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 757424c..a76bb14 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,10 @@
+2016-01-14  Yao Qi  <yao.qi@linaro.org>
+
+	* linux-aarch32-low.c (thumb2_breakpoint): Make it static.
+	* linux-aarch32-low.h (thumb2_breakpoint): Remove declaration.
+	* linux-arm-low.c (arm_gdbserver_get_next_pcs): Pass 1 to
+	arm_get_next_pcs_ctor.
+
 2016-01-12  Josh Stone  <jistone@redhat.com>
 	    Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
diff --git a/gdb/gdbserver/linux-aarch32-low.c b/gdb/gdbserver/linux-aarch32-low.c
index ed66e08..2bbbb24 100644
--- a/gdb/gdbserver/linux-aarch32-low.c
+++ b/gdb/gdbserver/linux-aarch32-low.c
@@ -45,7 +45,7 @@ static const unsigned long arm_breakpoint = arm_abi_breakpoint;
 #define arm_breakpoint_len 4
 static const unsigned short thumb_breakpoint = 0xde01;
 #define thumb_breakpoint_len 2
-const unsigned short thumb2_breakpoint[] = { 0xf7f0, 0xa000 };
+static const unsigned short thumb2_breakpoint[] = { 0xf7f0, 0xa000 };
 #define thumb2_breakpoint_len 4
 
 /* Some older versions of GNU/Linux and Android do not define
diff --git a/gdb/gdbserver/linux-aarch32-low.h b/gdb/gdbserver/linux-aarch32-low.h
index 5c68454..434a523 100644
--- a/gdb/gdbserver/linux-aarch32-low.h
+++ b/gdb/gdbserver/linux-aarch32-low.h
@@ -15,8 +15,6 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-extern const unsigned short thumb2_breakpoint[];
-
 extern struct regs_info regs_info_aarch32;
 
 void arm_fill_gregset (struct regcache *regcache, void *buf);
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index d967e58..927a6fa 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -942,7 +942,7 @@ arm_gdbserver_get_next_pcs (CORE_ADDR pc, struct regcache *regcache)
 			 /* Byte order is ignored assumed as host.  */
 			 0,
 			 0,
-			 (const gdb_byte *) &thumb2_breakpoint,
+			 1,
 			 regcache);
 
   next_pcs = arm_get_next_pcs (&next_pcs_ctx, pc);
Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]