public inbox for gdb-prs@sourceware.org
help / color / mirror / Atom feed
From: "cvs-commit at gcc dot gnu.org" <sourceware-bugzilla@sourceware.org>
To: gdb-prs@sourceware.org
Subject: [Bug gdb/12623] Crashes on 'n' or 's' single stepping in 'non-stop' mode.
Date: Tue, 28 Oct 2014 16:07:00 -0000	[thread overview]
Message-ID: <bug-12623-4717-5U1aQu5xkA@http.sourceware.org/bugzilla/> (raw)
In-Reply-To: <bug-12623-4717@http.sourceware.org/bugzilla/>

https://sourceware.org/bugzilla/show_bug.cgi?id=12623

--- Comment #6 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  7f5ef60532b466ec7a83a943f36e93e32e30eafe (commit)
      from  abbdbd03db7eea82cadbb418da733991cba91b15 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7f5ef60532b466ec7a83a943f36e93e32e30eafe

commit 7f5ef60532b466ec7a83a943f36e93e32e30eafe
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Oct 28 13:42:11 2014 +0000

    PR gdb/12623: non-stop crashes inferior, PC adjustment and 1-byte insns

    TL;DR - if we step an instruction that is as long as
    decr_pc_after_break (1-byte on x86) right after removing the
    breakpoint at PC, in non-stop mode, adjust_pc_after_break adjusts the
    PC, but it shouldn't.

    In non-stop mode, when a breakpoint is removed, it is moved to the
    "moribund locations" list.  This is because other threads that are
    running may have tripped on that breakpoint as well, and we haven't
    heard about it.  When a trap is reported, we check if perhaps it was
    such a deleted breakpoint that caused the trap.  If so, we also need
    to adjust the PC (decr_pc_after_break).

    Now, say that, on x86:

     - a breakpoint was placed at an address where we have an instruction
    of the same length as decr_pc_after_break on this arch (1 on x86).

     - the breakpoint is removed, and thus put on the moribund locations
       list.

     - the thread is single-stepped.

    As there's no breakpoint inserted at PC anymore, the single-step
    actually executes the 1-byte instruction normally.  GDB should _not_
    adjust the PC for the resulting SIGTRAP.  But, adjust_pc_after_break
    confuses the step SIGTRAP reported for this single-step as being a
    SIGTRAP for the moribund location of the breakpoint that used to be at
    the previous PC, and so infrun applies the decr_pc_after_break
    adjustment incorrectly.

    The confusion comes from the special case mentioned in the comment:

     static void
     adjust_pc_after_break (struct execution_control_state *ecs)
     {
     ...
          As a special case, we could have hardware single-stepped a
          software breakpoint.  In this case (prev_pc == breakpoint_pc),
          we also need to back up to the breakpoint address.  */

           if (thread_has_single_step_breakpoints_set (ecs->event_thread)
           || !ptid_equal (ecs->ptid, inferior_ptid)
           || !currently_stepping (ecs->event_thread)
           || (ecs->event_thread->stepped_breakpoint
               && ecs->event_thread->prev_pc == breakpoint_pc))
         regcache_write_pc (regcache, breakpoint_pc);

    The condition that incorrectly triggers is the
    "ecs->event_thread->prev_pc == breakpoint_pc" one.

    Afterwards, the next resume resume re-executes an instruction that had
    already executed, which if you're lucky, results in the inferior
    crashing.  If you're unlucky, you'll get silent bad behavior...

    The fix is to remember that we stepped a breakpoint.  Turns out the
    only case we step a breakpoint instruction today isn't covered by the
    testsuite.  It's the case of a 'handle nostop" signal arriving while a
    step is in progress _and_ we have a software watchpoint, which forces
    always single-stepping.  This commit extends sigstep.exp to cover
    that, and adds a new test for the adjust_pc_after_break issue.

    Tested on x86_64 Fedora 20, native and gdbserver.

    gdb/
    2014-10-28  Pedro Alves  <palves@redhat.com>

        PR gdb/12623
        * gdbthread.h (struct thread_info) <stepped_breakpoint>: New
        field.
        * infrun.c (resume) <stepping breakpoint instruction>: Set the
        thread's stepped_breakpoint field.  Skip if reverse debugging.
        Add comment.
        (init_thread_stepping_state, handle_signal_stop): Clear the
        thread's stepped_breakpoint field.

    gdb/testsuite/
    2014-10-28  Pedro Alves  <palves@redhat.com>

        PR gdb/12623
        * gdb.base/sigstep.c (no_handler): New global.
        (main): If 'no_handler is true, set the signal handlers to
        SIG_IGN.
        * gdb.base/sigstep.exp (breakpoint_over_handler): Add
        with_sw_watch and no_handler parameters.  Handle them.
        (top level) <stepping over handler when stopped at a breakpoint
        test>: Add a test axis for testing with a software watchpoint, and
        another for testing with the signal handler set to SIG_IGN.
        * gdb.base/step-sw-breakpoint-adjust-pc.c: New file.
        * gdb.base/step-sw-breakpoint-adjust-pc.exp: New file.

-----------------------------------------------------------------------

Summary of changes:
 gdb/ChangeLog                                      |   11 +++
 gdb/gdbthread.h                                    |    5 +
 gdb/infrun.c                                       |   32 ++++++-
 gdb/testsuite/ChangeLog                            |   14 +++
 gdb/testsuite/gdb.base/sigstep.c                   |    5 +-
 gdb/testsuite/gdb.base/sigstep.exp                 |   45 ++++++++-
 .../gdb.base/step-sw-breakpoint-adjust-pc.c        |   50 +++++++++++
 .../gdb.base/step-sw-breakpoint-adjust-pc.exp      |   94 ++++++++++++++++++++
 8 files changed, 246 insertions(+), 10 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.c
 create mode 100644 gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.exp

-- 
You are receiving this mail because:
You are on the CC list for the bug.


  parent reply	other threads:[~2014-10-28 16:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-30 15:15 [Bug gdb/12623] New: " chris.hall at highwayman dot com
2011-03-30 15:45 ` [Bug gdb/12623] " chris.hall at highwayman dot com
2013-09-16 11:29 ` mwaqas at codesourcery dot com
2014-10-26 23:18 ` palves at redhat dot com
2014-10-26 23:18 ` palves at redhat dot com
2014-10-26 23:19 ` palves at redhat dot com
2014-10-26 23:20 ` palves at redhat dot com
2014-10-28 16:07 ` cvs-commit at gcc dot gnu.org [this message]
2014-10-28 16:09 ` palves at redhat dot com
2014-11-05 18:40 ` palves at redhat dot com
2014-12-25  0:46 ` cvs-commit at gcc dot gnu.org

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=bug-12623-4717-5U1aQu5xkA@http.sourceware.org/bugzilla/ \
    --to=sourceware-bugzilla@sourceware.org \
    --cc=gdb-prs@sourceware.org \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).