Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Ulrich Weigand <weigand@i1.informatik.uni-erlangen.de>
To: cagney@gnu.org
Cc: gdb-patches@sources.redhat.com
Subject: Re: [patch/rfc] Use frame_type for sigtramp test in infrun.c
Date: Mon, 29 Mar 2004 23:38:00 -0000	[thread overview]
Message-ID: <200403292338.BAA16799@faui1d.informatik.uni-erlangen.de> (raw)

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


             reply	other threads:[~2004-03-29 23:38 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-03-29 23:38 Ulrich Weigand [this message]
2004-03-31 21:49 ` Andrew Cagney
2004-04-02 20:50 ` Andrew Cagney
2004-04-02 23:57   ` Joel Brobecker
2004-04-03  0:08   ` Joel Brobecker
2004-04-03  1:01     ` Andrew Cagney
2004-04-06  1:48       ` Joel Brobecker
2004-04-06 16:21         ` Joel Brobecker
2004-04-06 17:48           ` Andrew Cagney
2004-04-06 17:54             ` Daniel Jacobowitz
2004-04-06 18:11               ` Andrew Cagney
2004-04-06 23:33                 ` Andrew Cagney
2004-04-29 22:46         ` Andrew Cagney
  -- strict thread matches above, loose matches on Subject: below --
2004-03-16 18:57 Andrew Cagney
2004-03-19  0:09 ` Andrew Cagney
2004-03-21 22:38 ` Andrew Cagney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200403292338.BAA16799@faui1d.informatik.uni-erlangen.de \
    --to=weigand@i1.informatik.uni-erlangen.de \
    --cc=cagney@gnu.org \
    --cc=gdb-patches@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox