[PATCH RFA] Fix some ARM call dummy problems

[Kevin Buettner <kevinb at redhat dot com>]

The patch below fixes several problems with call dummies on the ARM.
In the testing that I've been doing, it fixes the following failures:

    FAIL: gdb.base/break.exp: backtrace while in called function
    FAIL: gdb.base/callfuncs.exp: continue after stop in call dummy preserves register contents
    FAIL: gdb.base/callfuncs.exp: finish after stop in call dummy preserves register contents
    FAIL: gdb.base/callfuncs.exp: back at main after return from call dummy breakpoint

It also adds the following PASS.  (This test was suppressed when the "back at
main..." test failed.)

    PASS: gdb.base/callfuncs.exp: return after stop in call dummy preserves register contents

The reason for the first and fourth failures listed above were because
GDB for ARM was unable to backtrace through a call dummy.  The second
and third failures were due to the fact that not all of the general
purpose registers were being saved.  Thus, when the inferior function
call returned, the values of some of the registers would be changed.

It was tempting to replace the existing ARM call dummy machinery with
generic call dummies, but I resisted doing this for these reasons:

    1) The changes needed would've been more extensive.  Since it's
       taking a *very* long time to get feedback on ARM patches these
       days, the chances are very good that I will have forgotten
       the details regarding such a patch if there are issues when
       the patch is finally reviewed.
    2) It is unclear to me how Thumb support should be arranged with
       generic call dummies.  By instead fixing the current mechanism,
       I felt there was less danger of breaking the existing Thumb
       support.
    3) There is already some #if 0'd code for supporting generic call
       dummies.  It's not clear to me why this code is not enabled,
       but one possibility is that there were problems with doing it.
       Or, it might simply be uncompleted work.  Regardless, I decided
       it was best to allow the ARM maintainer to complete this work.

I should also note that in the course of simplifying arm_pop_frame(),
I ended up deleting some code which is just plain wrong.  Code that
looks like this:

    sp -= sizeof (CORE_ADDR);

is almost never right.  The CORE_ADDR size in question is the size on
the host, not the target.  Prior to realizing that the special case
containing this code could be deleted, I was rewriting these statements
along the following lines:

    sp -= REGISTER_RAW_SIZE (regnum);

But, after a time, I realized that the special purpose code for
popping a call dummy was superfluous since the code for regular frames
will do the job quite nicely so long as the frame info has been
initialized correctly.  It just so happened that all of the erroneous
``sizeof (CORE_ADDR)'' was contained within this special case.

