From: Matt Evans <hidden> Date: 2018-03-26 16:55:27
When using SIG_DBG_BRANCH_TRACING, MSR.BE is left enabled in the
user context when single_step_exception() prepares the SIGTRAP
delivery. The resulting branch-trap-within-the-SIGTRAP-handler
isn't healthy.
Commit 2538c2d08f46141550a1e68819efa8fe31c6e3dc broke this, by
replacing an MSR mask operation of ~(MSR_SE | MSR_BE) with a call
to clear_single_step() which only clears MSR_SE.
This patch adds a new helper, clear_br_trace(), which clears the
debug trap before invoking the signal handler. This helper is a
NOP for BookE as SIG_DBG_BRANCH_TRACING isn't supported on BookE.
Signed-off-by: Matt Evans <redacted>
---
arch/powerpc/kernel/traps.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
@@ -460,7 +460,7 @@ static inline int check_io_access(struct pt_regs *regs)/* single-step stuff */#define single_stepping(regs) (current->thread.debug.dbcr0 & DBCR0_IC)#define clear_single_step(regs) (current->thread.debug.dbcr0 &= ~DBCR0_IC)-+#define clear_br_trace(regs) do {} while(0)#else/* On non-4xx, the reason for the machine check or programexceptionisintheMSR.*/
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2018-03-28 05:54:30
Matt Evans [off-list ref] writes:
When using SIG_DBG_BRANCH_TRACING, MSR.BE is left enabled in the
user context when single_step_exception() prepares the SIGTRAP
delivery. The resulting branch-trap-within-the-SIGTRAP-handler
isn't healthy.
Commit 2538c2d08f46141550a1e68819efa8fe31c6e3dc broke this, by
replacing an MSR mask operation of ~(MSR_SE | MSR_BE) with a call
to clear_single_step() which only clears MSR_SE.
This patch adds a new helper, clear_br_trace(), which clears the
debug trap before invoking the signal handler. This helper is a
NOP for BookE as SIG_DBG_BRANCH_TRACING isn't supported on BookE.
Signed-off-by: Matt Evans <redacted>
Hi Matt!
It seems we might not be regularly testing this code :}
How did you hit/find the bug? And do you have a test case by any chance?
I found the test code at the bottom of:
https://lwn.net/Articles/114587/
But it didn't immediately work.
cheers
From: Matt Evans <hidden> Date: 2018-03-28 10:36:10
Howdy Michael,
On 28 Mar 2018, at 06:54, Michael Ellerman [off-list ref] wrote:
=20
Matt Evans [off-list ref] writes:
=20
quoted
When using SIG_DBG_BRANCH_TRACING, MSR.BE is left enabled in the
user context when single_step_exception() prepares the SIGTRAP
delivery. The resulting branch-trap-within-the-SIGTRAP-handler
isn't healthy.
=20
Commit 2538c2d08f46141550a1e68819efa8fe31c6e3dc broke this, by
replacing an MSR mask operation of ~(MSR_SE | MSR_BE) with a call
to clear_single_step() which only clears MSR_SE.
=20
This patch adds a new helper, clear_br_trace(), which clears the
debug trap before invoking the signal handler. This helper is a
NOP for BookE as SIG_DBG_BRANCH_TRACING isn't supported on BookE.
=20
Signed-off-by: Matt Evans <redacted>
=20
Hi Matt!
=20
It seems we might not be regularly testing this code :}
I know, rite? ;-)
How did you hit/find the bug? And do you have a test case by any =
I'm using this feature as part of a debug harness I wrote to log a =
program=E2=80=99s control flow (to create a =E2=80=9Cknown good=E2=80=9D =
pattern to compare a PPC interpreter against). So at least the feature =
has /one/ user. ;-)
The symptoms of the bug are that if you use single-stepping you get a =
sequence of SIGTRAPs representing each instruction completion (good), =
but if you use branch tracing the process just dies with SIGTRAP (looks =
like it=E2=80=99s never caught by the signal handler). What=E2=80=99s =
really happening is that there /is/ a signal delivered to the handler, =
but (because branch tracing is left on) that then causes a second debug =
exception from the handler itself, i.e. whilst SIGTRAP=E2=80=99s masked.
OK, let me have a dig to reduce my program to something very basic and =
I=E2=80=99ll post something =E2=80=94 sorry, I should=E2=80=99ve got a =
PoC ready before. (I did start out inspired by that post you linked to, =
but IIRC I don=E2=80=99t think it worked out of the box for me either.)
Cheers,
Matt
From: Matt Evans <hidden> Date: 2018-03-29 11:54:39
Hi Michael,
On 28 Mar 2018, at 11:36, Matt Evans [off-list ref] wrote:
=20
Howdy Michael,
=20
quoted
On 28 Mar 2018, at 06:54, Michael Ellerman [off-list ref] =
wrote:
quoted
=20
Matt Evans [off-list ref] writes:
=20
quoted
When using SIG_DBG_BRANCH_TRACING, MSR.BE is left enabled in the
user context when single_step_exception() prepares the SIGTRAP
delivery. The resulting branch-trap-within-the-SIGTRAP-handler
isn't healthy.
=20
Commit 2538c2d08f46141550a1e68819efa8fe31c6e3dc broke this, by
replacing an MSR mask operation of ~(MSR_SE | MSR_BE) with a call
to clear_single_step() which only clears MSR_SE.
=20
This patch adds a new helper, clear_br_trace(), which clears the
debug trap before invoking the signal handler. This helper is a
NOP for BookE as SIG_DBG_BRANCH_TRACING isn't supported on BookE.
=20
Signed-off-by: Matt Evans <redacted>
=20
Hi Matt!
=20
It seems we might not be regularly testing this code :}
=20
I know, rite? ;-)
=20
quoted
How did you hit/find the bug? And do you have a test case by any =
=20
I'm using this feature as part of a debug harness I wrote to log a =
program=E2=80=99s control flow (to create a =E2=80=9Cknown good=E2=80=9D =
pattern to compare a PPC interpreter against). So at least the feature =
has /one/ user. ;-)
=20
The symptoms of the bug are that if you use single-stepping you get a =
sequence of SIGTRAPs representing each instruction completion (good), =
but if you use branch tracing the process just dies with SIGTRAP (looks =
like it=E2=80=99s never caught by the signal handler). What=E2=80=99s =
really happening is that there /is/ a signal delivered to the handler, =
but (because branch tracing is left on) that then causes a second debug =
exception from the handler itself, i.e. whilst SIGTRAP=E2=80=99s masked.
=20
OK, let me have a dig to reduce my program to something very basic and =
I=E2=80=99ll post something =E2=80=94 sorry, I should=E2=80=99ve got a =
PoC ready before. (I did start out inspired by that post you linked to, =
but IIRC I don=E2=80=99t think it worked out of the box for me either.)
I=E2=80=99ve put a simple SIG_DBG_BRANCH_TRACING test program here:
http://ozlabs.org/~matt/files/sig_dbg_brtrace_test.c
It=E2=80=99s commented regarding expected output. I=E2=80=99ve only =
tested this on a G4 =E2=80=94 it should work on PPC64 too but the ISA =
says support for branch tracing is optional for an implementation. =
I=E2=80=99d be interested in what POWERx does. :)
Cheers,
Matt
From: Michael Ellerman <hidden> Date: 2018-04-03 16:03:43
On Mon, 2018-03-26 at 16:55:21 UTC, Matt Evans wrote:
When using SIG_DBG_BRANCH_TRACING, MSR.BE is left enabled in the
user context when single_step_exception() prepares the SIGTRAP
delivery. The resulting branch-trap-within-the-SIGTRAP-handler
isn't healthy.
Commit 2538c2d08f46141550a1e68819efa8fe31c6e3dc broke this, by
replacing an MSR mask operation of ~(MSR_SE | MSR_BE) with a call
to clear_single_step() which only clears MSR_SE.
This patch adds a new helper, clear_br_trace(), which clears the
debug trap before invoking the signal handler. This helper is a
NOP for BookE as SIG_DBG_BRANCH_TRACING isn't supported on BookE.
Signed-off-by: Matt Evans <redacted>
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2018-04-04 06:11:40
Matt Evans [off-list ref] writes:
quoted
On 28 Mar 2018, at 11:36, Matt Evans [off-list ref] wrote:
quoted
On 28 Mar 2018, at 06:54, Michael Ellerman [off-list ref] wrote:
Matt Evans [off-list ref] writes:
quoted
When using SIG_DBG_BRANCH_TRACING, MSR.BE is left enabled in the
user context when single_step_exception() prepares the SIGTRAP
delivery. The resulting branch-trap-within-the-SIGTRAP-handler
isn't healthy.
=20
Commit 2538c2d08f46141550a1e68819efa8fe31c6e3dc broke this, by
replacing an MSR mask operation of ~(MSR_SE | MSR_BE) with a call
to clear_single_step() which only clears MSR_SE.
=20
This patch adds a new helper, clear_br_trace(), which clears the
debug trap before invoking the signal handler. This helper is a
NOP for BookE as SIG_DBG_BRANCH_TRACING isn't supported on BookE.
=20
Signed-off-by: Matt Evans <redacted>
=20
Hi Matt!
=20
It seems we might not be regularly testing this code :}
=20
I know, rite? ;-)
=20
quoted
How did you hit/find the bug? And do you have a test case by any chance?
=20
I found the test code at the bottom of:
https://lwn.net/Articles/114587/
=20
But it didn't immediately work.
=20
I'm using this feature as part of a debug harness I wrote to log a
program=E2=80=99s control flow (to create a =E2=80=9Cknown good=E2=80=9D=
pattern to compare a
quoted
PPC interpreter against). So at least the feature has /one/ user. ;-)
One is enough for us :)
=20
quoted
The symptoms of the bug are that if you use single-stepping you get a
sequence of SIGTRAPs representing each instruction completion (good),
but if you use branch tracing the process just dies with SIGTRAP
(looks like it=E2=80=99s never caught by the signal handler). What=E2=80=
=99s really
quoted
happening is that there /is/ a signal delivered to the handler, but
(because branch tracing is left on) that then causes a second debug
exception from the handler itself, i.e. whilst SIGTRAP=E2=80=99s masked.
=20
OK, let me have a dig to reduce my program to something very basic
and I=E2=80=99ll post something =E2=80=94 sorry, I should=E2=80=99ve got=
a PoC ready before.
quoted
(I did start out inspired by that post you linked to, but IIRC I
don=E2=80=99t think it worked out of the box for me either.)
It=E2=80=99s commented regarding expected output. I=E2=80=99ve only teste=
d this on a
G4 =E2=80=94 it should work on PPC64 too but the ISA says support for bra=
nch
tracing is optional for an implementation. I=E2=80=99d be interested in w=
hat
POWERx does. :)
I get no traps on any 64-bit machine I tried, including 970FX, Power6,
Power7, Power8, Power9.
So I guess it's never been implemented on "server" CPUs.
I'd be happy to turn your test program into a selftest, though it won't
actually catch bugs unless someone runs it on actual 32-bit systems :)
cheers