Re: [PATCH] Fix that different function breakpoints are set@same pc address (PR gdb/12703)

[Yao Qi <yao@codesourcery.com>]

On 10/10/2011 10:46 PM, Ulrich Weigand wrote:
>> > -      else if ((insn & 0xe000) == 0xe000)
>> > +      else if ((insn & 0xe000) == 0xe000 && (insn & 0x1800) != 0)
>> >  	{
> Instead of open-coding the check, I think it would be preferable to
> use the thumb_insn_size routine instead.
> 
> Note that there are a number of other places that either already open-
> code the correct check, or -worse- use the same incorrect check as the
> code above:
> 
> - thumb_in_function_epilogue_p
> - thumb_get_next_pc_raw
> - arm_breakpoint_from_pc
> 
> Would you mind converting them all to thumb_insn_size?

Yeah, we should replace these checks around many places with
thumb_insn_size, I agree.  Here is the patch for this purpose.
Regression tested on arm-linux-gnueabi with both -marm and -mthumb.
OK for mainline?

-- 
Yao (齐尧) 

	PR gdb/12703
	* arm-tdep.c (thumb_analyze_prologue): Call thumb_insn_size to check
	whether insn is a 32-bit Thumb-2 instruction.
	(thumb_in_function_epilogue_p): Likewise.
	(thumb_get_next_pc_raw): Likewise.
	(arm_breakpoint_from_pc): Likewise.

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 278e6e9..0db8b5f 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -231,6 +231,8 @@ 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
@@ -836,7 +838,7 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
 	  constant = read_memory_unsigned_integer (loc, 4, byte_order);
 	  regs[bits (insn, 8, 10)] = pv_constant (constant);
 	}
-      else if ((insn & 0xe000) == 0xe000)
+      else if (thumb_insn_size (insn) == 4) /* 32-bit Thumb-2 instructions.  */
 	{
 	  unsigned short inst2;
 
@@ -3093,7 +3095,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
 	  if (insn & 0x0100)  /* <registers> include PC.  */
 	    found_return = 1;
 	}
-      else if ((insn & 0xe000) == 0xe000)  /* 32-bit Thumb-2 instruction */
+      else if (thumb_insn_size (insn) == 4)  /* 32-bit Thumb-2 instruction */
 	{
 	  if (target_read_memory (scan_pc, buf, 2))
 	    break;
@@ -4335,14 +4337,9 @@ thumb_get_next_pc_raw (struct frame_info *frame, CORE_ADDR pc)
       int cond = itstate >> 4;
 
       if (! condition_true (cond, status))
-	{
-	  /* Advance to the next instruction.  All the 32-bit
-	     instructions share a common prefix.  */
-	  if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0)
-	    return MAKE_THUMB_ADDR (pc + 4);
-	  else
-	    return MAKE_THUMB_ADDR (pc + 2);
-	}
+	/* Advance to the next instruction.  All the 32-bit
+	   instructions share a common prefix.  */
+	return MAKE_THUMB_ADDR (pc + thumb_insn_size (inst1));
 
       /* Otherwise, handle the instruction normally.  */
     }
@@ -4376,7 +4373,7 @@ thumb_get_next_pc_raw (struct frame_info *frame, CORE_ADDR pc)
     {
       nextpc = pc_val + (sbits (inst1, 0, 10) << 1);
     }
-  else if ((inst1 & 0xe000) == 0xe000) /* 32-bit instruction */
+  else if (thumb_insn_size (inst1) == 4) /* 32-bit instruction */
     {
       unsigned short inst2;
       inst2 = read_memory_unsigned_integer (pc + 2, 2, byte_order_for_code);
@@ -8473,7 +8470,7 @@ arm_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, int *lenptr)
 	    {
 	      unsigned short inst1;
 	      inst1 = extract_unsigned_integer (buf, 2, byte_order_for_code);
-	      if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0)
+	      if (thumb_insn_size (inst1) == 4)
 		{
 		  *lenptr = tdep->thumb2_breakpoint_size;
 		  return tdep->thumb2_breakpoint;