linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] arm64: Store breakpoint single step state into pstate
Date: Tue, 12 Jan 2016 17:06:50 +0000	[thread overview]
Message-ID: <20160112170650.GI15737@arm.com> (raw)
In-Reply-To: <568B4F47.5080307@huawei.com>

On Tue, Jan 05, 2016 at 01:06:15PM +0800, Wangnan (F) wrote:
> On 2016/1/5 0:55, Will Deacon wrote:
> >The problem seems to be that we take the debug exception before the
> >breakpointed instruction has been executed and call perf_bp_event at
> >that moment, so when we single-step the faulting instruction we actually
> >step into the SIGIO handler and end up getting stuck.
> >
> >Your fix doesn't really address this afaict, in that you don't (can't?)
> >handle:
> >
> >   * A longjmp out of a signal handler
> >   * A watchpoint and a breakpoint that fire on the same instruction
> >   * User-controlled single-step from a signal handler that enables a
> >     breakpoint explicitly
> >   * Nested signals
> 
> Please have a look at [1], which I improve test__bp_signal() to
> check bullet 2 and 4 you mentioned above. Seems my fix is correct.
> 
> [1] http://lkml.kernel.org/g/1451969880-14877-1-git-send-email-wangnan0 at huawei.com

I'm still really uneasy about this change. Pairing up the signal delivery
with the sigreturn to keep track of the debug state is extremely fragile
and I'm not keen on adding this logic there. I also think we need to
track the address that the breakpoint is originally taken on so that we
can only perform the extra sigreturn work if we're returning to the same
instruction. Furthermore, I wouldn't want to do this for signals other
than those generated directly by a breakpoint.

An alternative would be to postpone the signal delivery until after the
stepping has been taken care of, but that's a change in ABI and I worry
we'll break somebody relying on the current behaviour.

What exactly does x86 do? I couldn't figure it out from the code.

Will

  reply	other threads:[~2016-01-12 17:06 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-23  8:52 [RESEND PATCH] arm64: Store breakpoint single step state into pstate Wang Nan
2015-12-23 10:44 ` kbuild test robot
2015-12-24  1:42 ` [PATCH v2] " Wang Nan
2016-01-04 16:55   ` Will Deacon
2016-01-05  1:41     ` Wangnan (F)
2016-01-05  4:58     ` [RFC PATCH] arm64: perf test: Improbe bp_signal Wang Nan
2016-01-05  5:09       ` Wangnan (F)
2016-01-05  8:53       ` Jiri Olsa
2016-01-05  9:00       ` Jiri Olsa
2016-01-05  9:05       ` Jiri Olsa
2016-01-05  9:09       ` Jiri Olsa
2016-01-05  5:06     ` [PATCH v2] arm64: Store breakpoint single step state into pstate Wangnan (F)
2016-01-12 17:06       ` Will Deacon [this message]
2016-01-15  8:20         ` xiakaixu
2016-01-21  8:06           ` xiakaixu
2016-01-18 11:39         ` Wangnan (F)
2016-01-05  9:57     ` [RFC PATCH v2] perf test: Improve bp_signal Wang Nan
2016-01-05 10:07       ` Jiri Olsa

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=20160112170650.GI15737@arm.com \
    --to=will.deacon@arm.com \
    --cc=linux-arm-kernel@lists.infradead.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 NNTP newsgroup(s).