[RFA] use frame IDs to detect function calls while stepping

[Mark Kettenis]

   Date: Mon, 1 Mar 2004 22:16:42 -0800
   From: Joel Brobecker <brobecker@gnat.com>

   > The expected behavior for the last "next" command is for GDB
   > to run until the inferior exits:
   > 
   >         (gdb) n
   >         Single stepping until exit from function _start, 
   >         which has no line number information.
   >         
   >         Program exited normally.
   > 
   > Unfortunately, here is what happens. At 0x000105ec, before we do
   > our second "next" command, we are about to execute the following
   > code:
   > 
   >         0x000105ec <_start+100>:        call  0x20950 <exit>
   >         0x000105f0 <_start+104>:        nop 
   > 
   > After two iterations (one for the call insn, and one for the delay
   > slot), GDB lands at the begining of function "exit" at 0x00020950,
   > which is:
   > 
   >         0x00020950 <exit+0>:    sethi  %hi(0xf000), %g1
   >         0x00020954 <exit+4>:    b,a   0x20914 <_PROCEDURE_LINKAGE_TABLE_>
   >         0x00020958 <exit+8>:    nop 
   > 
   > So at this point, the registers window has not been rotated.
   > I don't know if this is the cause for this problem, but at this
   > point GDB is unable to unwind the call stack:
   > 
   >         (gdb) bt
   >         #0  0x00020950 in _PROCEDURE_LINKAGE_TABLE_ ()
   > 
   > (And gets the wrong procedure name as well, but that's a separate
   > issue - although "x /i" does report what I believe is the correct
   > name, strange!).

Actually, it doesn't get the name wrong.  The function "exit" lives in
a shared library.  Since this is the first time you're calling exit(),
you don't end up in the function itself, but in the PLT entry for
"exit".  This PLT entry will invoke the dynamic linker to direct the
program to the real "exit" function [1].  The PLT entry function is
part of the Procedure Linkage Table (PLT) which is usally named
_PROCEDURE_LINKAGE_TABLE.

   > I am looking into the sparc unwinder code right now, to try to
   > understand a bit better the source of the problem.

   I think I found the source of the glitch. I may have the solution
   to fix it, but my little finger is telling that it might be a bit
   too extreme... Maybe MarkK has some comments about this?

It's not "extreme" at all.  Read on...

   What happens is that, at the point when we reach function "exit",
   the FP register is null:

	   (gdb) p /x $fp
	   $2 = 0x0

   The sparc unwinder in sparc_frame_cache() detects this, thinks there is
   something wrong, and aborts early. So, we never unwind the "_start"
   frame, and hence the following frame ID check doesn't notice the
   function call, as it should have in this case:

   +      if (frame_id_eq (get_frame_id (get_prev_frame (get_current_frame ())),
   +                       step_frame_id))
   +        {
   +          /* It's a subroutine call.  */
   +          handle_step_into_function (ecs);
   +          return;
   +        }

   With this example in mind, it seemed to me that the assertion that %fp
   register is not null is unfortunately incorrect. Given that the rest of
   the code in sparc_frame_cache() wasn't using the value of that register,
   I commented out the assertion, and retried.

OK.  Here %fp == 0 is marking the outermost frame.  So what we're
having is that we're effectively calling a frameless function (the PLT
entry) from the outermost frame.  Since this function is frameless, we
shouldn't be looking at %fp, but at %sp.  However, we bail out before
we do so.  So the check is bogus!  

I'm currently testing the attached patch.  If I see no regressions,
I'll check it in.  Is it needed on the branch too?

Mark


[1] It'll also patch things up such that any future calls to exit()
will go directly to the real function.  Pointless of course for
exit(), but this mechanism is used for all calls to functions living
in a shared library.


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* sparc-tdep.c (sparc_frame_cache): Don't bail out if %fp is zero.
	Reorganize code a bit.

Index: sparc-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.c,v
retrieving revision 1.149
diff -u -p -r1.149 sparc-tdep.c
--- sparc-tdep.c 7 Feb 2004 20:40:35 -0000 1.149
+++ sparc-tdep.c 3 Mar 2004 21:11:29 -0000
@@ -615,14 +615,6 @@ sparc_frame_cache (struct frame_info *ne
   cache = sparc_alloc_frame_cache ();
   *this_cache = cache;
 
-  /* In priciple, for normal frames, %fp (%i6) holds the frame
-     pointer, which holds the base address for the current stack
-     frame.  */
-
-  cache->base = frame_unwind_register_unsigned (next_frame, SPARC_FP_REGNUM);
-  if (cache->base == 0)
-    return cache;
-
   cache->pc = frame_func_unwind (next_frame);
   if (cache->pc != 0)
     {
@@ -632,10 +624,18 @@ sparc_frame_cache (struct frame_info *ne
 
   if (cache->frameless_p)
     {
-      /* We didn't find a valid frame, which means that CACHE->base
-	 currently holds the frame pointer for our calling frame.  */
-      cache->base = frame_unwind_register_unsigned (next_frame,
-						    SPARC_SP_REGNUM);
+      /* This function is frameless, so %fp (%i6) holds the frame
+         pointer for our calling frame.  Use %sp (%o6) as this frame's
+         base address.  */
+      cache->base =
+	frame_unwind_register_unsigned (next_frame, SPARC_SP_REGNUM);
+    }
+  else
+    {
+      /* For normal frames, %fp (%i6) holds the frame pointer, the
+         base address for the current stack frame.  */
+      cache->base =
+	frame_unwind_register_unsigned (next_frame, SPARC_FP_REGNUM);
     }
 
   return cache;