Re: [PATCH 2/4] Match instruction adjusts SP in thumb

[Will Newton <will.newton@linaro.org>]

On 3 July 2014 07:09, Yao Qi <yao@codesourcery.com> wrote:
> This is a refactor patch, that moves matching instructions adjusting
> SP into a new function, thumb_instruction_restores_sp.  The second
> call to thumb_instruction_restores_sp in thumb_in_function_epilogue_p
> is a little different from the original.  The original code matches
> 'POP <registers> without PC', but thumb_in_function_epilogue_p matches
> 'POP <registers> (with and without PC)'.  However, GDB found one
> instruction about return and is scanning the previous instruction,
> which should be an instruction about return too, so the code change
> doesn't affect the functionality.
>
> gdb:
>
> 2014-07-02  Yao Qi  <yao@codesourcery.com>
>
>         * arm-tdep.c (thumb_instruction_restores_sp): New function.
>         (thumb_in_function_epilogue_p): Call
>         thumb_instruction_restores_sp.
> ---
>  gdb/arm-tdep.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)

This patch looks good to me.

> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 0fc7fc1..153ef42 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -685,6 +685,17 @@ thumb2_instruction_changes_pc (unsigned short inst1, unsigned short inst2)
>    return 0;
>  }
>
> +/* Return 1 if the 16-bit Thumb instruction INSN restores SP in
> +   epilogue, 0 otherwise.  */
> +
> +static int
> +thumb_instruction_restores_sp (unsigned short insn)
> +{
> +  return (insn == 0x46bd  /* mov sp, r7 */
> +         || (insn & 0xff80) == 0xb000  /* add sp, imm */
> +         || (insn & 0xfe00) == 0xbc00);  /* pop <registers> */
> +}
> +
>  /* Analyze a Thumb prologue, looking for a recognizable stack frame
>     and frame pointer.  Scan until we encounter a store that could
>     clobber the stack frame unexpectedly, or an unknown instruction.
> @@ -3257,14 +3268,10 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
>         found_return = 1;
>        else if (insn == 0x46f7)  /* mov pc, lr */
>         found_return = 1;
> -      else if (insn == 0x46bd)  /* mov sp, r7 */
> -       found_stack_adjust = 1;
> -      else if ((insn & 0xff80) == 0xb000)  /* add sp, imm */
> -       found_stack_adjust = 1;
> -      else if ((insn & 0xfe00) == 0xbc00)  /* pop <registers> */
> +      else if (thumb_instruction_restores_sp (insn))
>         {
>           found_stack_adjust = 1;
> -         if (insn & 0x0100)  /* <registers> include PC.  */
> +         if ((insn & 0xfe00) == 0xbd00)  /* pop <registers, PC> */
>             found_return = 1;
>         }
>        else if (thumb_insn_size (insn) == 4)  /* 32-bit Thumb-2 instruction */
> @@ -3317,11 +3324,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
>        insn = extract_unsigned_integer (buf, 2, byte_order_for_code);
>        insn2 = extract_unsigned_integer (buf + 2, 2, byte_order_for_code);
>
> -      if (insn2 == 0x46bd)  /* mov sp, r7 */
> -       found_stack_adjust = 1;
> -      else if ((insn2 & 0xff80) == 0xb000)  /* add sp, imm */
> -       found_stack_adjust = 1;
> -      else if ((insn2 & 0xff00) == 0xbc00)  /* pop <registers> without PC */
> +      if (thumb_instruction_restores_sp (insn2))
>         found_stack_adjust = 1;
>        else if (insn == 0xe8bd)  /* ldm.w sp!, <registers> */
>         found_stack_adjust = 1;
> --
> 1.9.0
>



-- 
Will Newton
Toolchain Working Group, Linaro