Okay to commit?

	* arm-tdep.c (arm_init_extra_frame_info): Add special case for
	call dummies.
	(arm_frame_saved_pc): Likewise.
	(arm_push_dummy_frame): Make sure all of the GPRs are saved.
	(arm_pop_frame): Eliminate special case for call dummies.  It
	is no longer needed now that the frame info is being properly
	initialized.

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.19
diff -u -p -r1.19 arm-tdep.c
--- arm-tdep.c	2001/12/05 08:36:01	1.19
+++ arm-tdep.c	2001/12/13 17:39:26
@@ -1009,6 +1009,7 @@ void
 arm_init_extra_frame_info (int fromleaf, struct frame_info *fi)
 {
   int reg;
+  CORE_ADDR sp;
 
   if (fi->next)
     fi->pc = FRAME_SAVED_PC (fi->next);
@@ -1028,6 +1029,13 @@ arm_init_extra_frame_info (int fromleaf,
   else
 #endif
 
+  /* Compute stack pointer for this frame.  We use this value for both the
+     sigtramp and call dummy cases.  */
+  if (!fi->next)
+    sp = read_sp();
+  else
+    sp = fi->next->frame - fi->next->frameoffset + fi->next->framesize;
+
   /* Determine whether or not we're in a sigtramp frame. 
      Unfortunately, it isn't sufficient to test
      fi->signal_handler_caller because this value is sometimes set
@@ -1038,28 +1046,45 @@ arm_init_extra_frame_info (int fromleaf,
      Note: If an ARM IN_SIGTRAMP method ever needs to compare against
      the name of the function, the code below will have to be changed
      to first fetch the name of the function and then pass this name
-     to IN_SIGTRAMP. */
+     to IN_SIGTRAMP.  */
 
   if (SIGCONTEXT_REGISTER_ADDRESS_P () 
       && (fi->signal_handler_caller || IN_SIGTRAMP (fi->pc, 0)))
     {
-      CORE_ADDR sp;
-
-      if (!fi->next)
-	sp = read_sp();
-      else
-	sp = fi->next->frame - fi->next->frameoffset + fi->next->framesize;
-
       for (reg = 0; reg < NUM_REGS; reg++)
 	fi->fsr.regs[reg] = SIGCONTEXT_REGISTER_ADDRESS (sp, fi->pc, reg);
 
       /* FIXME: What about thumb mode? */
       fi->framereg = SP_REGNUM;
-      fi->frame = read_memory_integer (fi->fsr.regs[fi->framereg], 4);
+      fi->frame = read_memory_integer (fi->fsr.regs[fi->framereg],
+                                       REGISTER_RAW_SIZE (fi->framereg));
       fi->framesize = 0;
       fi->frameoffset = 0;
 
     }
+  else if (PC_IN_CALL_DUMMY (fi->pc, sp, fi->frame))
+    {
+      CORE_ADDR rp;
+      CORE_ADDR callers_sp;
+
+      /* Set rp point at the high end of the saved registers.  */
+      rp = fi->frame - REGISTER_SIZE;
+
+      /* Fill in addresses of saved registers.  */
+      fi->fsr.regs[PS_REGNUM] = rp;
+      rp -= REGISTER_RAW_SIZE (PS_REGNUM);
+      for (reg = PC_REGNUM; reg >= 0; reg--)
+	{
+	  fi->fsr.regs[reg] = rp;
+	  rp -= REGISTER_RAW_SIZE (reg);
+	}
+
+      callers_sp = read_memory_integer (fi->fsr.regs[SP_REGNUM],
+                                        REGISTER_RAW_SIZE (SP_REGNUM));
+      fi->framereg = FP_REGNUM;
+      fi->framesize = callers_sp - sp;
+      fi->frameoffset = fi->frame - sp;
+    }
   else
     {
       arm_scan_prologue (fi);
@@ -1105,7 +1130,12 @@ arm_frame_saved_pc (struct frame_info *f
     return generic_read_register_dummy (fi->pc, fi->frame, PC_REGNUM);
   else
 #endif
+  if (PC_IN_CALL_DUMMY (fi->pc, fi->frame - fi->frameoffset, fi->frame))
     {
+      return read_memory_integer (fi->fsr.regs[PC_REGNUM], REGISTER_RAW_SIZE (PC_REGNUM));
+    }
+  else
+    {
       CORE_ADDR pc = arm_find_callers_reg (fi, LR_REGNUM);
       return IS_THUMB_ADDR (pc) ? UNMAKE_THUMB_ADDR (pc) : pc;
     }
@@ -1151,13 +1181,15 @@ arm_push_dummy_frame (void)
      instruction stores the PC, it stores the address of the stm
      instruction itself plus 12.  */
   fp = sp = push_word (sp, prologue_start + 12);
-  sp = push_word (sp, read_register (PC_REGNUM));	/* FIXME: was PS_REGNUM */
-  sp = push_word (sp, old_sp);
-  sp = push_word (sp, read_register (FP_REGNUM));
 
-  for (regnum = 10; regnum >= 0; regnum--)
+  /* Push the processor status.  */
+  sp = push_word (sp, read_register (PS_REGNUM));
+
+  /* Push all 16 registers starting with r15.  */
+  for (regnum = PC_REGNUM; regnum >= 0; regnum--)
     sp = push_word (sp, read_register (regnum));
 
+  /* Update fp (for both Thumb and ARM) and sp.  */
   write_register (FP_REGNUM, fp);
   write_register (THUMB_FP_REGNUM, fp);
   write_register (SP_REGNUM, sp);
@@ -1372,45 +1404,25 @@ arm_push_arguments (int nargs, struct va
   return sp;
 }
 
+/* Pop the current frame.  So long as the frame info has been initialized
+   properly (see arm_init_extra_frame_info), this code works for dummy frames
+   as well as regular frames.  I.e, there's no need to have a special case
+   for dummy frames.  */
 void
 arm_pop_frame (void)
 {
   int regnum;
   struct frame_info *frame = get_current_frame ();
-
-  if (!PC_IN_CALL_DUMMY(frame->pc, frame->frame, read_fp()))
-    {
-      CORE_ADDR old_SP;
+  CORE_ADDR old_SP = frame->frame - frame->frameoffset + frame->framesize;
 
-      old_SP = read_register (frame->framereg);
-      for (regnum = 0; regnum < NUM_REGS; regnum++)
-        if (frame->fsr.regs[regnum] != 0)
-          write_register (regnum,
-		      read_memory_integer (frame->fsr.regs[regnum], 4));
+  for (regnum = 0; regnum < NUM_REGS; regnum++)
+    if (frame->fsr.regs[regnum] != 0)
+      write_register (regnum,
+		  read_memory_integer (frame->fsr.regs[regnum],
+				       REGISTER_RAW_SIZE (regnum)));
 
-      write_register (PC_REGNUM, FRAME_SAVED_PC (frame));
-      write_register (SP_REGNUM, old_SP);
-    }
-  else
-    {
-      CORE_ADDR sp;
-
-      sp = read_register (FP_REGNUM);
-      sp -= sizeof(CORE_ADDR); /* we don't care about this first word */
-
-      write_register (PC_REGNUM, read_memory_integer (sp, 4));
-      sp -= sizeof(CORE_ADDR);
-      write_register (SP_REGNUM, read_memory_integer (sp, 4));
-      sp -= sizeof(CORE_ADDR);
-      write_register (FP_REGNUM, read_memory_integer (sp, 4));
-      sp -= sizeof(CORE_ADDR);
-
-      for (regnum = 10; regnum >= 0; regnum--)
-        {
-          write_register (regnum, read_memory_integer (sp, 4));
-          sp -= sizeof(CORE_ADDR);
-        }
-    }
+  write_register (PC_REGNUM, FRAME_SAVED_PC (frame));
+  write_register (SP_REGNUM, old_SP);
 
   flush_cached_frames ();
 }