Re: [RFA] OSF/1 - "next" over prologueless function call

Andrew Cagney
2003-12-05T20:19:13Z

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

> (only took 15 years to realise how "obvious" it was :-)
>
>
> Right. It's only "obvious" because the we now have a frame ID which
> particularity is to be unique... Given how long it must have taken to
> "invent the wheel", maybe we didn't do too bad :-).

See the comments in the attached ...

>> You can use legacy_frame_p for differentiating old and new code.
>
>
> Yes. Despite the relative success on sparc-solaris (no new regression),
> I wasn't sure whether this wasn't potentially introducing new problems
> on the architectures where the legacy frames are still used. So I added
> it to the condition.
>
> That gives us a pretty horrible condition, something like that:
>
> if (((stop_pc == ecs->stop_func_start /* Quick test */
> || (!legacy_frame_pd (current_gdbarch)
> && frame_id_eq
> (get_frame_id (get_prev_frame (get_current_frame ())),
> step_frame_id))
> || 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)
>
> How about we move this into a function?

There are several things we could move to a function ....

> My current version only replaces the condition I added by:
>
> || frame_in_procedure_called_during_step ()
>
> So the condition stay relatively readable (not too many levels in terms
> of nesting). But maybe it's time to turn the entire condition into a
> function and add more comments.
>
> Something like this?
>
> static int
> inside_function_called_from_step (CORE_ADDR stop_pc,
> CORE_ADDR stop_func_start)
> {
> /* If bla bla bla */
> if (stop_pc == stop_func_start
> && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, stop_func_name))
> return 1;
>
> /* If the frame ID of the previous frame is equal to step_frame_id,
> then this function was indeed called during a step. Only rely
> on this test if we don't use the legacy frames. */
> if (!legacy_frame_p (current_gdbarch)
> && frame_id_eq (...)
> && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, stop_func_name))
> return 1;
>
> /* cagney/2003-12-04: Do we really need this, now that we check the
> previous frame id? */
> if (in_prologue (stop_pc, ecs->stop_func_start))
> && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, stop_func_name))
> return 1;
>
> /* Don't stop stepping inside a solib call trampoline. */
> if (IN_SOLIB_CALL_TRAMPOLINE (stop_pc, stop_func_name))
> return 1;
>
> /* Hmmm, continue stepping if we don't know anything about where
> we landed. */
> if (stop_func_name == 0)
> return 1;
>
> return 0;
> }
>
> The trouble is that this change is not completly mechanical. And
> we also end up repeating the "!IN_SOLIB_CALL_TRAMPOLINE" test.
> We can also coalesce the first 3 tests into one if, but then
> we lose a bit the advantage of using a function.

There's always plan B.

Looking at the body of that IF, I believe it always returns. That
should let us do:

if (legacy_frame_p ())
if (all the existing tests)
call a function to do the body of work ()
return;
else if (our new improved test)
call a function to do the body of work ()
return;

that way the legacy and non legacy cases are clearly split - we're free
to refine the new conditional with out worrying about breaking the old
code. However ....

(Oh, what exactly is that body of code doing - any ideas?)

> Anyway, it's more a matter of style, so I will defer to the preference
> of the maintainers, and send a patch along these lines. If we decide
> to move the entire condition into a new function, I can send 2 patches:
> One that moves the current condition, and then another that fixes
> the osf1 problem.
>
>
>> Hmm, is "stop_pc == ecs->stop_func_start" a valid test, what happens if
>> the program is at 1: and there's a next?
>>
>> foo:
>> ...
>> 1: goto foo
>
>
> Unless the function is completely prologueless, all should be well
> because the goto will branch at a location past the first instruction.
> If the function is prologueless, however... I think this would be a rare
> occurrence.

I'm not so sure :-( In fact any lack of such code suggests a GCC
optimizer bug :-)

>> Hmm, is in_prologue() adding value when frame_id always works? Unless
>> it's being used to handle stepping through a prologue?
>
>
> Not sure about the real intent of in_prologue(). I know it saved us
> most of the time in the osf/1 case, but was it the primary intent
> when this condition was added? I think that once we have answered
> this question, we will be able to decide whether or not this condition
> can be removed.

... valid point.

Staring at that code its doing so many things its depressing:

- step into function
- step over function
- step out of function
- step over trampoline

but the basic intent was possibly step into a function and then skip
over its prologue?

So I'm guessing for the moment just replace
stop_pc == ecs->stop_func_start
with the frame id test in the new code?

:-(

> I also realize that I may not have been clear about the usage of
> legacy_frame_p. I am a bit nervous at doing without, but you may be
> more confident than me, since I don't have much knowledge of the new
> frame architecture yet. So the choice of using it or not is still open.

Andrew

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 13167 bytes --]

1.155 (grossman 24-Oct-95): #if 0
1.155 (grossman 24-Oct-95): /* I disabled this test because it was too complicated and slow. The
1.155 (grossman 24-Oct-95): SKIP_PROLOGUE was especially slow, because it caused unnecessary
1.155 (grossman 24-Oct-95): prologue examination on various architectures. The code in the #else
1.155 (grossman 24-Oct-95): clause has been tested on the Sparc, Mips, PA, and Power
1.155 (grossman 24-Oct-95): architectures, so it's pretty likely to be correct. -Stu 10/24/95 */
1.155 (grossman 24-Oct-95):
1.137 (kingdon 08-Oct-94): /* See if we left the step range due to a subroutine call that
1.137 (kingdon 08-Oct-94): we should proceed to the end of. */
1.137 (kingdon 08-Oct-94):
1.80 (kingdon 11-Jul-93): if (stop_func_start)
1.80 (kingdon 11-Jul-93): {
1.130 (grossman 02-Jun-94): struct symtab *s;
1.130 (grossman 02-Jun-94):
1.80 (kingdon 11-Jul-93): /* Do this after the IN_SIGTRAMP check; it might give
1.80 (kingdon 11-Jul-93): an error. */
1.80 (kingdon 11-Jul-93): prologue_pc = stop_func_start;
1.130 (grossman 02-Jun-94):
1.130 (grossman 02-Jun-94): /* Don't skip the prologue if this is assembly source */
1.130 (grossman 02-Jun-94): s = find_pc_symtab (stop_pc);
1.130 (grossman 02-Jun-94): if (s && s->language != language_asm)
1.130 (grossman 02-Jun-94): SKIP_PROLOGUE (prologue_pc);
1.80 (kingdon 11-Jul-93): }
1.1 (rich 28-Mar-91):
1.101 (kingdon 17-Oct-93): if ((/* Might be a non-recursive call. If the symbols are missing
1.101 (kingdon 17-Oct-93): enough that stop_func_start == prev_func_start even though
1.101 (kingdon 17-Oct-93): they are really two functions, we will treat some calls as
1.101 (kingdon 17-Oct-93): jumps. */
1.101 (kingdon 17-Oct-93): stop_func_start != prev_func_start
1.101 (kingdon 17-Oct-93):
1.101 (kingdon 17-Oct-93): /* Might be a recursive call if either we have a prologue
1.101 (kingdon 17-Oct-93): or the call instruction itself saves the PC on the stack. */
1.101 (kingdon 17-Oct-93): || prologue_pc != stop_func_start
1.137 (kingdon 08-Oct-94): || read_sp () != step_sp)
1.105 (grossman 26-Oct-93): && (/* PC is completely out of bounds of any known objfiles. Treat
1.105 (grossman 26-Oct-93): like a subroutine call. */
1.105 (grossman 26-Oct-93): ! stop_func_start
1.101 (kingdon 17-Oct-93):
1.110 (kingdon 30-Dec-93): /* If we do a call, we will be at the start of a function... */
1.101 (kingdon 17-Oct-93): || stop_pc == stop_func_start
1.37 (bothner 01-Mar-92):
1.110 (kingdon 30-Dec-93): /* ...except on the Alpha with -O (and also Irix 5 and
1.110 (kingdon 30-Dec-93): perhaps others), in which we might call the address
1.110 (kingdon 30-Dec-93): after the load of gp. Since prologues don't contain
1.110 (kingdon 30-Dec-93): calls, we can't return to within one, and we don't
1.110 (kingdon 30-Dec-93): jump back into them, so this check is OK. */
1.110 (kingdon 30-Dec-93):
1.101 (kingdon 17-Oct-93): || stop_pc < prologue_pc
1.101 (kingdon 17-Oct-93):
1.134 (schauer 15-Jul-94): /* ...and if it is a leaf function, the prologue might
1.134 (schauer 15-Jul-94): consist of gp loading only, so the call transfers to
1.134 (schauer 15-Jul-94): the first instruction after the prologue. */
1.134 (schauer 15-Jul-94): || (stop_pc == prologue_pc
1.134 (schauer 15-Jul-94):
1.134 (schauer 15-Jul-94): /* Distinguish this from the case where we jump back
1.134 (schauer 15-Jul-94): to the first instruction after the prologue,
1.134 (schauer 15-Jul-94): within a function. */
1.134 (schauer 15-Jul-94): && stop_func_start != prev_func_start)
1.134 (schauer 15-Jul-94):
1.101 (kingdon 17-Oct-93): /* If we end up in certain places, it means we did a subroutine
1.101 (kingdon 17-Oct-93): call. I'm not completely sure this is necessary now that we
1.101 (kingdon 17-Oct-93): have the above checks with stop_func_start (and now that
1.102 (kingdon 17-Oct-93): find_pc_partial_function is pickier). */
1.140 (law 24-Nov-94): || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, stop_func_name)
1.101 (kingdon 17-Oct-93):
1.101 (kingdon 17-Oct-93): /* If none of the above apply, it is a jump within a function,
1.101 (kingdon 17-Oct-93): or a return from a subroutine. The other case is longjmp,
1.101 (kingdon 17-Oct-93): which can no longer happen here as long as the
1.101 (kingdon 17-Oct-93): handling_longjmp stuff is working. */
1.101 (kingdon 17-Oct-93): ))
1.130 (grossman 02-Jun-94): #else
1.130 (grossman 02-Jun-94): /* This is experimental code which greatly simplifies the subroutine call
1.130 (grossman 02-Jun-94): test. I've actually tested on the Alpha, and it works great. -Stu */
1.130 (grossman 02-Jun-94):
1.155 (grossman 24-Oct-95): if (stop_pc == stop_func_start /* Quick test */
1.155 (grossman 24-Oct-95): || in_prologue (stop_pc, stop_func_start)
1.155 (grossman 24-Oct-95): || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, stop_func_name)
1.155 (grossman 24-Oct-95): || stop_func_start == 0)
1.130 (grossman 02-Jun-94): #endif
1.155 (grossman 24-Oct-95):
1.80 (kingdon 11-Jul-93): {
1.80 (kingdon 11-Jul-93): /* It's a subroutine call. */
1.34 (grossman 22-Feb-92):
1.80 (kingdon 11-Jul-93): if (step_over_calls == 0)
1.80 (kingdon 11-Jul-93): {
1.80 (kingdon 11-Jul-93): /* I presume that step_over_calls is only 0 when we're
1.80 (kingdon 11-Jul-93): supposed to be stepping at the assembly language level
1.80 (kingdon 11-Jul-93): ("stepi"). Just stop. */
1.80 (kingdon 11-Jul-93): stop_step = 1;
1.80 (kingdon 11-Jul-93): break;
1.80 (kingdon 11-Jul-93): }
1.37 (bothner 01-Mar-92):
1.80 (kingdon 11-Jul-93): if (step_over_calls > 0)
1.80 (kingdon 11-Jul-93): /* We're doing a "next". */
1.80 (kingdon 11-Jul-93): goto step_over_function;
1.80 (kingdon 11-Jul-93):
1.80 (kingdon 11-Jul-93): /* If we are in a function call trampoline (a stub between
1.80 (kingdon 11-Jul-93): the calling routine and the real function), locate the real
1.80 (kingdon 11-Jul-93): function. That's what tells us (a) whether we want to step
1.80 (kingdon 11-Jul-93): into it at all, and (b) what prologue we want to run to
1.80 (kingdon 11-Jul-93): the end of, if we do step into it. */
1.80 (kingdon 11-Jul-93): tmp = SKIP_TRAMPOLINE_CODE (stop_pc);
1.80 (kingdon 11-Jul-93): if (tmp != 0)
1.80 (kingdon 11-Jul-93): stop_func_start = tmp;
1.80 (kingdon 11-Jul-93):
1.80 (kingdon 11-Jul-93): /* If we have line number information for the function we
1.80 (kingdon 11-Jul-93): are thinking of stepping into, step into it.
1.80 (kingdon 11-Jul-93):
1.80 (kingdon 11-Jul-93): If there are several symtabs at that PC (e.g. with include
1.80 (kingdon 11-Jul-93): files), just want to know whether *any* of them have line
1.80 (kingdon 11-Jul-93): numbers. find_pc_line handles this. */
1.80 (kingdon 11-Jul-93): {
1.80 (kingdon 11-Jul-93): struct symtab_and_line tmp_sal;
1.77 (kingdon 29-Jun-93):
1.80 (kingdon 11-Jul-93): tmp_sal = find_pc_line (stop_func_start, 0);
1.80 (kingdon 11-Jul-93): if (tmp_sal.line != 0)
1.80 (kingdon 11-Jul-93): goto step_into_function;
1.80 (kingdon 11-Jul-93): }
1.77 (kingdon 29-Jun-93):
1.37 (bothner 01-Mar-92): step_over_function:
1.80 (kingdon 11-Jul-93): /* A subroutine call has happened. */
1.80 (kingdon 11-Jul-93): {
1.80 (kingdon 11-Jul-93): /* Set a special breakpoint after the return */
1.80 (kingdon 11-Jul-93): struct symtab_and_line sr_sal;
1.80 (kingdon 11-Jul-93): sr_sal.pc =
1.80 (kingdon 11-Jul-93): ADDR_BITS_REMOVE
1.80 (kingdon 11-Jul-93): (SAVED_PC_AFTER_CALL (get_current_frame ()));
1.80 (kingdon 11-Jul-93): sr_sal.symtab = NULL;
1.80 (kingdon 11-Jul-93): sr_sal.line = 0;
1.80 (kingdon 11-Jul-93): step_resume_breakpoint =
1.80 (kingdon 11-Jul-93): set_momentary_breakpoint (sr_sal, get_current_frame (),
1.80 (kingdon 11-Jul-93): bp_step_resume);
1.137 (kingdon 08-Oct-94): step_resume_breakpoint->frame = step_frame_address;
1.80 (kingdon 11-Jul-93): if (breakpoints_inserted)
1.80 (kingdon 11-Jul-93): insert_breakpoints ();
1.80 (kingdon 11-Jul-93): }
1.80 (kingdon 11-Jul-93): goto keep_going;
1.34 (grossman 22-Feb-92):
1.37 (bothner 01-Mar-92): step_into_function:
1.80 (kingdon 11-Jul-93): /* Subroutine call with source code we should not step over.
1.80 (kingdon 11-Jul-93): Do step to the first line of code in it. */
1.130 (grossman 02-Jun-94): {
1.130 (grossman 02-Jun-94): struct symtab *s;
1.130 (grossman 02-Jun-94):
1.130 (grossman 02-Jun-94): s = find_pc_symtab (stop_pc);
1.130 (grossman 02-Jun-94): if (s && s->language != language_asm)
1.130 (grossman 02-Jun-94): SKIP_PROLOGUE (stop_func_start);
1.130 (grossman 02-Jun-94): }
1.80 (kingdon 11-Jul-93): sal = find_pc_line (stop_func_start, 0);
1.80 (kingdon 11-Jul-93): /* Use the step_resume_break to step until
1.80 (kingdon 11-Jul-93): the end of the prologue, even if that involves jumps
1.80 (kingdon 11-Jul-93): (as it seems to on the vax under 4.2). */
1.80 (kingdon 11-Jul-93): /* If the prologue ends in the middle of a source line,
1.111 (schauer 01-Jan-94): continue to the end of that source line (if it is still
1.111 (schauer 01-Jan-94): within the function). Otherwise, just go to end of prologue. */
1.1 (rich 28-Mar-91): #ifdef PROLOGUE_FIRSTLINE_OVERLAP
1.80 (kingdon 11-Jul-93): /* no, don't either. It skips any code that's
1.80 (kingdon 11-Jul-93): legitimately on the first line. */
1.1 (rich 28-Mar-91): #else
1.111 (schauer 01-Jan-94): if (sal.end && sal.pc != stop_func_start && sal.end < stop_func_end)
1.80 (kingdon 11-Jul-93): stop_func_start = sal.end;
1.1 (rich 28-Mar-91): #endif
1.37 (bothner 01-Mar-92):
1.80 (kingdon 11-Jul-93): if (stop_func_start == stop_pc)
1.80 (kingdon 11-Jul-93): {
1.80 (kingdon 11-Jul-93): /* We are already there: stop now. */
1.80 (kingdon 11-Jul-93): stop_step = 1;
1.80 (kingdon 11-Jul-93): break;
1.80 (kingdon 11-Jul-93): }
1.80 (kingdon 11-Jul-93): else
1.80 (kingdon 11-Jul-93): /* Put the step-breakpoint there and go until there. */
1.80 (kingdon 11-Jul-93): {
1.80 (kingdon 11-Jul-93): struct symtab_and_line sr_sal;
1.80 (kingdon 11-Jul-93):
1.80 (kingdon 11-Jul-93): sr_sal.pc = stop_func_start;
1.80 (kingdon 11-Jul-93): sr_sal.symtab = NULL;
1.80 (kingdon 11-Jul-93): sr_sal.line = 0;
1.80 (kingdon 11-Jul-93): /* Do not specify what the fp should be when we stop
1.80 (kingdon 11-Jul-93): since on some machines the prologue
1.80 (kingdon 11-Jul-93): is where the new fp value is established. */
1.80 (kingdon 11-Jul-93): step_resume_breakpoint =
1.89 (kingdon 18-Sep-93): set_momentary_breakpoint (sr_sal, NULL, bp_step_resume);
1.80 (kingdon 11-Jul-93): if (breakpoints_inserted)
1.80 (kingdon 11-Jul-93): insert_breakpoints ();
1.80 (kingdon 11-Jul-93):
1.80 (kingdon 11-Jul-93): /* And make sure stepping stops right away then. */
1.80 (kingdon 11-Jul-93): step_range_end = step_range_start;
1.1 (rich 28-Mar-91): }
1.80 (kingdon 11-Jul-93): goto keep_going;
1.80 (kingdon 11-Jul-93): }