[patch/rfc,6.1?] Use right frame ID in step_over_function

[patch/rfc,6.1?] Use right frame ID in step_over_function

[Andrew Cagney]

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

Hello,

This goes into the "how did it ever work" category.  The idea of 
step_over_function is that it:

- finds the caller's resume address
- finds the caller's frame ID

and then sets a breakpoint for that caller instance of the function. 
The current code:

- finds the caller's resume address
- finds the _callee_ frame ID

and then uses that to set the breakpoint.  Now that is plain weird!  It 
only works because either:

- the step_frame_id patches up the bug

- the values match as GDB is using the inner-most, rather than 
outer-most frame address as part of the frame ID

The bug apepars when trying to step over nested shared library non-debug 
info functions (making sense?).

I'll follow this up after 6.1 branch is in place.

Its pretty heavy a change to apply to that branch and this late. 
However, like Joel's related patch, I suspect it will be needed :-/

Andrew

PS: Why do I have this feeling of dejavu?

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 2150 bytes --]

	* infrun.c (step_over_function): When non-legacy code, and no
	step_frame_id, use the unwinder to get the caller's frame ID.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.137
diff -u -r1.137 infrun.c
--- infrun.c	16 Feb 2004 20:49:51 -0000	1.137
+++ infrun.c	29 Feb 2004 04:10:59 -0000
@@ -2930,6 +2930,7 @@
 step_over_function (struct execution_control_state *ecs)
 {
   struct symtab_and_line sr_sal;
+  struct frame_id sr_id;
 
   init_sal (&sr_sal);		/* initialize to zeros */
 
@@ -2973,13 +2974,29 @@
   sr_sal.section = find_pc_overlay (sr_sal.pc);
 
   check_for_old_step_resume_breakpoint ();
-  step_resume_breakpoint =
-    set_momentary_breakpoint (sr_sal, get_frame_id (get_current_frame ()),
-			      bp_step_resume);
 
   if (frame_id_p (step_frame_id)
       && !IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc))
-    step_resume_breakpoint->frame_id = step_frame_id;
+    /* NOTE: cagney/2004-02-27: Use the global state's idea of the
+       stepping frame ID.  I suspect this is done as it is lighter
+       weight than a call to get_prev_frame.  */
+    sr_id = step_frame_id;
+  else if (legacy_frame_p (current_gdbarch))
+    /* NOTE: cagney/2004-02-27: This is the way it was 'cos this is
+       the way it always was.  It should be using the unwound (or
+       caller's) ID, and not this (or the callee's) ID.  It appeared
+       to work because: legacy architectures used the wrong end of the
+       frame for the ID.stack (inner-most rather than outer-most) so
+       that the callee's id.stack (un adjusted) matched the caller's
+       id.stack giving the "correct" id; more often than not
+       !IN_SOLIB_DYNSYM_RESOLVE_CODE and hence the code above (it was
+       originally later in the function) fixed the ID by using global
+       state.  */
+    sr_id = get_frame_id (get_current_frame ());
+  else
+    sr_id = get_frame_id (get_prev_frame (get_current_frame ()));
+
+  step_resume_breakpoint = set_momentary_breakpoint (sr_sal, sr_id, bp_step_resume);
 
   if (breakpoints_inserted)
     insert_breakpoints ();

Re: [patch/rfc,6.1?] Use right frame ID in step_over_function

[Daniel Jacobowitz]

On Sat, Feb 28, 2004 at 11:33:51PM -0500, Andrew Cagney wrote:
> Hello,
> 
> This goes into the "how did it ever work" category.  The idea of 
> step_over_function is that it:
> 
> - finds the caller's resume address
> - finds the caller's frame ID
> 
> and then sets a breakpoint for that caller instance of the function. 
> The current code:
> 
> - finds the caller's resume address
> - finds the _callee_ frame ID
> 
> and then uses that to set the breakpoint.  Now that is plain weird!  It 
> only works because either:
> 
> - the step_frame_id patches up the bug
> 
> - the values match as GDB is using the inner-most, rather than 
> outer-most frame address as part of the frame ID
> 
> The bug apepars when trying to step over nested shared library non-debug 
> info functions (making sense?).

No, not really.  Could you give us a testcase?  What platform have you
seen this behavior on?

> I'll follow this up after 6.1 branch is in place.
> 
> Its pretty heavy a change to apply to that branch and this late. 
> However, like Joel's related patch, I suspect it will be needed :-/
> 
> Andrew
> 
> PS: Why do I have this feeling of dejavu?

Because we discussed this problem in July 2003 and neither of us had
time to come back to the issue?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch/rfc,6.1?] Use right frame ID in step_over_function

[Andrew Cagney]

>>The bug apepars when trying to step over nested shared library non-debug 
>>> info functions (making sense?).
> 
> 
> No, not really.  Could you give us a testcase?  What platform have you
> seen this behavior on?

PPC/NetBSD.

