This is the mail archive of the gdb-patches@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]

[PATCH] Only use 32-bit thumb-2 breakpoint instruction if necessary


It takes two PTRACE_POKETEXT calls to write a 32-bit thumb-2 breakpoint to
a 2-byte aligned and non 4-byte aligned address, and other threads can
observe the partially modified instruction in the memory between these two
calls.  This causes problems on single stepping multi-thread program in
GDBserver and GDB.

32-bit thumb-2 breakpoint was invented for single stepping 32-bit thumb-2
instructions in an IT block see:
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-January/007488.html
but we can use the 16-bit thumb breakpoint instruction anywhere else.
That is what this patch does.  The change in set_breakpoint_type_at is similar
to breakpoint.c:breakpoint_kind.

Note also that this patch changes the breakpoint kinds used both in GDB and
GDBServer not only to fix the multi-threading issue but also in order to
keep them in sync.  Otherwise if GDBServer needed to install a software
single step on a GDB breakpoint the breakpoint kinds would not match and
the breakpoint would be removed.

This patch fixes fails in gdb.threads/schedlock.exp in thumb mode.

Even with this patch, 32-bit thumb-2 breakpoint is still used for 32-bit thumb-2
instructions in IT block, so the problem is still there.  This patch is a
partial fix to PR server/21169.

This is marked as a known fail in the new test called arm-single-step.exp.

The fixed portion of this problem is also tested by that test and without
the corresponding fix, it fails quickly consistently very quickly.

No regressions, tested on ubuntu 14.04 ARMv7 and x86.
With gdb, gdbserver-{native,extended} / { -marm -mthumb }

gdb/ChangeLog:

	PR server/21169
	* arch/arm-get-next-pcs.c:(thumb_get_next_pcs_raw_itblock):
	New function.
	(thumb_get_next_pcs_raw_itblock_1): New function.
	(thumb_get_next_pcs_raw): Move thumb2 IT block next pc logic
	to thumb_get_next_pcs_raw_itblock_1.
	* arch/arm-get-next-pcs.h: Include vector.
	(read_mem_uint_ftype): New typedef.
	(struct arm_get_next_pcs_ops): Use read_mem_uint_ftype typedef.
	(struct nextpc_itblock): New struct.
	(thumb_get_next_pcs_raw_itblock_1): New declaration.
	(thumb_get_next_pcs_raw_itblock): New declaration.
	* arm-tdep.c (arm_breakpoint_kind_from_pc): Use 16-bit breakpoints
	unless the instruction is in an IT block.
	(sefltest::arm_get_next_pcs_tests): New namespace.
	(sefltest::arm_get_next_pcs_tests::thumb_it_block_test):
	New namespace.
	(thumb_it_block_test::test): New function.
	(_initialize_arm_tdep): Register
	selftests::arm_get_next_pcs_tests::thumb_it_block_test::test function.
	(thumb_it_block_test::insns, insns_size): New variables.
	(thumb_it_block_test::read_mem_uint): New function.
	(thumb_it_block_test::test): New function.

gdb/gdbserver/ChangeLog:

	PR server/21169
	* linux-aarch32-low.c: Include arm-get-next-pcs.h and vector.
	(arm_breakpoint_kind_from_current_state): Use
	thumb_get_next_pcs_raw_itblock to install a thumb-2 breakpoint
	only in an IT block.
	(get_next_pcs_read_memory_unsigned_integer): Move from linux-arm-low.c.
	* linux-aarch32-low.h (get_next_pcs_read_memory_unsigned_integer):
	New declaration.
	* linux-arm-low.c (get_next_pcs_read_memory_unsigned_integer):
	Moved to linux-aarch32-low.c.
	* mem-break.c (set_breakpoint_type_at): Call
	target_breakpoint_kind_from_current_state to get breakpoint kind
	for single_step breakpoint.

gdb/testsuite/ChangeLog:

	PR server/21169
	* gdb.threads/arm-single-step.c: New file
	* gdb.threads/arm-single-step.exp: New file
---
 gdb/arch/arm-get-next-pcs.c                   | 206 +++++++++++++--------
 gdb/arch/arm-get-next-pcs.h                   |  32 +++-
 gdb/arm-tdep.c                                | 254 +++++++++++++++++++++++++-
 gdb/gdbserver/linux-aarch32-low.c             |  53 +++++-
 gdb/gdbserver/linux-aarch32-low.h             |   3 +
 gdb/gdbserver/linux-arm-low.c                 |  21 ---
 gdb/gdbserver/mem-break.c                     |  11 +-
 gdb/testsuite/gdb.threads/arm-single-step.c   | 106 +++++++++++
 gdb/testsuite/gdb.threads/arm-single-step.exp |  53 ++++++
 9 files changed, 624 insertions(+), 115 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/arm-single-step.c
 create mode 100644 gdb/testsuite/gdb.threads/arm-single-step.exp

