[1/2] This patch fixes GDBServer's run control for single stepping

Message ID wwokshlplje0.fsf@ericsson.com
State New, archived
Headers

Commit Message

Antoine Tremblay April 3, 2017, 1:18 p.m. UTC
  Yao Qi writes:

> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
>> I think maybe the best solution would be to abstract only that part of
>> get_next_pc in a function: the if block starting with if
>> (self->has_thumb2_breakpoint) around line 301.
>>
>> And have only that part return the next_pc + the breakpoint kind, this
>> would avoid breaking all the virtual get_next_pc functions just for that
>> case and allow the same code to be used in kind_from_current_state.
>>
>> We'll still redo the work but at least the code will be in one
>> place. WDYT ?
>
> That should be fine, although I am not exactly sure what are you
> going to do.

It looks like this, comments ?:

----
commit 9a8b46f9038e9d3805c8e6ec1bdf0dff1c95c065
Author: Antoine Tremblay <antoine.tremblay@ericsson.com>
Date:   Mon Apr 3 09:13:20 2017 -0400

    refactor get-next-pc
---
 gdb/arch/arm-get-next-pcs.c       | 199 ++++++++++++++++++++++----------------
 gdb/arch/arm-get-next-pcs.h       |   9 ++
 gdb/gdbserver/linux-aarch32-low.c |  46 ++++++++-
 gdb/gdbserver/linux-aarch32-low.h |   3 +
 gdb/gdbserver/linux-arm-low.c     |  21 ----
 gdb/gdbserver/mem-break.c         |  16 ++-
 6 files changed, 188 insertions(+), 106 deletions(-)
  

Comments

Yao Qi April 3, 2017, 3:18 p.m. UTC | #1
Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

> +  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);
> +	}
> +      next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
> +			  (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));

It is incorrect to choose ARM_BP_KIND_THUMB2 if the instruction is
16-bit.  IMO, this function should only tell whether PC is in IT block
nor not.  It shouldn't involve any breakpoint kinds selection.

> +      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);
> +	    }
> +

If all the following instructions' condition is false, breakpoint should
be set on the first instruction out side of IT block.  We can still use
16-bit thumb breakpoint.

> +	  next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
> +			      (MAKE_THUMB_ADDR (pc),
> ARM_BP_KIND_THUMB2));

The same issue.

> +	  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.  */

How do we fall through now?

> +	}
> +      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 archi2tecturally
> +	     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 (std::pair<CORE_ADDR, arm_breakpoint_kinds>
> +			      (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
> +
> +	  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);
> +
> +	  if (itstate != 0 && ((itstate >> 4) & 1) == cond_negated)
> +	    {
> +	      next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
> +				  (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
> +	    }
> +	  else
> +	    {
> +	      next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
> +				  (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB));
> +	    }

Why do you choose breakpoint in this way?

> diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
> index 6e6926a..f3845cf 100644
> --- a/gdb/gdbserver/mem-break.c
> +++ b/gdb/gdbserver/mem-break.c
> @@ -855,7 +855,21 @@ 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);

I read my patch again, but it looks wrong.  If we single-step an
instruction with a state change, like bx or blx, current get_next_pcs
correctly marked the address bit.  However, with the change like this,
we'll get the wrong breakpoint kind.
  
Antoine Tremblay April 3, 2017, 4:56 p.m. UTC | #2
Yao Qi writes:

> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
>> +  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);
>> +	}
>> +      next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
>> +			  (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
>
> It is incorrect to choose ARM_BP_KIND_THUMB2 if the instruction is
> 16-bit.

Good point this does not look correct indeed, a call to
breakpoint_from_pc would be better at this point.

> IMO, this function should only tell whether PC is in IT block
> nor not.  It shouldn't involve any breakpoint kinds selection.
>

Yes I think you're right, I could make it return std::pair<CORE_ADDR,
bool (is_it_block ())>

And use breakpoint_from_pc if true , and BP_KIND_THUMB if false.

>> +      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);
>> +	    }
>> +
>
> If all the following instructions' condition is false, breakpoint should
> be set on the first instruction out side of IT block.  We can still use
> 16-bit thumb breakpoint.
>
>> +	  next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
>> +			      (MAKE_THUMB_ADDR (pc),
>> ARM_BP_KIND_THUMB2));
>
> The same issue.
>
>> +	  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.  */
>
> How do we fall through now?

No breakpoints are added to the vector, so it falls 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 archi2tecturally
>> +	     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 (std::pair<CORE_ADDR, arm_breakpoint_kinds>
>> +			      (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
>> +
>> +	  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);
>> +
>> +	  if (itstate != 0 && ((itstate >> 4) & 1) == cond_negated)
>> +	    {
>> +	      next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
>> +				  (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
>> +	    }
>> +	  else
>> +	    {
>> +	      next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
>> +				  (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB));
>> +	    }
>
> Why do you choose breakpoint in this way?
>

In the case of if (itstate != 0 && ((itstate >> 4) & 1) == cond_negated)
we are in the IT block

But if that is false we're after the IT block so it doesn't need a
thumb2 breakpoint. (See the comment above in the code)

>> diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
>> index 6e6926a..f3845cf 100644
>> --- a/gdb/gdbserver/mem-break.c
>> +++ b/gdb/gdbserver/mem-break.c
>> @@ -855,7 +855,21 @@ 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);
>
> I read my patch again, but it looks wrong.  If we single-step an
> instruction with a state change, like bx or blx, current get_next_pcs
> correctly marked the address bit.  However, with the change like this,
> we'll get the wrong breakpoint kind.