> On Sat, Feb 28, 2004 at 11:33:51PM -0500, Andrew Cagney wrote:
> 
>>> Hello,
>>> 
>>> This goes into the "how did it ever work" category.  The idea of 
>>> step_over_function is that it:
>>> 
>>> - finds the caller's resume address
>>> - finds the caller's frame ID
>>> 
>>> and then sets a breakpoint for that caller instance of the function. 
>>> The current code:
>>> 
>>> - finds the caller's resume address
>>> - finds the _callee_ frame ID
>>> 
>>> and then uses that to set the breakpoint.  Now that is plain weird!  It 
>>> only works because either:
>>> 
>>> - the step_frame_id patches up the bug
>>> 
>>> - the values match as GDB is using the inner-most, rather than 
>>> outer-most frame address as part of the frame ID

The NetBSD/PPC case, the old code does this:
     /* NOTE: cagney/2002-04-14: The ->frame points to the inner-most
        address of the current frame.  Things might be easier if the
        ->frame pointed to the outer-most address of the frame.  In the
        mean time, the address of the prev frame is used as the base
        address of this frame.  */

>>> The bug apepars when trying to step over nested shared library non-debug 
>>> info functions (making sense?).

So I suspect it applied to all PPC, just that NetBSD's are a bit wierd.

Andrew

Re: [patch/rfc,6.1?] Use right frame ID in step_over_function

[Andrew Cagney]

>>The bug apepars when trying to step over nested shared library non-debug 
>>> info functions (making sense?).
> 
> 
> No, not really.  Could you give us a testcase?  What platform have you
> seen this behavior on?

PPC/NetBSD.

> On Sat, Feb 28, 2004 at 11:33:51PM -0500, Andrew Cagney wrote:
> 
>>> Hello,
>>> 
>>> This goes into the "how did it ever work" category.  The idea of 
>>> step_over_function is that it:
>>> 
>>> - finds the caller's resume address
>>> - finds the caller's frame ID
>>> 
>>> and then sets a breakpoint for that caller instance of the function. 
>>> The current code:
>>> 
>>> - finds the caller's resume address
>>> - finds the _callee_ frame ID
>>> 
>>> and then uses that to set the breakpoint.  Now that is plain weird!  It 
>>> only works because either:
>>> 
>>> - the step_frame_id patches up the bug
>>> 
>>> - the values match as GDB is using the inner-most, rather than 
>>> outer-most frame address as part of the frame ID

The NetBSD/PPC case, the old code does this:
     /* NOTE: cagney/2002-04-14: The ->frame points to the inner-most
        address of the current frame.  Things might be easier if the
        ->frame pointed to the outer-most address of the frame.  In the
        mean time, the address of the prev frame is used as the base
        address of this frame.  */

>>> The bug apepars when trying to step over nested shared library non-debug 
>>> info functions (making sense?).

So I suspect it applied to all PPC, just that NetBSD's are a bit wierd.

Andrew

Re: [patch/rfc,6.1?] Use right frame ID in step_over_function

[Andrew Cagney]

> Hello,
> 
> This goes into the "how did it ever work" category.  The idea of step_over_function is that it:
> 
> - finds the caller's resume address
> - finds the caller's frame ID
> 
> and then sets a breakpoint for that caller instance of the function. The current code:
> 
> - finds the caller's resume address
> - finds the _callee_ frame ID
> 
> and then uses that to set the breakpoint.  Now that is plain weird!  It only works because either:
> 
> - the step_frame_id patches up the bug
> 
> - the values match as GDB is using the inner-most, rather than outer-most frame address as part of the frame ID
> 
> The bug apepars when trying to step over nested shared library non-debug info functions (making sense?).
> 
> I'll follow this up after 6.1 branch is in place.
> 
> Its pretty heavy a change to apply to that branch and this late. However, like Joel's related patch, I suspect it will be needed :-/

I've checked this into the mainline.  For the moment I think I'll drop 
the idea of committing it to the branch.

Andrew



> 	* infrun.c (step_over_function): When non-legacy code, and no
> 	step_frame_id, use the unwinder to get the caller's frame ID.
> 

Re: [patch/rfc,6.1?] Use right frame ID in step_over_function

[Daniel Jacobowitz]

On Fri, Mar 05, 2004 at 06:02:23PM -0500, Andrew Cagney wrote:
> >Hello,
> >
> >This goes into the "how did it ever work" category.  The idea of 
> >step_over_function is that it:
> >
> >- finds the caller's resume address
> >- finds the caller's frame ID
> >
> >and then sets a breakpoint for that caller instance of the function. The 
> >current code:
> >
> >- finds the caller's resume address
> >- finds the _callee_ frame ID
> >
> >and then uses that to set the breakpoint.  Now that is plain weird!  It 
> >only works because either:
> >
> >- the step_frame_id patches up the bug
> >
> >- the values match as GDB is using the inner-most, rather than outer-most 
> >frame address as part of the frame ID
> >
> >The bug apepars when trying to step over nested shared library non-debug 
> >info functions (making sense?).
> >
> >I'll follow this up after 6.1 branch is in place.
> >
> >Its pretty heavy a change to apply to that branch and this late. However, 
> >like Joel's related patch, I suspect it will be needed :-/
> 
> I've checked this into the mainline.  For the moment I think I'll drop 
> the idea of committing it to the branch.

