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

Will Newton
2014-07-03T08:35:29Z

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