diff --git a/gdb/arch/arm-get-next-pcs.c b/gdb/arch/arm-get-next-pcs.c
index 398dd59a..c9be7e0 100644
--- a/gdb/arch/arm-get-next-pcs.c
+++ b/gdb/arch/arm-get-next-pcs.c
@@ -258,6 +258,125 @@ arm_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
   return next_pcs;
 }
 
+/* See arm-get-next-pcs.h.  */
+
+std::vector<nextpc_itblock>
+thumb_get_next_pcs_raw_itblock (struct regcache *regcache,
+				read_mem_uint_ftype read_mem_uint,
+				int byte_order_for_code)
+{
+  CORE_ADDR pc = regcache_read_pc (regcache);
+  ULONGEST status = regcache_raw_get_unsigned (regcache, ARM_PS_REGNUM);
+  ULONGEST itstate = ((status >> 8) & 0xfc) | ((status >> 25) & 0x3);
+
+  return thumb_get_next_pcs_raw_itblock_1 (pc, status, itstate,
+					   read_mem_uint,
+					   byte_order_for_code);
+}
+
+/* See arm-get-next-pcs.h.  */
+
+std::vector<nextpc_itblock>
+thumb_get_next_pcs_raw_itblock_1 (CORE_ADDR pc, ULONGEST status,
+				  ULONGEST itstate,
+				  read_mem_uint_ftype read_mem_uint,
+				  int byte_order_for_code)
+{
+  std::vector<nextpc_itblock> next_pcs;
+  unsigned short inst1 = read_mem_uint (pc, 2, byte_order_for_code);
+
+  if ((inst1 & 0xff00) == 0xbf00 && (inst1 & 0x000f) != 0)
+    {
+      /* An IT instruction.  Because this instruction does not
+	 modify the flags, we can accurately predict the next
+	 executed instruction.  */
+      itstate = inst1 & 0x00ff;
+      pc += thumb_insn_size (inst1);
+
+      while (itstate != 0 && ! condition_true (itstate >> 4, status))
+	{
+	  inst1 = read_mem_uint (pc, 2,byte_order_for_code);
+	  pc += thumb_insn_size (inst1);
+	  itstate = thumb_advance_itstate (itstate);
+	}
+      /* The instruction is after the itblock if itstate != 0.  */
+      next_pcs.push_back
+	(nextpc_itblock { MAKE_THUMB_ADDR (pc), itstate != 0 });
+      return next_pcs;
+    }
+  else if (itstate != 0)
+    {
+      /* We are in a conditional block.  Check the condition.  */
+      if (! condition_true (itstate >> 4, status))
+	{
+	  /* Advance to the next executed instruction.  */
+	  pc += thumb_insn_size (inst1);
+	  itstate = thumb_advance_itstate (itstate);
+
+	  while (itstate != 0 && ! condition_true (itstate >> 4, status))
+	    {
+	      inst1 = read_mem_uint (pc, 2, byte_order_for_code);
+
+	      pc += thumb_insn_size (inst1);
+	      itstate = thumb_advance_itstate (itstate);
+	    }
+
+	  /* The instruction is after the itblock if itstate != 0.  */
+	  next_pcs.push_back
+	    (nextpc_itblock { MAKE_THUMB_ADDR (pc), itstate != 0 });
+	  return next_pcs;
+	}
+      else if ((itstate & 0x0f) == 0x08)
+	{
+	  /* This is the last instruction of the conditional
+	     block, and it is executed.  We can handle it normally
+	     because the following instruction is not conditional,
+	     and we must handle it normally because it is
+	     permitted to branch.  Fall through.  */
+	}
+      else
+	{
+	  int cond_negated;
+
+	  /* There are conditional instructions after this one.
+	     If this instruction modifies the flags, then we can
+	     not predict what the next executed instruction will
+	     be.  Fortunately, this instruction is architecturally
+	     forbidden to branch; we know it will fall through.
+	     Start by skipping past it.  */
+	  pc += thumb_insn_size (inst1);
+	  itstate = thumb_advance_itstate (itstate);
+
+	  /* Set a breakpoint on the following instruction.  */
+	  gdb_assert ((itstate & 0x0f) != 0);
+	  next_pcs.push_back (nextpc_itblock { MAKE_THUMB_ADDR (pc), true });
+
+	  cond_negated = (itstate >> 4) & 1;
+
+	  /* Skip all following instructions with the same
+	     condition.  If there is a later instruction in the IT
+	     block with the opposite condition, set the other
+	     breakpoint there.  If not, then set a breakpoint on
+	     the instruction after the IT block.  */
+	  do
+	    {
+	      inst1 = read_mem_uint (pc, 2, byte_order_for_code);
+	      pc += thumb_insn_size (inst1);
+	      itstate = thumb_advance_itstate (itstate);
+	    }
+	  while (itstate != 0 && ((itstate >> 4) & 1) == cond_negated);
+
+	  /* The instruction is after the itblock if itstate != 0.  */
+	  next_pcs.push_back
+	    (nextpc_itblock { MAKE_THUMB_ADDR (pc), itstate != 0 });
+
+	  return next_pcs;
+	}
+    }
+
+  return next_pcs;
+}
+
 /* Find the next possible PCs for thumb mode.  */
 
 static VEC (CORE_ADDR) *