Since I'm not sure if you answered this already - is there a platform
(presumably NetBSD/PPC?) on which this changes testsuite results, or
did you just see it in using GDB on that platform?

Anyway, great - there's a hack in the ARM sigtramp unwinder that I
-suspect- is dead now.  I'll investigate.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch/rfc,6.1?] Use right frame ID in step_over_function

[Andrew Cagney]

>>>Its pretty heavy a change to apply to that branch and this late. However, 
>>>> >like Joel's related patch, I suspect it will be needed :-/
>>
>>> 
>>> I've checked this into the mainline.  For the moment I think I'll drop 
>>> the idea of committing it to the branch.
> 
> 
> Since I'm not sure if you answered this already - is there a platform
> (presumably NetBSD/PPC?) on which this changes testsuite results, or
> did you just see it in using GDB on that platform?

Yes.  NetBSD/PPC with a proper frame unwinder demonstrated an 
improvement (but the problem wasn't NetBSD specific).

Andrew

> Anyway, great - there's a hack in the ARM sigtramp unwinder that I
> -suspect- is dead now.  I'll investigate.

Re: [patch/rfc,6.1?] Use right frame ID in step_over_function

[Andrew Cagney]

>>>Its pretty heavy a change to apply to that branch and this late. However, 
>>>> >like Joel's related patch, I suspect it will be needed :-/
>>
>>> 
>>> I've checked this into the mainline.  For the moment I think I'll drop 
>>> the idea of committing it to the branch.
> 
> 
> Since I'm not sure if you answered this already - is there a platform
> (presumably NetBSD/PPC?) on which this changes testsuite results, or
> did you just see it in using GDB on that platform?

Yes.  NetBSD/PPC with a proper frame unwinder demonstrated an 
improvement (but the problem wasn't NetBSD specific).

Andrew

> Anyway, great - there's a hack in the ARM sigtramp unwinder that I
> -suspect- is dead now.  I'll investigate.

Re: [patch/rfc,6.1?] Use right frame ID in step_over_function

[Daniel Jacobowitz]

On Fri, Mar 05, 2004 at 06:02:23PM -0500, Andrew Cagney wrote:
> >Hello,
> >
> >This goes into the "how did it ever work" category.  The idea of 
> >step_over_function is that it:
> >
> >- finds the caller's resume address
> >- finds the caller's frame ID
> >
> >and then sets a breakpoint for that caller instance of the function. The 
> >current code:
> >
> >- finds the caller's resume address
> >- finds the _callee_ frame ID
> >
> >and then uses that to set the breakpoint.  Now that is plain weird!  It 
> >only works because either:
> >
> >- the step_frame_id patches up the bug
> >
> >- the values match as GDB is using the inner-most, rather than outer-most 
> >frame address as part of the frame ID
> >
> >The bug apepars when trying to step over nested shared library non-debug 
> >info functions (making sense?).
> >
> >I'll follow this up after 6.1 branch is in place.
> >
> >Its pretty heavy a change to apply to that branch and this late. However, 
> >like Joel's related patch, I suspect it will be needed :-/
> 
> I've checked this into the mainline.  For the moment I think I'll drop 
> the idea of committing it to the branch.

Since I'm not sure if you answered this already - is there a platform
(presumably NetBSD/PPC?) on which this changes testsuite results, or
did you just see it in using GDB on that platform?

Anyway, great - there's a hack in the ARM sigtramp unwinder that I
-suspect- is dead now.  I'll investigate.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch/rfc,6.1?] Use right frame ID in step_over_function

[Andrew Cagney]

> Hello,
> 
> This goes into the "how did it ever work" category.  The idea of step_over_function is that it:
> 
> - finds the caller's resume address
> - finds the caller's frame ID
> 
> and then sets a breakpoint for that caller instance of the function. The current code:
> 
> - finds the caller's resume address
> - finds the _callee_ frame ID
> 
> and then uses that to set the breakpoint.  Now that is plain weird!  It only works because either:
> 
> - the step_frame_id patches up the bug
> 
> - the values match as GDB is using the inner-most, rather than outer-most frame address as part of the frame ID
> 
> The bug apepars when trying to step over nested shared library non-debug info functions (making sense?).
> 
> I'll follow this up after 6.1 branch is in place.
> 
> Its pretty heavy a change to apply to that branch and this late. However, like Joel's related patch, I suspect it will be needed :-/

I've checked this into the mainline.  For the moment I think I'll drop 
the idea of committing it to the branch.

Andrew



> 	* infrun.c (step_over_function): When non-legacy code, and no
> 	step_frame_id, use the unwinder to get the caller's frame ID.
>