[RFA] More tweaks to arm_skip_prologue

[RFA] More tweaks to arm_skip_prologue

[Michael Snyder]

I know that some of these tweaks to arm_skip_prologue will also
suggest similar tweaks to arm_scan_prologue.  I'll do those next.
Wouldn't it be nice if the two shared code?    ;-)

2002-04-22  Michael Snyder  <msnyder@redhat.com>

	* arm-tdep.c (arm_skip_prologue): Better handling for frameless 
	functions.  Treat "mov ip, sp" as optional.  Recognize 
	"str lr, [sp, #-nn]".

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.51
diff -p -r1.51 arm-tdep.c
*** arm-tdep.c	22 Apr 2002 23:22:04 -0000	1.51
--- arm-tdep.c	23 Apr 2002 00:00:30 -0000
*************** arm_skip_prologue (CORE_ADDR pc)
*** 446,467 ****
       by disassembling the instructions. */
    skip_pc = pc;
    inst = read_memory_integer (skip_pc, 4);
!   if (inst != 0xe1a0c00d)	/* mov ip, sp */
!     return pc;
  
!   skip_pc += 4;
!   inst = read_memory_integer (skip_pc, 4);
!   if ((inst & 0xfffffff0) == 0xe92d0000)	/* stmfd sp!,{a1,a2,a3,a4}  */
      {
        skip_pc += 4;
        inst = read_memory_integer (skip_pc, 4);
      }
  
!   if ((inst & 0xfffff800) != 0xe92dd800)	/* stmfd sp!,{...,fp,ip,lr,pc} */
!     return pc;
  
!   skip_pc += 4;
!   inst = read_memory_integer (skip_pc, 4);
  
    /* Any insns after this point may float into the code, if it makes
       for better instruction scheduling, so we skip them only if we
--- 446,475 ----
       by disassembling the instructions. */
    skip_pc = pc;
    inst = read_memory_integer (skip_pc, 4);
!   if (inst == 0xe1a0c00d)       /* mov ip, sp */
!     {
!       skip_pc += 4;
!       inst = read_memory_integer (skip_pc, 4);
!     }
  
!   /* Some prologues begin with "str lr, [sp, #-nn]".  */
!   if ((inst & 0xffffff00) == 0xe52de000)        /* str lr, [sp, #-nn] */
      {
        skip_pc += 4;
        inst = read_memory_integer (skip_pc, 4);
      }
  
!   if ((inst & 0xfffffff0) == 0xe92d0000)	/* stmfd sp!,{a1,a2,a3,a4}  */
!     {
!       skip_pc += 4;
!       inst = read_memory_integer (skip_pc, 4);
!     }
  
!   if ((inst & 0xfffff800) == 0xe92dd800)	/* stmfd sp!,{fp,ip,lr,pc} */
!     {
!       skip_pc += 4;
!       inst = read_memory_integer (skip_pc, 4);
!     }
  
    /* Any insns after this point may float into the code, if it makes
       for better instruction scheduling, so we skip them only if we

Re: [RFA] More tweaks to arm_skip_prologue

[Richard Earnshaw]

> 
> I know that some of these tweaks to arm_skip_prologue will also
> suggest similar tweaks to arm_scan_prologue.  I'll do those next.
> Wouldn't it be nice if the two shared code?    ;-)
> 
> 2002-04-22  Michael Snyder  <msnyder@redhat.com>
> 
> 	* arm-tdep.c (arm_skip_prologue): Better handling for frameless 
> 	functions.  Treat "mov ip, sp" as optional.  Recognize 
> 	"str lr, [sp, #-nn]".
> 
In principal OK, but see embedded notes.

>        by disassembling the instructions. */
>     skip_pc = pc;
>     inst = read_memory_integer (skip_pc, 4);
> !   if (inst == 0xe1a0c00d)       /* mov ip, sp */
> !     {
> !       skip_pc += 4;
> !       inst = read_memory_integer (skip_pc, 4);
> !     }

If the sequence doesn't start with mov ip, sp then we either have a 
scheduled prologue where the first instruction is messing with 
call-clobbered register, or we have a frameless prologue.  I suspect that 
if are in this situation then we should use a different unwind function to 
keep things simpler.

>   
> !   /* Some prologues begin with "str lr, [sp, #-nn]".  */
> !   if ((inst & 0xffffff00) == 0xe52de000)        /* str lr, [sp, #-nn] */
>       {
>         skip_pc += 4;
>         inst = read_memory_integer (skip_pc, 4);
>       }

This should only ever be "str lr, [sp, #-4]!" (note the writeback).  What 
about an "stmfd sp!, {...., lr}" (non-frame) prologue instruction.

>   
> !   if ((inst & 0xfffffff0) == 0xe92d0000)	/* stmfd sp!,{a1,a2,a3,a4}  */
> !     {
> !       skip_pc += 4;
> !       inst = read_memory_integer (skip_pc, 4);
> !     }
>   
> !   if ((inst & 0xfffff800) == 0xe92dd800)	/* stmfd sp!,{fp,ip,lr,pc} */
> !     {
> !       skip_pc += 4;
> !       inst = read_memory_integer (skip_pc, 4);
> !     }
>   

