[commit] Don't use STEP_FRAME_ID; Was: [PATCH] Fix signals.exp test case on S/390
Andrew Cagney
cagney@gnu.org
Mon Mar 15 17:12:00 GMT 2004
I've committted the attached.
As Ulrich explains in the below, trying to use STEP_FRAME_ID was
creating a big nasty rat hole.
The patch does preserve the old behavior when legacy_frame_p (hmm, which
must be on its last legs :-).
> Andrew Cagney wrote:
>
>>>> > It will run into the first if, and simply use step_frame_id,
>>>> > which is wrong in this case. That's why my patch add another
>>>> > condition to the first if, to make it not taken and actually
>>>> > use the (correct) get_prev_frame case.
>>
>>>
>>> Where is step_frame_id pointing?
>
>
> To the function that was interrupted by the signal (i.e. the
> function where I entered 'next').
Good.
>>> Anyway, I think this code:
>>> > if (frame_id_p (step_frame_id)
>>> > && !IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc))
>>> > /* 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;
>>> should simply be deleted. I wondered about it and you've just confirmed
>>> my suspicions. With that code gone is half the problem solved?
>
>
> Yes, deleting this works just fine for me, in fact ...
Good.
>>> That leaves the other problem, which is much harder :-(
>
>
> ... it even solves the other problem as well!
Yow!
> The reason for this is that the whole problematic if
> that uses frame_id_inner becomes irrelevant:
>
> if (pc_in_sigtramp (stop_pc)
> && frame_id_inner (step_frame_id,
> frame_id_build (read_sp (), 0)))
> /* We stepped out of a signal handler, and into its
> calling trampoline. This is misdetected as a
> subroutine call, but stepping over the signal
> trampoline isn't such a bad idea. In order to do that,
> we have to ignore the value in step_frame_id, since
> that doesn't represent the frame that'll reach when we
> return from the signal trampoline. Otherwise we'll
> probably continue to the end of the program. */
> step_frame_id = null_frame_id;
>
> step_over_function (ecs);
>
> With those lines in step_over_function deleted, step_over_function
> does not care about step_frame_id at all any more, and thus there
> is no need to fiddle with step_frame_id here ...
>>>> > Finally, the patch below reintroduces a pc_in_sigtramp
>>>> > gdbarch callback to s390-tdep.c; I had thought this would
>>>> > be no longer necessary when using the new frame code, but
>>>> > apparently there's still other users ...
>>
>>>
>>> Yes, it shouldn't be needed. get_frame_type == SIGTRAMP_FRAME is
>>> sufficient. work-in-progress.
>
>
> Actually, when deleting the lines in step_over_function, it turns
> out that I don't need pc_in_sigtramp any more ...
>
> Summing up: after completely reverting my patch, and simply
> deleting those lines, I get a gdb that passes signals.exp
> (and has no test suite regressions), and also handles stepping
> out of a signal handler correctly.
ya!
thanks,
Andrew
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: diffs
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20040315/2f38eef6/attachment.ksh>
More information about the Gdb-patches
mailing list