This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch/rfc] Use frame_type for sigtramp test in infrun.c


Andrew Cagney wrote:

>        * infrun.c (handle_inferior_event): For non legacy frames, use the
>        frame ID and frame type to identify a signal trampoline.  Update
>        comments.

This breaks signals.exp on s390 again.

What now happens is the following:

- we are in main and do 'next'; step_frame_id is set to the current frame
- gdb starts single-stepping
- in the meantime, a signal has arrived and the kernel invokes the handler
  Note that the kernel directly jumps to the handler, and sets up the
  return address to point to the signal return trampoline.
- gdb single-steps and stops on the first instruction of the handler
  at this point, the call-chain is
    handler
    <signal trampoline>
    main
- gdb thinks it has stepped into a subroutine, and does
  handle_step_into_function, which calls keep_going
- keep_going saves the current pc (i.e. start of handler) into prev_pc
- we run until the return from handler, i.e. we stop on the first
  (and only) instruction of the signal trampoline
- now the 'if' you changed hits because we are in fact in a signal trampoline
  frame. 
- within the if, there's a second if that tries to distiguish between
  stepping into a trampoline before the signal handler from one after
  the signal handler.  This is attempted via
    if (frame_id_inner (current_frame, step_frame_id))
- unfortunately, step_frame_id is still the frame of 'main', and thus
  the current (i.e. trampoline) frame *is* 'inner', even though we
  actually have a trampoline after, not before the handler.
- gdb now does
            sr_sal.line = 0;
            sr_sal.pc = prev_pc;
            /* We could probably be setting the frame to
               step_frame_id; I don't think anyone thought to try it.  */
            check_for_old_step_resume_breakpoint ();
            step_resume_breakpoint =
              set_momentary_breakpoint (sr_sal, null_frame_id, bp_step_resume);
  However, prev_pc was set to first instruction of handler above
- since we never again enter the handler, the breakpoint is never hit
  and we run until the end of main


I'm not sure what the correct way to fix this issue would be.

However, simply removing the whole 'if' block makes signals.exp pass on s390.
This is because both the handler and the signal return trampoline are now
simply treated as calls into subroutines, and both are skipped with
step_over_function, so that everything works just as expected.

Why is this if needed in the first place?  Isn't this just to work around
frame problems that caused step_over_function to not handle signal handlers
correctly?  I.e. if we have new-style frames that work properly, can't we
just skip that whole if?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]