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

Joel Brobecker
2003-12-04T23:24:10Z

> (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 :-).

> >However, I tried it on sparc-solaris with the gdb-6.0 sources because
> >I knew this target hasn't transitioned to the new frame framework.
> >It doesn't look like GDB is liking this change there (I've got a lot of
> >timeouts in call-ar-st). I am currently retrying on the head right now,
> >hoping the testsuite completes in a reasonable amount of time.

I redid the testing this time using the head, and found no new
regression introduced by this change [1].

> >--- infrun.c 25 Nov 2003 16:01:36 -0000 1.122
> >+++ infrun.c 3 Dec 2003 19:24:07 -0000
> >@@ -2473,6 +2473,8 @@ process_event_stop_test:
> > }
> >
> > if (((stop_pc == ecs->stop_func_start /* Quick test */
> >+ || 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)

> 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?

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.

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.

> 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.

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.

Thanks,
--
Joel

[1]: There was one glitch that occurred during the testing, though.
Even before I applied any change, the structs.exp test keeps
timeouting. I tried letting it complete, but had to stop it
after an entire night of timeouts. So I did my testing after
having deactivated this test.