@@ -300,89 +419,14 @@ thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
 
   if (self->has_thumb2_breakpoint)
     {
-      if ((inst1 & 0xff00) == 0xbf00 && (inst1 & 0x000f) != 0)
-	{
-	  /* An IT instruction.  Because this instruction does not
-	     modify the flags, we can accurately predict the next
-	     executed instruction.  */
-	  itstate = inst1 & 0x00ff;
-	  pc += thumb_insn_size (inst1);
+      auto itblock_next_pcs = thumb_get_next_pcs_raw_itblock
+	(regcache, self->ops->read_mem_uint, byte_order_for_code);
 
-	  while (itstate != 0 && ! condition_true (itstate >> 4, status))
-	    {
-	      inst1 = self->ops->read_mem_uint (pc, 2,byte_order_for_code);
-	      pc += thumb_insn_size (inst1);
-	      itstate = thumb_advance_itstate (itstate);
-	    }
+      for (const auto &it_nextpc : itblock_next_pcs)
+	VEC_safe_push (CORE_ADDR, next_pcs, it_nextpc.nextpc);
 
-	  VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc));
-	  return next_pcs;
-	}
-      else if (itstate != 0)
-	{
-	  /* We are in a conditional block.  Check the condition.  */
-	  if (! condition_true (itstate >> 4, status))
-	    {
-	      /* Advance to the next executed instruction.  */
-	      pc += thumb_insn_size (inst1);
-	      itstate = thumb_advance_itstate (itstate);
-
-	      while (itstate != 0 && ! condition_true (itstate >> 4, status))
-		{
-		  inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
-
-		  pc += thumb_insn_size (inst1);
-		  itstate = thumb_advance_itstate (itstate);
-		}
-
-	      VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc));
-	      return next_pcs;
-	    }
-	  else if ((itstate & 0x0f) == 0x08)
-	    {
-	      /* This is the last instruction of the conditional
-		 block, and it is executed.  We can handle it normally
-		 because the following instruction is not conditional,
-		 and we must handle it normally because it is
-		 permitted to branch.  Fall through.  */
-	    }
-	  else
-	    {
-	      int cond_negated;
-
-	      /* There are conditional instructions after this one.
-		 If this instruction modifies the flags, then we can
-		 not predict what the next executed instruction will
-		 be.  Fortunately, this instruction is architecturally
-		 forbidden to branch; we know it will fall through.
-		 Start by skipping past it.  */
-	      pc += thumb_insn_size (inst1);
-	      itstate = thumb_advance_itstate (itstate);
-
-	      /* Set a breakpoint on the following instruction.  */
-	      gdb_assert ((itstate & 0x0f) != 0);
-	      VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc));
-
-	      cond_negated = (itstate >> 4) & 1;
-
-	      /* Skip all following instructions with the same
-		 condition.  If there is a later instruction in the IT
-		 block with the opposite condition, set the other
-		 breakpoint there.  If not, then set a breakpoint on
-		 the instruction after the IT block.  */
-	      do
-		{
-		  inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
-		  pc += thumb_insn_size (inst1);
-		  itstate = thumb_advance_itstate (itstate);
-		}
-	      while (itstate != 0 && ((itstate >> 4) & 1) == cond_negated);
-
-	      VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc));
-
-	      return next_pcs;
-	    }
-	}
+      if (!itblock_next_pcs.empty ())
+	return next_pcs;
     }
   else if (itstate & 0x0f)
     {
diff --git a/gdb/arch/arm-get-next-pcs.h b/gdb/arch/arm-get-next-pcs.h
index 2300ac1..cc8a6a7 100644
--- a/gdb/arch/arm-get-next-pcs.h
+++ b/gdb/arch/arm-get-next-pcs.h
@@ -20,14 +20,18 @@
 #ifndef ARM_GET_NEXT_PCS_H
 #define ARM_GET_NEXT_PCS_H 1
 #include "gdb_vecs.h"
+#include <vector>
 
 /* Forward declaration.  */
 struct arm_get_next_pcs;
 
+typedef ULONGEST (*read_mem_uint_ftype) (CORE_ADDR memaddr,
+					 int len, int byte_order);
+
 /* get_next_pcs operations.  */
 struct arm_get_next_pcs_ops
 {
-  ULONGEST (*read_mem_uint) (CORE_ADDR memaddr, int len, int byte_order);
+  read_mem_uint_ftype read_mem_uint;
   CORE_ADDR (*syscall_next_pc) (struct arm_get_next_pcs *self);
   CORE_ADDR (*addr_bits_remove) (struct arm_get_next_pcs *self, CORE_ADDR val);
   int (*is_thumb) (struct arm_get_next_pcs *self);
@@ -52,6 +56,16 @@ struct arm_get_next_pcs
   struct regcache *regcache;
 };
 
+/* Contains the CORE_ADDR and if it's in an IT block.
+   To be returned by thumb_get_next_pcs_raw_itblock.  */
+struct nextpc_itblock
+{
+  /* Next PC.  */
+  CORE_ADDR nextpc;
+  /* Is this PC in an IT block.  */
+  bool in_itblock;
+};
+
 /* Initialize arm_get_next_pcs.  */
 void arm_get_next_pcs_ctor (struct arm_get_next_pcs *self,
 			    struct arm_get_next_pcs_ops *ops,
@@ -63,4 +77,20 @@ void arm_get_next_pcs_ctor (struct arm_get_next_pcs *self,
 /* Find the next possible PCs after the current instruction executes.  */
 VEC (CORE_ADDR) *arm_get_next_pcs (struct arm_get_next_pcs *self);
 
+/* This is a subfunction of thumb_get_next_pcs_raw_itblock it is used to
+   provide a unit testable interface to
+   thumb_get_next_pcs_raw_itblock.  */
+
+std::vector<nextpc_itblock>
+thumb_get_next_pcs_raw_itblock_1 (CORE_ADDR pc, ULONGEST status,
+				  ULONGEST itstate,
+				  read_mem_uint_ftype read_mem_uint,
+				  int byte_order_for_code);
+
+/* Return the next possible PCs after and if those are in a thumb2 it
+   block. */
+std::vector<nextpc_itblock> thumb_get_next_pcs_raw_itblock
+(struct regcache *regcache, read_mem_uint_ftype read_mem_uint,
+ int byte_order_for_code);
+
 #endif /* ARM_GET_NEXT_PCS_H */
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index b3c3705..6133b75 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -7837,11 +7837,18 @@ arm_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
 
   if (arm_pc_is_thumb (gdbarch, *pcptr))
     {
+      /* Check if we are in an IT block.  */
+      CORE_ADDR adjusted_addr
+	= arm_adjust_breakpoint_address(gdbarch, *pcptr);
+
       *pcptr = UNMAKE_THUMB_ADDR (*pcptr);
 
-      /* If we have a separate 32-bit breakpoint instruction for Thumb-2,
-	 check whether we are replacing a 32-bit instruction.  */
-      if (tdep->thumb2_breakpoint != NULL)
+      /* If we have a separate 32-bit breakpoint instruction for Thumb-2
+	 check whether we are replacing a 32-bit instruction.
+
+	 Also check that the instruction at PCPTR is in an IT block.  This
+	 is needed to keep GDB and GDBServer breakpoint kinds in sync.  */
+      if (adjusted_addr != *pcptr && tdep->thumb2_breakpoint != NULL)
 	{
 	  gdb_byte buf[2];
 
@@ -9592,7 +9599,14 @@ arm_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
 namespace selftests
 {
 static void arm_record_test (void);
-}
+namespace arm_get_next_pcs_tests
+{
+namespace thumb_it_block_test
+{
+static void test (void);
+} /* namespace thumb_it_block_test */
+} /* namespace arm_get_next_pcs_tests */
+} /* namespace selftests */
 
 extern initialize_file_ftype _initialize_arm_tdep; /* -Wmissing-prototypes */
 
@@ -9733,6 +9747,8 @@ vfp - VFP co-processor."),
 
 #if GDB_SELF_TEST
   register_self_test (selftests::arm_record_test);
+  register_self_test
+    (selftests::arm_get_next_pcs_tests::thumb_it_block_test::test);
 #endif
 
 }
@@ -13211,7 +13227,235 @@ arm_record_test (void)
     SELF_CHECK (arm_record.arm_regs[0] == 7);
   }
 }
-} // namespace selftests
+
+
+/* Namespace for arm_get_gext_pcs test series.  */
+namespace arm_get_next_pcs_tests
+{
+namespace thumb_it_block_test
+{
+/* Pointer to instruction array to test.  */
+static const gdb_byte *insns;
+/* Size of the instruction array.  */
+static int insns_size;
+
+/* Mockup read function for the test function.  */
+static ULONGEST
+read_mem_uint (CORE_ADDR memaddr, int len, int byte_order)
+{
+  gdb_byte buf[sizeof (ULONGEST)];
+
+  if (memaddr + len > insns_size || len > sizeof (ULONGEST))
+    return 0;
+
+  memcpy (buf, insns + memaddr, len);
+  return extract_unsigned_integer (buf, len, (enum bfd_endian) byte_order);
+}
+
+/* Test getting the next PCs when dealing with an IT block.  */
+
+static void
+test ()
+{
+  /* Instructions with an IT block and an instruction after the IT block.  */
+  const gdb_byte itte_insns[] = {
+    /* itte eq */
+    0x06, 0xbf,
+    /* nopeq */
+    0x00, 0xbf,
+    /* nopeq */
+    0x00, 0xbf,
+    /* nopne */
+    0x00, 0xbf,
+    /* nop */
+    0x00, 0xbf,
+  };
+
+  insns = itte_insns;
+  insns_size = sizeof (itte_insns);
+
+  /* Test that if PC is at IT instruction the next PC is the first one
+     that validates the condition in the IT block.  */
+  {
+    CORE_ADDR pc = 0;
+    /* Simulate CMP that is Equal.  */
+    ULONGEST status = FLAG_Z;
+    ULONGEST itstate = insns[0] & 0x00ff;
+    auto next_pcs = thumb_get_next_pcs_raw_itblock_1
+      (pc, status, itstate, read_mem_uint, 1);
+
+    SELF_CHECK (next_pcs.size () == 1);
+    SELF_CHECK (next_pcs.back ().nextpc == MAKE_THUMB_ADDR (0x02));
+    SELF_CHECK (next_pcs.back ().in_itblock);
+  }
+
+  /* Test that if PC is in an IT block and the instruction at PC does not
+     validate the condition that the next PC is the one that does validate
+     the condition.  */
+  {
+    CORE_ADDR pc = 0x2;
+    /* Simulate CMP that Not Equal.  */
+    ULONGEST status = ~FLAG_Z;
+    ULONGEST itstate = insns[0] & 0x00ff;
+    auto next_pcs = thumb_get_next_pcs_raw_itblock_1
+      (pc, status, itstate, read_mem_uint, 1);
+
+    SELF_CHECK (next_pcs.size () == 1);
+    SELF_CHECK (next_pcs.back ().nextpc == MAKE_THUMB_ADDR (0x06));
+    SELF_CHECK (next_pcs.back ().in_itblock);
+  }
+
+  /* Test that if PC is at the last instruction of an IT block and it is
+     executed.  No next PC is returned from
+     thumb_get_next_pcs_raw_itblock_1.  */
+  {
+    CORE_ADDR pc = 0x6;
+    /* Simulate CMP that is Not Equal.  */
+    ULONGEST status = ~FLAG_Z;
+    ULONGEST itstate = insns[0] & 0x00ff;
+
+    /* Get the itstate for the last instruction.  */
+    for (int i = 0; i < 2; i++)
+      itstate = thumb_advance_itstate (itstate);
+
+    auto next_pcs = thumb_get_next_pcs_raw_itblock_1
+      (pc, status, itstate, read_mem_uint, 1);
+
+    SELF_CHECK (next_pcs.empty ());
+  }
+
+  /* Test that if PC is at the last instruction of an IT block and it is
+     not executed.  The next PC is after the itblock.  */
+  {
+    CORE_ADDR pc = 0x6;
+    /* Simulate CMP that is Equal.  */
+    ULONGEST status = FLAG_Z;
+    ULONGEST itstate = insns[0] & 0x00ff;
+
+    /* Get the itstate for the last instruction.  */
+    for (int i = 0; i < 2; i++)
+      itstate = thumb_advance_itstate (itstate);
+
+    auto next_pcs = thumb_get_next_pcs_raw_itblock_1
+      (pc, status, itstate, read_mem_uint, 1);
+
+    SELF_CHECK (next_pcs.size () == 1);
+    SELF_CHECK (next_pcs.back ().nextpc == MAKE_THUMB_ADDR (0x8));
+    SELF_CHECK (!next_pcs.back ().in_itblock);
+  }
+
+  /* Test that if PC is at the last instruction of an IT block and it is
+     executed.  No next_pc is returned from
+     thumb_get_next_pcs_raw_itblock_1.  */
+  {
+    CORE_ADDR pc = 0x6;
+    /* Simulate CMP that Not Equal.  */
+    ULONGEST status = ~FLAG_Z;
+    ULONGEST itstate = insns[0] & 0x00ff;
+
+    /* Get the itstate for the last instruction.  */
+    for (int i = 0; i < 2; i++)
+      itstate = thumb_advance_itstate (itstate);
+
+    auto next_pcs = thumb_get_next_pcs_raw_itblock_1
+      (pc, status, itstate, read_mem_uint, 1);
+
+    SELF_CHECK (next_pcs.empty ());
+  }
+
+  /* Test that if PC is stopped at an instruction that validates the
+     condition, the next PCs returned are the next instruction and the next
+     instruction that does not validate the condition, since the
+     instruction at PC could modify the flags.  */
+  {
+    CORE_ADDR pc = 0x2;
+    /* Simulate CMP that Equal.  */
+    ULONGEST status = FLAG_Z;
+    ULONGEST itstate = insns[0] & 0x00ff;
+
+    auto next_pcs = thumb_get_next_pcs_raw_itblock_1
+      (0x2, status, itstate, read_mem_uint, 1);
+
+    SELF_CHECK (next_pcs.size () == 2);
+    SELF_CHECK (next_pcs.front ().nextpc == MAKE_THUMB_ADDR (0x4));
+    SELF_CHECK (next_pcs.front ().in_itblock);
+    SELF_CHECK (next_pcs.back ().nextpc == MAKE_THUMB_ADDR (0x6));
+    SELF_CHECK (next_pcs.back ().in_itblock);
+  }
+
+  /* Instructions with IT block created by an itt instruction and an
+     instruction after the IT block.  */
+  const gdb_byte itt_insns[] = {
+    /* itt eq */
+    0x04, 0xbf,
+    /* nopeq */
+    0x00, 0xbf,
+    /* nopeq */
+    0x00, 0xbf,
+    /* nop */
+    0x00, 0xbf,
+  };
+
+  insns = itt_insns;
+  insns_size = sizeof (itt_insns);
+
+  /* Test that if PC is at IT instruction and there's no instruction that
+     validates the condition in the IT block then the next PC is outside
+     of the it block.  */
+  {
+    CORE_ADDR pc = 0;
+    /* Simulate CMP that is Not Equal.  */
+    ULONGEST status = ~FLAG_Z;
+    ULONGEST itstate = insns[0] & 0x00ff;
+    auto next_pcs = thumb_get_next_pcs_raw_itblock_1
+      (pc, status, itstate, read_mem_uint, 1);
+
+    SELF_CHECK (next_pcs.size () == 1);
+    SELF_CHECK (next_pcs.back ().nextpc == MAKE_THUMB_ADDR (0x06));
+    SELF_CHECK (!next_pcs.back ().in_itblock);
+  }
+
+  /* Test that if PC is in an IT block and the instruction at PC does not
+     validate the condition and that there is no instruction that does in
+     the IT block that the next instruction is outside of the IT
+     block.  */
+  {
+    CORE_ADDR pc = 0x2;
+    /* Simulate CMP that Not Equal.  */
+    ULONGEST status = ~FLAG_Z;
+    ULONGEST itstate = insns[0] & 0x00ff;
+    auto next_pcs = thumb_get_next_pcs_raw_itblock_1
+      (pc, status, itstate, read_mem_uint, 1);
+
+    SELF_CHECK (next_pcs.size () == 1);
+    SELF_CHECK (next_pcs.back ().nextpc == MAKE_THUMB_ADDR (0x06));
+    SELF_CHECK (!next_pcs.back ().in_itblock);
+  }
+
+  /* Test that if PC is stopped at an instruction that validates the
+     condition the next PCs returned are the next instruction and the
+     instruction after the IT block since in this case there's no
+     instruction that would not validate the condition in the IT
+     block.  */
+  {
+
+    CORE_ADDR pc = 0x2;
+    /* Simulate CMP that is Equal.  */
+    ULONGEST status = FLAG_Z;
+    ULONGEST itstate = insns[0] & 0x00ff;
+    auto next_pcs = thumb_get_next_pcs_raw_itblock_1
+      (pc, status, itstate, read_mem_uint, 1);
+
+    SELF_CHECK (next_pcs.size () == 2);
+    SELF_CHECK (next_pcs.front ().nextpc == MAKE_THUMB_ADDR (0x4));
+    SELF_CHECK (next_pcs.front ().in_itblock);
+    SELF_CHECK (next_pcs.back ().nextpc == MAKE_THUMB_ADDR (0x6));
+    SELF_CHECK (!next_pcs.back ().in_itblock);
+  }
+}
+} /* namespace thumb_it_block_test */
+} /* namespace arm_thumb_get_pcs_tests */
+} /* namespace selftests */
 #endif /* GDB_SELF_TEST */
 
 /* Cleans up local record registers and memory allocations.  */
