[PATCH] Fix signals.exp test case on S/390

Ulrich Weigand
2004-03-10T20:32:55Z

Hello,

I was trying to get the signals.exp test case to pass on s390.
This exposed a number of problems in the Linux kernel and gdb;
in particular I think I've found a bug in gdb common code
(step_over_function in infrun.c).

However, I'm not completely sure I fully understand the intended
flow of control throughout infrun.c, so I'd appreciate comments
on what I found.

Basically, the test case arranges for a signal (SIGALRM) to
arrive while the inferior is stopped, and then does a 'next'.
When the inferior is continued with PT_SINGLESTEP due to the
'next', the kernel tries to deliver the pending SIGALRM.

This in turn causes another ptrace intercept. gdb gets active,
notices that it doesn't want to do anything special with the
signal, and continues with PT_SINGLESTEP giving the signal
number.

At this point, the Linux kernel delivers the signal, runs to
the first instruction of the signal handler, and gets the
single step trap there. (Note that there was a bug in the
kernel that caused this not to work, but I've fixed that one.)

Now the interesting aspect starts. handle_inferior_event
needs to figure out what has happened here. There is special
code in handle_inferior_event that tries to recognize the
'we've just stepped into a signal handler' situation:

/* Did we just take a signal? */
if (pc_in_sigtramp (stop_pc)
&& !pc_in_sigtramp (prev_pc)
&& INNER_THAN (read_sp (), step_sp))
{
/* We've just taken a signal; go until we are back to
the point where we took it and one more. */

Note, however, that this fundamentally does not work on
Linux because we use a signal trampoline only to *exit*
from a signal handler -- the kernel *starts* signal handlers
by jumping to them directly without any trampoline.

So this test doesn't trigger, and the event turns out to
be interpreted as call to a subroutine, and is passed on
to handle_step_info_function, which decides to step over
that function call using step_over_function. This would
basically work just fine for this case -- we'd continue
just after the signal handler terminates, which is what
we want here.

However, step_over_function continues to run not only
until a specific PC has been reached, but at the same time
a specific *frame* needs to be reached. In the situation
we're in right now:

PC frame
1st instruction of signal handler sig. handler frame
1st instruction of sigreturn trampoline sigtramp frame
interrupted main routine main routine frame

step_over_function tries to continue running until it
reaches the current frame's return address (i.e. the 1st
instruction of the sigreturn trampoline) while at the same
time reaching the frame currently being stepped (i.e the
main routine's frame). Of course, these two events never
coincide, and hence gdb steps until the program terminates.

Now, a similar situation occurs if there is a dynamic linker
resolver frame between a called subroutine and the main
routine, and there is special code in step_over_function to
skip the intermediate frame in this case. The patch below
extends this special handling to the case of an intermediate
signal trampoline frame, which fixes the symptom for me --
the signals.exp test case now passes.

Is this an acceptable solution or should the signal-handler
code in handle_inferior_event be made smarter?

Also, I've run into another problem when single-stepping
*out* of a signal handler. In this case code in
handle_step_into_function is supposed to recognize that
we've just stepped into a signal trampoline and just
continue on over the sigreturn system call.

This doesn't work on s390 because of the comparison:
&& frame_id_inner (step_frame_id,
frame_id_build (read_sp (), 0)))

since our frame IDs contain the DWARF CFA as stack_addr,
which is biased relative to the stack pointer at function
entry. So comparing this CFA to a real SP value is bogus.

Other places compare either two stack frame IDs or two
real SP values, either of which should work. The patch below
changes the comparison to
&& INNER_THAN (step_sp, read_sp ()))

similar to most other places in infrun.c that manage
signal trampolines.

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

The whole patch builds on s390-ibm-linux with no test suite
regressions and fixes the signals.exp test case.

Bye,
Ulrich

ChangeLog:

* infrun.c (handle_step_into_function): Do not compare a frame ID
stack_addr directly against a read_sp () stack pointer value.
(step_over_function): Skip signal trampoline frame on function
return.
* s390-tdep.c (s390_sigtramp_frame_sniffer): Move core signal
trampoline detection logic to ...
(s390_pc_in_sigtramp): ... this new function.
(s390_gdbarch_init): Call set_gdbarch_pc_in_sigtramp.

Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.138
diff -c -p -r1.138 infrun.c
*** gdb/infrun.c 6 Mar 2004 00:10:06 -0000 1.138
--- gdb/infrun.c 10 Mar 2004 01:33:06 -0000
*************** 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
*************** step_over_function (struct execution_con
*** 2976,2982 ****
check_for_old_step_resume_breakpoint ();

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. */
--- 2975,2982 ----
check_for_old_step_resume_breakpoint ();

if (frame_id_p (step_frame_id)
! && !IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc)
! && !pc_in_sigtramp (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. */
Index: gdb/s390-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/s390-tdep.c,v
retrieving revision 1.129
diff -c -p -r1.129 s390-tdep.c
*** gdb/s390-tdep.c 26 Feb 2004 23:48:01 -0000 1.129
--- gdb/s390-tdep.c 10 Mar 2004 01:33:06 -0000
*************** static const struct frame_unwind s390_si
*** 2237,2258 ****
s390_sigtramp_frame_prev_register
};

! static const struct frame_unwind *
! s390_sigtramp_frame_sniffer (struct frame_info *next_frame)
{
- CORE_ADDR pc = frame_pc_unwind (next_frame);
bfd_byte sigreturn[2];
-
if (read_memory_nobpt (pc, sigreturn, 2))
! return NULL;

if (sigreturn[0] != 0x0a /* svc */)
! return NULL;

if (sigreturn[1] != 119 /* sigreturn */
&& sigreturn[1] != 173 /* rt_sigreturn */)
return NULL;
!
return &s390_sigtramp_frame_unwind;
}

--- 2237,2266 ----
s390_sigtramp_frame_prev_register
};

! static int
! s390_pc_in_sigtramp (CORE_ADDR pc, char *name)
{
bfd_byte sigreturn[2];
if (read_memory_nobpt (pc, sigreturn, 2))
! return 0;

if (sigreturn[0] != 0x0a /* svc */)
! return 0;

if (sigreturn[1] != 119 /* sigreturn */
&& sigreturn[1] != 173 /* rt_sigreturn */)
+ return 0;
+
+ return 1;
+ }
+
+ static const struct frame_unwind *
+ s390_sigtramp_frame_sniffer (struct frame_info *next_frame)
+ {
+ CORE_ADDR pc = frame_pc_unwind (next_frame);
+ if (!s390_pc_in_sigtramp (pc, NULL))
return NULL;
!
return &s390_sigtramp_frame_unwind;
}

*************** s390_gdbarch_init (struct gdbarch_info i
*** 3024,3029 ****
--- 3032,3038 ----
set_gdbarch_return_value (gdbarch, s390_return_value);

/* Frame handling. */
+ set_gdbarch_pc_in_sigtramp (gdbarch, s390_pc_in_sigtramp);
set_gdbarch_in_solib_call_trampoline (gdbarch, in_plt_section);
dwarf2_frame_set_init_reg (gdbarch, s390_dwarf2_frame_init_reg);
frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer);