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

[Yao Qi <yao at codesourcery dot com>]

On 06/24/2011 06:38 PM, Yao Qi wrote:
>> > I agree with Yao when he says in the PR that there seems to be
>> > some other root cause for the bug.  Shouldn't
>> > thumb_instruction_changes_pc have caught that "b.n" ?
>> > 
>> > 00008160 <fault_isr>:
>> >     8160:    e7fe          b.n    8160 <fault_isr>
>> >     ...
>> > 
>> > 00008164 <reset_isr>:
>> >     8164:    4a05          ldr    r2, [pc, #20]    ; (817c <reset_isr+0x18>)
>> > 
> thumb_instruction_changes_pc can handle "b.n".  AFAICS, the problem is
> in thumb_analyze_prologue.  In thumb_analyze_prologue, there are a lot
> if/else branches, like below,
> 
>       else if ((insn & 0xe000) == 0xe000)  // <-- [1]
> 	{
>           ....
> 	  else if (thumb2_instruction_changes_pc (insn, inst2))
> 	    {
> 	      /* Don't scan past anything that might change control flow.  */
> 	      break;
> 	    }
> 	  else
> 	    {
> 	      /* The optimizer might shove anything into the prologue,
> 		 so we just skip what we don't recognize.  */
> 	      unrecognized_pc = start;
> 	    }
> 
> 	  start += 2;
> 	}
>       else if (thumb_instruction_changes_pc (insn))
> 	{
> 	  /* Don't scan past anything that might change control flow.  */
> 	  break;
> 	}
> 
> The instruction "b.n 8160" is 0xe7fe, so condition check [1] is true,
> and thumb_instruction_changes_pc is unreachable.  This is cause of this
> problem, I doubt.
> 
> 
> The line of code [1] is discussed in this patch
> 
>   [rfa] ARM prologue parsing support for Thumb-2 instructions
>   http://sourceware.org/ml/gdb-patches/2010-10/msg00132.html
> 
> IIUC, condition check [1] is for 32-bit Thumb-2 instructions (I may be
> wrong, of course).  I have an untested patch.
> 

When talking with Terry Guo on HelloGCC workshop last Saturday in
Beijing, it reminds me that I still have a patch pending there, and
forget to ping it.

  http://sourceware.org/ml/gdb-patches/2011-06/msg00370.html

I regression tested this patch on armv7l-unknown-linux-gnueabi with
{-mthumb, -marm}, no new fails.  OK for mainline?

> 	gdb/
> 	* arm-tdep.c (thumb_analyze_prologue): Check condition for 32-bit
> 	Thumb-2 instructions. 
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 2dd8c9e..7f5a0e1 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -832,8 +832,9 @@ 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 ((insn & 0xe000) == 0xe000 && (insn & 0x1800) != 0)
>  	{
> +	  /* 32-bit Thumb-2 instructions.  */
>  	  unsigned short inst2;
>  
>  	  inst2 = read_memory_unsigned_integer (start + 2, 2,

-- 
Yao (éå)