diff --git a/gdb/gdbserver/linux-aarch32-low.c b/gdb/gdbserver/linux-aarch32-low.c
index 2b710ba..30b7d1a 100644
--- a/gdb/gdbserver/linux-aarch32-low.c
+++ b/gdb/gdbserver/linux-aarch32-low.c
@@ -18,6 +18,7 @@
 #include "server.h"
 #include "arch/arm.h"
 #include "arch/arm-linux.h"
+#include "arch/arm-get-next-pcs.h"
 #include "linux-low.h"
 #include "linux-aarch32-low.h"
 
@@ -28,6 +29,8 @@
 #include <elf.h>
 #endif
 
+#include <vector>
+
 /* Correct in either endianness.  */
 #define arm_abi_breakpoint 0xef9f0001UL
 
@@ -285,15 +288,53 @@ arm_sw_breakpoint_from_kind (int kind , int *size)
 int
 arm_breakpoint_kind_from_current_state (CORE_ADDR *pcptr)
 {
-  if (arm_is_thumb_mode ())
+  struct regcache *regcache = get_thread_regcache (current_thread, 1);
+  CORE_ADDR pc = regcache_read_pc (regcache);
+
+  /* Two cases here:
+
+     - If GDBServer is not single stepping then the PC is the current PC
+     and the PC doesn't contain the THUMB bit, even if it is a THUMB
+     instruction.
+
+     - If single stepping, PCPTR is the next expected PC.  In this case
+     PCPTR contains the THUMB bit if needed.  GDBServer should not rely on
+     arm_is_thumb_mode in that case but only on the THUMB bit in the
+     PCPTR.  */
+  if (((pc == *pcptr) && arm_is_thumb_mode ()) || IS_THUMB_ADDR (*pcptr))
     {
-      *pcptr = MAKE_THUMB_ADDR (*pcptr);
-      return arm_breakpoint_kind_from_pc (pcptr);
+      auto itblock_next_pcs = thumb_get_next_pcs_raw_itblock
+	(regcache, get_next_pcs_read_memory_unsigned_integer, 0);
+
+      /* If this PC is in an itblock let arm_breakpoint_kind_from_pc
+	 decide the kind.  Otherwise always use a 2 byte breakpoint.  */
+      for (const auto &nextpc : itblock_next_pcs)
+	{
+	  if (MAKE_THUMB_ADDR (*pcptr) == nextpc.nextpc && nextpc.in_itblock)
+	    return arm_breakpoint_kind_from_pc (pcptr);
+	}
+
+      *pcptr = UNMAKE_THUMB_ADDR (*pcptr);
+      return ARM_BP_KIND_THUMB;
     }
   else
-    {
-      return arm_breakpoint_kind_from_pc (pcptr);
-    }
+    return arm_breakpoint_kind_from_pc (pcptr);
+}
+
+/* Read memory from the inferior.
+   BYTE_ORDER is ignored and there to keep compatiblity with GDB's
+   read_memory_unsigned_integer. */
+ULONGEST
+get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
+					   int len,
+					   int byte_order)
+{
+  ULONGEST res;
+
+  res = 0;
+  target_read_memory (memaddr, (unsigned char *) &res, len);
+
+  return res;
 }
 
 void