You're right, that's a problem however I think it could be fixed in
arm_breakpoint_kind_from_current_state by adding a case where
placed_address != current_pc in which case the thumb bit would be the
deciding factor rather then arm_is_thumb_mode ()...

I'll resubmit a proper patch with those fixes.
  

Patch

diff --git a/gdb/arch/arm-get-next-pcs.c b/gdb/arch/arm-get-next-pcs.c
index 398dd59a..d967e16 100644
--- a/gdb/arch/arm-get-next-pcs.c
+++ b/gdb/arch/arm-get-next-pcs.c
@@ -22,6 +22,7 @@ 
 #include "common-regcache.h"
 #include "arm.h"
 #include "arm-get-next-pcs.h"
+#include <vector>
 
 /* See arm-get-next-pcs.h.  */
 
@@ -258,6 +259,115 @@  arm_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
   return next_pcs;
 }
 
+/* See arm-get-next-pcs.h.  */
+
+std::vector <std::pair<CORE_ADDR, arm_breakpoint_kinds>>
+thumb_get_next_pcs_raw_itblock
+(CORE_ADDR pc, unsigned short inst1,
+ ULONGEST status, ULONGEST itstate,
+ ULONGEST (*read_mem_uint) (CORE_ADDR memaddr, int len, int byte_order),
+ int byte_order_for_code)
+{
+  std::vector <std::pair <CORE_ADDR, arm_breakpoint_kinds>> next_pcs;
+
+  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);
+	}
+      next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
+			  (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
+      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);
+	    }
+
+	  next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
+			      (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
+	  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 archi2tecturally
+	     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 (std::pair<CORE_ADDR, arm_breakpoint_kinds>
+			      (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
+
+	  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);
+
+	  if (itstate != 0 && ((itstate >> 4) & 1) == cond_negated)
+	    {
+	      next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
+				  (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
+	    }
+	  else
+	    {
+	      next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
+				  (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB));
+	    }
+
+	  return next_pcs;
+	}
+    }
+
+  return next_pcs;
+}
+
 /* Find the next possible PCs for thumb mode.  */
 
 static VEC (CORE_ADDR) *
@@ -300,89 +410,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);
-
-	  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 != 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);
+      std::vector <std::pair <CORE_ADDR, arm_breakpoint_kinds>> itblock_next_pcs
+	= thumb_get_next_pcs_raw_itblock
+	(pc, inst1, status, itstate, 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);
-		}
-
-	      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;
-	    }
-	}
+      for(auto const &nextpc : itblock_next_pcs) {
+	VEC_safe_push (CORE_ADDR, next_pcs, nextpc.first);
+      }
     }
   else if (itstate & 0x0f)
     {
diff --git a/gdb/arch/arm-get-next-pcs.h b/gdb/arch/arm-get-next-pcs.h
index 2300ac1..830844b 100644
--- a/gdb/arch/arm-get-next-pcs.h
+++ b/gdb/arch/arm-get-next-pcs.h
@@ -20,6 +20,7 @@ 
 #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;
@@ -63,4 +64,12 @@  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);
 
+/* Return the next possible PCs after PC if those are in a thumb2 it block.  */
+std::vector <std::pair <CORE_ADDR, arm_breakpoint_kinds>>
+thumb_get_next_pcs_raw_itblock
+(CORE_ADDR pc, unsigned short inst1,
+ ULONGEST status, ULONGEST itstate,
+ ULONGEST (*read_mem_uint) (CORE_ADDR memaddr, int len, int byte_order),
+ int byte_order_for_code);
+
 #endif /* ARM_GET_NEXT_PCS_H */
diff --git a/gdb/gdbserver/linux-aarch32-low.c b/gdb/gdbserver/linux-aarch32-low.c
index 2b710ba..ff4869f 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
 
@@ -287,8 +290,31 @@  arm_breakpoint_kind_from_current_state (CORE_ADDR *pcptr)
 {
   if (arm_is_thumb_mode ())
     {
-      *pcptr = MAKE_THUMB_ADDR (*pcptr);
-      return arm_breakpoint_kind_from_pc (pcptr);
+      struct regcache *regcache = get_thread_regcache (current_thread, 1);
+      CORE_ADDR pc = regcache_read_pc (regcache);
+      ULONGEST status, itstate;
+      unsigned short inst1;
+
+      *pcptr = UNMAKE_THUMB_ADDR (*pcptr);
+
+      inst1 = get_next_pcs_read_memory_unsigned_integer
+	(pc, 2, 0);
+      status = regcache_raw_get_unsigned (regcache, ARM_PS_REGNUM);
+      itstate = ((status >> 8) & 0xfc) | ((status >> 25) & 0x3);
+
+      std::vector <std::pair <CORE_ADDR, arm_breakpoint_kinds>> itblock_next_pcs
+	= thumb_get_next_pcs_raw_itblock
+	(pc, inst1, status, itstate,
+	 get_next_pcs_read_memory_unsigned_integer,
+	 0);
+
+      /* If this PC is in an itblock let thumb_get_next_pcs_raw_itblock
+	 decide the kind.  */
+	for(auto const &nextpc : itblock_next_pcs) {
+	  if (MAKE_THUMB_ADDR (*pcptr) == nextpc.first)
+	    return nextpc.second;
+	}
+	return ARM_BP_KIND_THUMB;
     }
   else
     {
@@ -296,6 +322,22 @@  arm_breakpoint_kind_from_current_state (CORE_ADDR *pcptr)
     }
 }
 
+/* Read memory from the inferiror.
+   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
 initialize_low_arch_aarch32 (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..f3845cf 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -855,7 +855,21 @@  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,