R.

Re: [RFA] More tweaks to arm_skip_prologue

[Michael Snyder]

Richard Earnshaw wrote:
> 
> >
> > I know that some of these tweaks to arm_skip_prologue will also
> > suggest similar tweaks to arm_scan_prologue.  I'll do those next.
> > Wouldn't it be nice if the two shared code?    ;-)
> >
> > 2002-04-22  Michael Snyder  <msnyder@redhat.com>
> >
> >       * arm-tdep.c (arm_skip_prologue): Better handling for frameless
> >       functions.  Treat "mov ip, sp" as optional.  Recognize
> >       "str lr, [sp, #-nn]".
> >
> In principal OK, but see embedded notes.
> 
> >        by disassembling the instructions. */
> >     skip_pc = pc;
> >     inst = read_memory_integer (skip_pc, 4);
> > !   if (inst == 0xe1a0c00d)       /* mov ip, sp */
> > !     {
> > !       skip_pc += 4;
> > !       inst = read_memory_integer (skip_pc, 4);
> > !     }
> 
> If the sequence doesn't start with mov ip, sp then we either have a
> scheduled prologue where the first instruction is messing with
> call-clobbered register, or we have a frameless prologue.  I suspect that
> if are in this situation then we should use a different unwind function to
> keep things simpler.

You know way more about the architecture than I do, but
check arm_scan_prologue -- it already does the same thing.
I'd like to bring them into sync, and then think about
possibly making them smarter.

> 
> >
> > !   /* Some prologues begin with "str lr, [sp, #-nn]".  */
> > !   if ((inst & 0xffffff00) == 0xe52de000)        /* str lr, [sp, #-nn] */
> >       {
> >         skip_pc += 4;
> >         inst = read_memory_integer (skip_pc, 4);
> >       }
> 
> This should only ever be "str lr, [sp, #-4]!" (note the writeback). 

OK, I'll correct that.

> What about an "stmfd sp!, {...., lr}" (non-frame) prologue instruction.

Can you give me a pattern to match for?
I haven't actually seen that instruction in a prologue.

Re: [RFA] More tweaks to arm_skip_prologue

[Richard Earnshaw]

> > If the sequence doesn't start with mov ip, sp then we either have a
> > scheduled prologue where the first instruction is messing with
> > call-clobbered register, or we have a frameless prologue.  I suspect that
> > if are in this situation then we should use a different unwind function to
> > keep things simpler.
> 
> You know way more about the architecture than I do, but
> check arm_scan_prologue -- it already does the same thing.
> I'd like to bring them into sync, and then think about
> possibly making them smarter.
> 

I was thinking more along the lines of (in pseudo code)

	arm_skip_prologue (...)
	{
	  if (first_instruction == "mov ip, lr")
	    skip_atpcs_frame_prologue (...)
	  else
	    skip_prologue_maybe_frameless (...)

Maybe we could even make the sub-functions common to the two existing uses.


> > What about an "stmfd sp!, {...., lr}" (non-frame) prologue instruction.
> 
> Can you give me a pattern to match for?
> I haven't actually seen that instruction in a prologue.

Try compiling the following with -O2 -fomit-frame-pointer.

void f (int *);
void h (int);
void g (int a, int b)
{
  f(&a);
  h(b);
}

You should get something like:

_g:
        stmfd   sp!, {r4, lr}  @ Prologue
        sub     sp, sp, #4     @ Prologue
        str     r0, [sp, #0]   @ Prologue
        mov     r4, r1         @ [note 1]
        mov     r0, sp
        bl      _f
        mov     r0, r4
        bl      _h
        add     sp, sp, #4
        ldmfd   sp!, {r4, pc}

[1]  We should probably consider this instruction as part of the prologue 
as well, but it isn't clear we can detect this reliably (ie not get any 
false positives).  It's possible that we could switch on any mov from 
r0-r3.

R.

Re: [RFA] More tweaks to arm_skip_prologue

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 234 bytes --]

Richard Earnshaw wrote:
[...]
> This should only ever be "str lr, [sp, #-4]!" (note the writeback).  

Richard, how's this revised patch?

Do you think that, if we detect the str lr, [sp, -4]!
we should just return pc + 4 immediately?