diff --git a/gdb/gdbserver/linux-aarch32-low.h b/gdb/gdbserver/linux-aarch32-low.h
index fecfcbe..77fca32 100644
--- a/gdb/gdbserver/linux-aarch32-low.h
+++ b/gdb/gdbserver/linux-aarch32-low.h
@@ -27,6 +27,9 @@ int arm_breakpoint_kind_from_pc (CORE_ADDR *pcptr);
 const gdb_byte *arm_sw_breakpoint_from_kind (int kind , int *size);
 int arm_breakpoint_kind_from_current_state (CORE_ADDR *pcptr);
 int arm_breakpoint_at (CORE_ADDR where);
+ULONGEST get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
+						    int len,
+						    int byte_order);
 
 void initialize_low_arch_aarch32 (void);
 
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index fc2b348..fc6b5cc 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -139,11 +139,6 @@ static int arm_regmap[] = {
   64
 };
 
-/* Forward declarations needed for get_next_pcs ops.  */
-static ULONGEST get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
-							   int len,
-							   int byte_order);
-
 static CORE_ADDR get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self,
 						CORE_ADDR val);
 
@@ -252,22 +247,6 @@ get_next_pcs_is_thumb (struct arm_get_next_pcs *self)
   return arm_is_thumb_mode ();
 }
 
