This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [PATCH] Fix signals.exp test case on S/390
- From: Andrew Cagney <cagney at gnu dot org>
- To: Ulrich Weigand <weigand at i1 dot informatik dot uni-erlangen dot de>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Thu, 11 Mar 2004 11:07:54 -0500
- Subject: Re: [PATCH] Fix signals.exp test case on S/390
- References: <200403110007.BAA05694@faui1d.informatik.uni-erlangen.de>
Andrew Cagney wrote:
I'm lost here. What happens with:
- get_frame_id (get_prev_frame (signal handler))
- get_frame_id (sigreturn trampoline)
They should match
Well, they do match, that's why things work with my patch.
Ok, good.
The problem is that without my patch, step_over_function
doesn't actually *use* get_frame_id (get_prev_frame ...),
see the 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;
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 ()));
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?
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?
That leaves the other problem, which is much harder :-(
*************** handle_step_into_function (struct execut
*** 1265,1272 ****
/* We're doing a "next". */
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
--- 1265,1271 ----
/* We're doing a "next". */
if (pc_in_sigtramp (stop_pc)
! && INNER_THAN (step_sp, read_sp ()))
/* We stepped out of a signal handler, and into its
calling trampoline. This is misdetected as a
subroutine call, but stepping over the signal
Both INNER_THAN and frame_id_build(read_sp (),0) / frame_id_inner are
wrong here. They test variations on:
/* Returns non-zero when L is strictly inner-than R (they have
different frame .bases). Neither L, nor R can be `null'. See note
above about frameless functions. */
...
/* Methods for constructing and comparing Frame IDs.
NOTE: Given stackless functions A and B, where A calls B (and hence
B is inner-to A). The relationships: !eq(A,B); !eq(B,A);
!inner(A,B); !inner(B,A); all hold.
This is because, while B is inner-to A, B is not strictly inner-to A.
Being stackless, they have an identical .stack_addr value, and differ
only by their unordered .code_addr and/or .special_addr values.
Because frame_id_inner is only used as a safety net (e.g.,
detect a corrupt stack) the lack of strictness is not a problem.
Code needing to determine an exact relationship between two frames
must instead use frame_id_eq and frame_id_unwind. For instance,
in the above, to determine that A stepped-into B, the equation
"A.id != B.id && A.id == id_unwind (B)" can be used. */
and that isn't sufficient. It's easy to construct a situtation where
the handler and sigtramp have the same ID/SP.
A better strategy here might be to, if the user isn't instruction
stepping or cntrl-ced, single-step the inferior until it finds its way
out of the sigtramp and then make a decision.
Andrew
PS:
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.