[-- Attachment #2: frameless.patch --]
[-- Type: text/plain, Size: 2176 bytes --]

2002-04-22  Michael Snyder  <msnyder@redhat.com>

	* arm-tdep.c (arm_skip_prologue): Better handling for frameless 
	functions.  Treat "mov ip, sp" as optional.  Recognize 
	"str lr, [sp, #-4]".

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.51
diff -p -r1.51 arm-tdep.c
*** arm-tdep.c	22 Apr 2002 23:22:04 -0000	1.51
--- arm-tdep.c	23 Apr 2002 00:00:30 -0000
*************** arm_skip_prologue (CORE_ADDR pc)
*** 446,467 ****
       by disassembling the instructions. */
    skip_pc = pc;
    inst = read_memory_integer (skip_pc, 4);
!   if (inst != 0xe1a0c00d)	/* mov ip, sp */
!     return pc;
  
!   skip_pc += 4;
!   inst = read_memory_integer (skip_pc, 4);
!   if ((inst & 0xfffffff0) == 0xe92d0000)	/* stmfd sp!,{a1,a2,a3,a4}  */
      {
        skip_pc += 4;
        inst = read_memory_integer (skip_pc, 4);
      }
  
!   if ((inst & 0xfffff800) != 0xe92dd800)	/* stmfd sp!,{...,fp,ip,lr,pc} */
!     return pc;
  
!   skip_pc += 4;
!   inst = read_memory_integer (skip_pc, 4);
  
    /* Any insns after this point may float into the code, if it makes
       for better instruction scheduling, so we skip them only if we
--- 446,475 ----
       by disassembling the instructions. */
    skip_pc = pc;
    inst = read_memory_integer (skip_pc, 4);
!   if (inst == 0xe1a0c00d)       /* mov ip, sp */
!     {
!       skip_pc += 4;
!       inst = read_memory_integer (skip_pc, 4);
!     }
  
!   /* Some prologues begin with "str lr, [sp, #-4]!".  */
!   if (inst == 0xe52de004)		        /* str lr, [sp, #-nn]! */
      {
        skip_pc += 4;
        inst = read_memory_integer (skip_pc, 4);
      }
  
!   if ((inst & 0xfffffff0) == 0xe92d0000)	/* stmfd sp!,{a1,a2,a3,a4}  */
!     {
!       skip_pc += 4;
!       inst = read_memory_integer (skip_pc, 4);
!     }
  
!   if ((inst & 0xfffff800) == 0xe92dd800)	/* stmfd sp!,{fp,ip,lr,pc} */
!     {
!       skip_pc += 4;
!       inst = read_memory_integer (skip_pc, 4);
!     }
  
    /* Any insns after this point may float into the code, if it makes
       for better instruction scheduling, so we skip them only if we

Re: [RFA] More tweaks to arm_skip_prologue

[Richard Earnshaw]

> Richard Earnshaw wrote:
> [...]
> > This should only ever be "str lr, [sp, #-4]!" (note the writeback).  
> 
> Richard, how's this revised patch?
> 
> Do you think that, if we detect the str lr, [sp, -4]!
> we should just return pc + 4 immediately?

Well, there could be some stack allocation (and maybe some stores to it), 
there might, in theory, be some floating point stacking as well.

> 
> 2002-04-22  Michael Snyder  <msnyder@redhat.com>
> 
> 	* arm-tdep.c (arm_skip_prologue): Better handling for frameless 
> 	functions.  Treat "mov ip, sp" as optional.  Recognize 
> 	"str lr, [sp, #-4]".

Working on the principal that this is better than what we have, I think 
this should go in.  More needs to be done, but I don't think it should 
block this change.

See, for example, the test case I posted in my previous message.

Re: [RFA] More tweaks to arm_skip_prologue

[Michael Snyder]

Richard Earnshaw wrote:
> 
> > Richard Earnshaw wrote:
> > [...]
> > > This should only ever be "str lr, [sp, #-4]!" (note the writeback).
> >
> > Richard, how's this revised patch?
> >
> > Do you think that, if we detect the str lr, [sp, -4]!
> > we should just return pc + 4 immediately?
> 
> Well, there could be some stack allocation (and maybe some stores to it),
> there might, in theory, be some floating point stacking as well.
> 
> >
> > 2002-04-22  Michael Snyder  <msnyder@redhat.com>
> >
> >       * arm-tdep.c (arm_skip_prologue): Better handling for frameless
> >       functions.  Treat "mov ip, sp" as optional.  Recognize
> >       "str lr, [sp, #-4]".
> 
> Working on the principal that this is better than what we have, I think
> this should go in.  More needs to be done, but I don't think it should
> block this change.

Great, that's what I hoped for.
I will be doing some more work on this code, so if you 
want to continue sending suggestions...    ;-)

Re: [RFA] More tweaks to arm_skip_prologue

[Michael Snyder]

Richard Earnshaw wrote:
> 
> > Richard Earnshaw wrote:
> > [...]
> > > This should only ever be "str lr, [sp, #-4]!" (note the writeback).
> >
> > Richard, how's this revised patch?
> >
> > Do you think that, if we detect the str lr, [sp, -4]!
> > we should just return pc + 4 immediately?
> 
> Well, there could be some stack allocation (and maybe some stores to it),
> there might, in theory, be some floating point stacking as well.
> 
> >
> > 2002-04-22  Michael Snyder  <msnyder@redhat.com>
> >
> >       * arm-tdep.c (arm_skip_prologue): Better handling for frameless
> >       functions.  Treat "mov ip, sp" as optional.  Recognize
> >       "str lr, [sp, #-4]".
> 
> Working on the principal that this is better than what we have, I think
> this should go in.  More needs to be done, but I don't think it should
> block this change.

Committed