-/* Read memory from the inferiror.
-   BYTE_ORDER is ignored and there to keep compatiblity with GDB's
-   read_memory_unsigned_integer. */
-static ULONGEST
-get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
-					   int len,
-					   int byte_order)
-{
-  ULONGEST res;
-
-  res = 0;
-  target_read_memory (memaddr, (unsigned char *) &res, len);
-
-  return res;
-}
-
 /* Fetch the thread-local storage pointer for libthread_db.  */
 
 ps_err_e
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index 6e6926a..1e04063 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -855,7 +855,16 @@ set_breakpoint_type_at (enum bkpt_type type, CORE_ADDR where,
 {
   int err_ignored;
   CORE_ADDR placed_address = where;
-  int breakpoint_kind = target_breakpoint_kind_from_pc (&placed_address);
+  int breakpoint_kind;
+
+  /* Get the kind of breakpoint to PLACED_ADDRESS except single-step
+     breakpoint.  Get the kind of single-step breakpoint according to
+     the current register state.  */
+  if (type == single_step_breakpoint)
+    breakpoint_kind
+      = target_breakpoint_kind_from_current_state (&placed_address);
+  else
+    breakpoint_kind = target_breakpoint_kind_from_pc (&placed_address);
 
   return set_breakpoint (type, raw_bkpt_type_sw,
 			 placed_address, breakpoint_kind, handler,
diff --git a/gdb/testsuite/gdb.threads/arm-single-step.c b/gdb/testsuite/gdb.threads/arm-single-step.c
new file mode 100644
index 0000000..9783f33
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/arm-single-step.c
@@ -0,0 +1,106 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017 Free Software Foundation, Inc.
+
+   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 <pthread.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <signal.h>
+
+#define NUM_THREADS 2
+#define TIMEOUT 5
+
+static pthread_t child_thread[NUM_THREADS];
+static volatile int run = 1;
+
+static void
+handler (int signo)
+{
+  run = 0;
+}
+
+/* Misalign a 4 bytes instruction (the 2nd nop) on purpose.  Force it to
+   be at an address that is a multiple of 2, but not 4.  */
+#define THUMB2_INST				\
+  asm ("	.align 4 \n"			\
+       "	nop.n \n"				\
+       "	nop.w \n"			\
+       )					\
+
+#define ITBLOCK						\
+  asm ("	.align 4 \n"				\
+       "	nop\n"					\
+       "	cmp r0, #0\n"				\
+       "	itte eq\n"				\
+       "	nop.w \n"				\
+       "	nop.w \n"				\
+       "	nop.w \n"					\
+       "	nop \n"					\
+       )						\
+
+#define LOOP(macro)				\
+  while (run)				\
+      macro;			\
+
+static void
+out_of_loop ()
+{
+  return;
+}
+
+static void *
+thumb2_function (void *arg)
+{
+  LOOP (THUMB2_INST); /* break thumb2 */
+  out_of_loop ();
+  pthread_exit (NULL);
+}
+
+void *
+itblock_function (void *arg)
+{
+  LOOP (ITBLOCK); /* break itblock */
+  out_of_loop ();
+  pthread_exit (NULL);
+}
+
+int
+main (void)
+{
+  int res;
+  int i, j;
+  void *(*functions[2]) (void *);
+
+  functions[0] = thumb2_function;
+  functions[1] = itblock_function;
+
+  signal (SIGALRM, handler);
+
+  for (i = 0; i < sizeof (functions); i++)
+    {
+      alarm (TIMEOUT);
+
+      for (j = 0; j < NUM_THREADS; j++)
+	res = pthread_create (&child_thread[j], NULL, functions[i], NULL);
+
+      for (j = 0; j < NUM_THREADS; j++)
+	res = pthread_join (child_thread[j], NULL);
+
+      run = 1;
+    }
+
+  exit (EXIT_SUCCESS);
+}
diff --git a/gdb/testsuite/gdb.threads/arm-single-step.exp b/gdb/testsuite/gdb.threads/arm-single-step.exp
new file mode 100644
index 0000000..0e97184
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/arm-single-step.exp
@@ -0,0 +1,53 @@
+# Copyright 2017 Free Software Foundation, Inc.
+
+# 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/>.
+
+# This test checks that GDBServer doesn't crash the inferior while writing
+# a breakpoint at an address that is aligned on 2 bytes but not on 4
+# bytes.
+# This file tests the partial resolution of PR server/21169.
+
+if {![istarget arm*-*eabi*]} then {
+    verbose "Skipping Thumb-2 threaded arm single-step tests."
+    return
+}
+
+standard_testfile
+set executable ${testfile}
+
+if { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested "failed to compile"
+    return -1
+}
+
+clean_restart $executable
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test_no_output "set scheduler-locking off"
+
+# thumb2 is a misaligned 4 bytes instruction (the 2nd nop) it's forced to
+# be at an address that is a multiple of 2, but not 4.
+# itblock is the same but in an itblock.
+
+foreach_with_prefix inst { "thumb2" "itblock" } {
+    gdb_breakpoint [gdb_get_line_number "break $inst"] "temporary"
+    gdb_continue_to_breakpoint "break thumb2" ".* break $inst .*"
+    if { $inst == "itblock" } {
+	setup_kfail "server/21169" *-*-*
+    }
+    gdb_test "step" ".*out_of_loop.*" "stepping $inst"
+}
-- 
2.7.4


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]