[2/4] Match instruction adjusts SP in thumb

Message ID 1404367792-23234-3-git-send-email-yao@codesourcery.com
State New, archived
Headers

Commit Message

Yao Qi July 3, 2014, 6:09 a.m. UTC
  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(-)
  

Comments

Will Newton July 3, 2014, 8:35 a.m. UTC | #1
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
>
  
Joel Brobecker July 11, 2014, 1:25 p.m. UTC | #2
> 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.

OK.
  

Patch

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;