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

[Joel Brobecker <brobecker@gnat.com>]

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

[aghaaaaa .... With the patch this time, with my thanks to Elena and Daniel]

Hello,

This is a followup on the discussion that took place in the following
thread:

    [RFA] OSF/1 - "next" over prologueless function call
    http://sources.redhat.com/ml/gdb-patches/2003-12/msg00037.html

During the discussion, it appeared that it was better to rely on
frame IDs to detect cases when we stepped inside a function rather
than further adjusting the test that checks whether we landed at
the begining of a function or not.

After a bit of testing on various platforms, I found that only relying
on a test that checks the ID of frame #1 against the step_frame_id was
not sufficient, unfortunately. The sparc-solaris testing revealed 2
regressions.

After careful analysis of the regressions and a bit of dicussion
with Andrew, here is what we found:

  1. We sometimes step levels of function calls down from the point
     where we started stepping. This is to get past the dynsym
     resolve code. So once we get more than one level deep, the
     frame ID test can no longer work.
     
     That was regression #1.

  2. We have a testcase where we try to "next" our way out of a function
     for which we have no line number information. The expected output
     was to run until the end of the program. But instead we stopped
     before.

     It turned out that we were landing inside a shared library
     trampoline after having left the function we were in, so again
     the frame ID check didn't kick in. We didn't know what to do,
     simply stopped there.

     That was regression #2.

Given the current implementation, and the analysis of the regressions,
we determined that the logic should be something like this:

  . If we landed in undebuggable code (identified by lack of symbol
    name), and we're stepping over this kind of code, then:
    
        Run our way out of the function.
  
  . If we are in a shlib call trampoline, then:

        Likewise.
        (This test was already part of the previous check, BTW)

  . If we are in a function called from the function where we started
    stepping, as identified by frame ID unwind, then:

        Likewise.

I tried it and no longer had any regression.

2004-02-05  J. Brobecker  <brobecker@gnat.com>

        * infrun.c (handle_inferior_event): Rewrite the checks detecting
        cases when we stepped inside a function.

Tested on alpha-tru64, pa-hpux, sparc-solaris and x86-linux.
And it also fixes the problem I originally reported.

OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: infrun.c.diff --]
[-- Type: text/plain, Size: 2945 bytes --]

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.134
diff -u -p -r1.134 infrun.c
--- infrun.c	1 Feb 2004 18:05:09 -0000	1.134
+++ infrun.c	4 Feb 2004 17:35:39 -0000
@@ -2482,6 +2482,17 @@ process_event_stop_test:
       return;
     }
 
+  if (ecs->stop_func_name == NULL
+      && step_over_calls == STEP_OVER_UNDEBUGGABLE)
+    {
+      /* We couldn't determine where we stopped, so we just stepped
+         inside undebuggable code.  Since we want to step over this
+         kind of code, we keep going until the inferior returns from
+         the current function.  */
+      handle_step_into_function (ecs);
+      return;
+    }
+
   /* We can't update step_sp every time through the loop, because
      reading the stack pointer would slow down stepping too much.
      But we can update it every time we leave the step range.  */
@@ -2571,15 +2582,44 @@ process_event_stop_test:
       return;
     }
 
-  if (((stop_pc == ecs->stop_func_start	/* Quick test */
-	|| in_prologue (stop_pc, ecs->stop_func_start))
-       && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name))
-      || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, ecs->stop_func_name)
-      || ecs->stop_func_name == 0)
+  if (legacy_frame_p (current_gdbarch))
     {
-      /* It's a subroutine call.  */
-      handle_step_into_function (ecs);
-      return;
+      /* FIXME: brobecker/2004-02-04: The current architecture is still
+         using the legacy frame code, so we prefer not to rely on frame IDs
+         to check whether we just stepped into a function or not.  Some
+         experiments conducted on sparc-solaris before it was converted
+         to the new frame code showed that it could introduce some
+         severe problems.  Once all targets have transitioned to the new
+         frame code, this block can be deleted.  */
+      if (((stop_pc == ecs->stop_func_start	/* Quick test */
+            || in_prologue (stop_pc, ecs->stop_func_start))
+           && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name))
+          || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, ecs->stop_func_name)
+          || ecs->stop_func_name == 0)
+        {
+          /* It's a subroutine call.  */
+          handle_step_into_function (ecs);
+          return;
+        }
+    }
+  else
+    {
+      if (IN_SOLIB_CALL_TRAMPOLINE (stop_pc, ecs->stop_func_name))
+        {
+          /* We landed in a shared library call trampoline, so it
+             is a subroutine call.  */
+          handle_step_into_function (ecs);
+          return;
+        }
+
+      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;
+        }
+
     }
 
   /* We've wandered out of the step range.  */