* [RFC] Fix SW breakpoint handling for Cell multi-arch
@ 2015-08-27 11:54 Ulrich Weigand
2015-08-27 15:43 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Ulrich Weigand @ 2015-08-27 11:54 UTC (permalink / raw)
To: palves; +Cc: gdb-patches
Hi Pedro,
a second major issue with Cell multi-arch debugging right now is related
to the new target-side SW breakpoint handling. Cell uses linux-nat as
primary target for the PowerPC side, which now returns true from the
to_supports_stopped_by_sw_breakpoint hook.
This works fine for the PowerPC side. However, when a breakpoint on the
SPU side is hit, the kernel does *not* provide a siginfo with TRAP_BRKPT,
but instead simply delivers a SIGTRAP without siginfo. The linux-nat
target therefore does not recognize the breakpoint and does decrement
the PC; this completely throws off infrun, which expects the target to
have done this.
The attached patch fixes this in the spu-multiarch target by overriding
to_wait as well as to to_stopped_by_sw_breakpoint, and handles the SPU
case there. This does seem to fix the problem for me.
Does this look reasonable to you, or do you have any other suggestions?
Thanks,
Ulrich
Index: binutils-gdb/gdb/spu-multiarch.c
===================================================================
--- binutils-gdb.orig/gdb/spu-multiarch.c
+++ binutils-gdb/gdb/spu-multiarch.c
@@ -311,6 +311,76 @@ spu_search_memory (struct target_ops* op
pattern, pattern_len, found_addrp);
}
+/* Override the to_wait routine. */
+static ptid_t
+spu_wait (struct target_ops *ops,
+ ptid_t ptid, struct target_waitstatus *ourstatus,
+ int target_options)
+{
+ struct target_ops *ops_beneath = find_target_beneath (ops);
+ ptid_t event_ptid;
+
+ event_ptid = ops_beneath->to_wait (ops_beneath, ptid,
+ ourstatus, target_options);
+
+ /* Special handling is only necessary if we got some trap. */
+ if (ourstatus->kind != TARGET_WAITKIND_STOPPED
+ || ourstatus->value.sig != GDB_SIGNAL_TRAP)
+ return event_ptid;
+
+ /* If the target is supposed to recognize SW breakpoints, we have a problem.
+ While linux-nat handles this just fine on the PPU side, the kernel does
+ not report SW breakpoints on the SPU side. So we detect this case and
+ handle it manually here. */
+ if (ops_beneath->to_supports_stopped_by_sw_breakpoint (ops_beneath))
+ {
+ struct regcache *regcache = get_thread_regcache (event_ptid);
+ CORE_ADDR pc = regcache_read_pc (regcache);
+
+ if (SPUADDR_SPU (pc) >= 0)
+ {
+ struct address_space *aspace = get_regcache_aspace (regcache);
+
+ /* We're supposed to perform decr_pc_after_break processing in the
+ target_wait routine, so we do it here. The following code is
+ basically equivalent to adjust_pc_after_break, but makes some
+ assumptions that are always true on SPU: decr_pc_after_break is
+ a nonzero value, and hardware single-stepping is unsupported. */
+ pc -= gdbarch_decr_pc_after_break (get_regcache_arch (regcache));
+
+ if (software_breakpoint_inserted_here_p (aspace, pc)
+ || (target_is_non_stop_p ()
+ && moribund_breakpoint_here_p (aspace, pc)))
+ regcache_write_pc (regcache, pc);
+ }
+ }
+
+ return event_ptid;
+}
+
+/* Override the to_stopped_by_sw_breakpoint routine. */
+static int
+spu_stopped_by_sw_breakpoint (struct target_ops *ops)
+{
+ struct target_ops *ops_beneath = find_target_beneath (ops);
+
+ /* We have to handle this here if we stopped on the SPU; see spu_wait. */
+ struct regcache *regcache = get_thread_regcache (inferior_ptid);
+ CORE_ADDR pc = regcache_read_pc (regcache);
+
+ if (SPUADDR_SPU (pc) >= 0)
+ {
+ struct address_space *aspace = get_regcache_aspace (regcache);
+
+ /* Note that decr_pc_after_break processing was done in spu_wait. */
+ return (software_breakpoint_inserted_here_p (aspace, pc)
+ || (target_is_non_stop_p ()
+ && moribund_breakpoint_here_p (aspace, pc)));
+ }
+
+ return ops_beneath->to_stopped_by_sw_breakpoint (ops_beneath);
+}
+
/* Push and pop the SPU multi-architecture support target. */
@@ -386,6 +456,8 @@ init_spu_ops (void)
spu_ops.to_xfer_partial = spu_xfer_partial;
spu_ops.to_search_memory = spu_search_memory;
spu_ops.to_region_ok_for_hw_watchpoint = spu_region_ok_for_hw_watchpoint;
+ spu_ops.to_wait = spu_wait;
+ spu_ops.to_stopped_by_sw_breakpoint = spu_stopped_by_sw_breakpoint;
spu_ops.to_thread_architecture = spu_thread_architecture;
spu_ops.to_stratum = arch_stratum;
spu_ops.to_magic = OPS_MAGIC;
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC] Fix SW breakpoint handling for Cell multi-arch
2015-08-27 11:54 [RFC] Fix SW breakpoint handling for Cell multi-arch Ulrich Weigand
@ 2015-08-27 15:43 ` Pedro Alves
2015-08-27 16:23 ` Ulrich Weigand
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2015-08-27 15:43 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On 08/27/2015 12:54 PM, Ulrich Weigand wrote:
> Hi Pedro,
>
> a second major issue with Cell multi-arch debugging right now is related
> to the new target-side SW breakpoint handling. Cell uses linux-nat as
> primary target for the PowerPC side, which now returns true from the
> to_supports_stopped_by_sw_breakpoint hook.
>
> This works fine for the PowerPC side. However, when a breakpoint on the
> SPU side is hit, the kernel does *not* provide a siginfo with TRAP_BRKPT,
> but instead simply delivers a SIGTRAP without siginfo.
Does si_code indicate that it was a kernel-generated SIGTRAP (that is,
SI_KERNEL)? Wondering whether that would still be distinguishable
from trace/single-step traps and user sent SIGTRAPs. See comment and
table about x86's si_code in nat/linux-nat.h. I don't know whether
the SPU has to care about all the cases there, but I suspect
not (e.g., I'd assume SPU code can't exec?).
If not, then we'll have to cope... :-/ . Any chance the kernel gets
fixed, in order for some future gdb stop worrying about this? I was
hoping to get rid of the moribund locations heuristic at some point.
> The linux-nat
> target therefore does not recognize the breakpoint and does decrement
> the PC; this completely throws off infrun, which expects the target to
> have done this.
>
> The attached patch fixes this in the spu-multiarch target by overriding
> to_wait as well as to to_stopped_by_sw_breakpoint, and handles the SPU
> case there. This does seem to fix the problem for me.
>
> Does this look reasonable to you, or do you have any other suggestions?
Looks reasonable to me, if the suggestion above leads nowhere...
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] Fix SW breakpoint handling for Cell multi-arch
2015-08-27 15:43 ` Pedro Alves
@ 2015-08-27 16:23 ` Ulrich Weigand
2015-08-27 16:44 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Ulrich Weigand @ 2015-08-27 16:23 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves wrote:
> On 08/27/2015 12:54 PM, Ulrich Weigand wrote:
> > Hi Pedro,
> >
> > a second major issue with Cell multi-arch debugging right now is related
> > to the new target-side SW breakpoint handling. Cell uses linux-nat as
> > primary target for the PowerPC side, which now returns true from the
> > to_supports_stopped_by_sw_breakpoint hook.
> >
> > This works fine for the PowerPC side. However, when a breakpoint on the
> > SPU side is hit, the kernel does *not* provide a siginfo with TRAP_BRKPT,
> > but instead simply delivers a SIGTRAP without siginfo.
>
> Does si_code indicate that it was a kernel-generated SIGTRAP (that is,
> SI_KERNEL)? Wondering whether that would still be distinguishable
> from trace/single-step traps and user sent SIGTRAPs. See comment and
> table about x86's si_code in nat/linux-nat.h. I don't know whether
> the SPU has to care about all the cases there, but I suspect
> not (e.g., I'd assume SPU code can't exec?).
That's an interesting idea. Indeed the kernel uses SI_KERNEL for
SIGTRAPs indicating SW breakpoints on SPU, but nowhere else in all
of PowerPC code. This means simply accepting either TRAP_BRKPT or
SI_KERNEL should work. And indeed the patch appended below works
just as well as the original patch for me.
> If not, then we'll have to cope... :-/ . Any chance the kernel gets
> fixed, in order for some future gdb stop worrying about this? I was
> hoping to get rid of the moribund locations heuristic at some point.
There's probably no chance of changing the kernel at this point; Cell
is really just in maintenance mode at this point (the only supported
OS is RHEL 5).
Bye,
Ulrich
Index: binutils-gdb/gdb/gdbserver/linux-low.c
===================================================================
--- binutils-gdb.orig/gdb/gdbserver/linux-low.c
+++ binutils-gdb/gdb/gdbserver/linux-low.c
@@ -651,7 +651,7 @@ check_stopped_by_breakpoint (struct lwp_
{
if (siginfo.si_signo == SIGTRAP)
{
- if (siginfo.si_code == GDB_ARCH_TRAP_BRKPT)
+ if (GDB_ARCH_IS_TRAP_BRKPT (siginfo.si_code))
{
if (debug_threads)
{
Index: binutils-gdb/gdb/linux-nat.c
===================================================================
--- binutils-gdb.orig/gdb/linux-nat.c
+++ binutils-gdb/gdb/linux-nat.c
@@ -2801,7 +2801,7 @@ check_stopped_by_breakpoint (struct lwp_
{
if (siginfo.si_signo == SIGTRAP)
{
- if (siginfo.si_code == GDB_ARCH_TRAP_BRKPT)
+ if (GDB_ARCH_IS_TRAP_BRKPT (siginfo.si_code))
{
if (debug_linux_nat)
fprintf_unfiltered (gdb_stdlog,
Index: binutils-gdb/gdb/nat/linux-ptrace.h
===================================================================
--- binutils-gdb.orig/gdb/nat/linux-ptrace.h
+++ binutils-gdb/gdb/nat/linux-ptrace.h
@@ -135,12 +135,19 @@ struct buffer;
running to a breakpoint and checking what comes out of
siginfo->si_code.
- The generic Linux target code should use GDB_ARCH_TRAP_BRKPT
- instead of TRAP_BRKPT to abstract out this x86 peculiarity. */
+ The ppc kernel does use TRAP_BRKPT for software breakpoints
+ in PowerPC code, but it uses SI_KERNEL for software breakpoints
+ in SPU code on a Cell/B.E. However, SI_KERNEL is never seen
+ on a SIGTRAP for any other reason.
+
+ The generic Linux target code should use GDB_ARCH_IS_TRAP_BRKPT
+ instead of TRAP_BRKPT to abstract out these peculiarities. */
#if defined __i386__ || defined __x86_64__
-# define GDB_ARCH_TRAP_BRKPT SI_KERNEL
+# define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == SI_KERNEL)
+#elif defined __powerpc__
+# define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == SI_KERNEL || (X) == TRAP_BRKPT)
#else
-# define GDB_ARCH_TRAP_BRKPT TRAP_BRKPT
+# define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == TRAP_BRKPT)
#endif
#ifndef TRAP_HWBKPT
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC] Fix SW breakpoint handling for Cell multi-arch
2015-08-27 16:23 ` Ulrich Weigand
@ 2015-08-27 16:44 ` Pedro Alves
2015-08-27 17:40 ` Ulrich Weigand
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2015-08-27 16:44 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On 08/27/2015 05:23 PM, Ulrich Weigand wrote:
> Pedro Alves wrote:
>> On 08/27/2015 12:54 PM, Ulrich Weigand wrote:
>>> Hi Pedro,
>>>
>>> a second major issue with Cell multi-arch debugging right now is related
>>> to the new target-side SW breakpoint handling. Cell uses linux-nat as
>>> primary target for the PowerPC side, which now returns true from the
>>> to_supports_stopped_by_sw_breakpoint hook.
>>>
>>> This works fine for the PowerPC side. However, when a breakpoint on the
>>> SPU side is hit, the kernel does *not* provide a siginfo with TRAP_BRKPT,
>>> but instead simply delivers a SIGTRAP without siginfo.
>>
>> Does si_code indicate that it was a kernel-generated SIGTRAP (that is,
>> SI_KERNEL)? Wondering whether that would still be distinguishable
>> from trace/single-step traps and user sent SIGTRAPs. See comment and
>> table about x86's si_code in nat/linux-nat.h. I don't know whether
>> the SPU has to care about all the cases there, but I suspect
>> not (e.g., I'd assume SPU code can't exec?).
>
> That's an interesting idea. Indeed the kernel uses SI_KERNEL for
> SIGTRAPs indicating SW breakpoints on SPU, but nowhere else in all
> of PowerPC code. This means simply accepting either TRAP_BRKPT or
> SI_KERNEL should work. And indeed the patch appended below works
> just as well as the original patch for me.
Excellent!
> Index: binutils-gdb/gdb/gdbserver/linux-low.c
> ===================================================================
> --- binutils-gdb.orig/gdb/gdbserver/linux-low.c
> +++ binutils-gdb/gdb/gdbserver/linux-low.c
> @@ -651,7 +651,7 @@ check_stopped_by_breakpoint (struct lwp_
> {
> if (siginfo.si_signo == SIGTRAP)
> {
> - if (siginfo.si_code == GDB_ARCH_TRAP_BRKPT)
> + if (GDB_ARCH_IS_TRAP_BRKPT (siginfo.si_code))
> {
> if (debug_threads)
> {
> Index: binutils-gdb/gdb/linux-nat.c
> ===================================================================
> --- binutils-gdb.orig/gdb/linux-nat.c
> +++ binutils-gdb/gdb/linux-nat.c
> @@ -2801,7 +2801,7 @@ check_stopped_by_breakpoint (struct lwp_
> {
> if (siginfo.si_signo == SIGTRAP)
> {
> - if (siginfo.si_code == GDB_ARCH_TRAP_BRKPT)
> + if (GDB_ARCH_IS_TRAP_BRKPT (siginfo.si_code))
> {
> if (debug_linux_nat)
> fprintf_unfiltered (gdb_stdlog,
> Index: binutils-gdb/gdb/nat/linux-ptrace.h
> ===================================================================
> --- binutils-gdb.orig/gdb/nat/linux-ptrace.h
> +++ binutils-gdb/gdb/nat/linux-ptrace.h
> @@ -135,12 +135,19 @@ struct buffer;
> running to a breakpoint and checking what comes out of
> siginfo->si_code.
>
> - The generic Linux target code should use GDB_ARCH_TRAP_BRKPT
> - instead of TRAP_BRKPT to abstract out this x86 peculiarity. */
> + The ppc kernel does use TRAP_BRKPT for software breakpoints
> + in PowerPC code, but it uses SI_KERNEL for software breakpoints
> + in SPU code on a Cell/B.E. However, SI_KERNEL is never seen
> + on a SIGTRAP for any other reason.
> +
> + The generic Linux target code should use GDB_ARCH_IS_TRAP_BRKPT
> + instead of TRAP_BRKPT to abstract out these peculiarities. */
> #if defined __i386__ || defined __x86_64__
> -# define GDB_ARCH_TRAP_BRKPT SI_KERNEL
> +# define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == SI_KERNEL)
> +#elif defined __powerpc__
> +# define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == SI_KERNEL || (X) == TRAP_BRKPT)
> #else
> -# define GDB_ARCH_TRAP_BRKPT TRAP_BRKPT
> +# define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == TRAP_BRKPT)
> #endif
>
> #ifndef TRAP_HWBKPT
>
LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC] Fix SW breakpoint handling for Cell multi-arch
2015-08-27 16:44 ` Pedro Alves
@ 2015-08-27 17:40 ` Ulrich Weigand
0 siblings, 0 replies; 5+ messages in thread
From: Ulrich Weigand @ 2015-08-27 17:40 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves wrote:
> On 08/27/2015 05:23 PM, Ulrich Weigand wrote:
> > That's an interesting idea. Indeed the kernel uses SI_KERNEL for
> > SIGTRAPs indicating SW breakpoints on SPU, but nowhere else in all
> > of PowerPC code. This means simply accepting either TRAP_BRKPT or
> > SI_KERNEL should work. And indeed the patch appended below works
> > just as well as the original patch for me.
>
> Excellent!
I've pushed this as well.
Thanks,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-08-27 17:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-27 11:54 [RFC] Fix SW breakpoint handling for Cell multi-arch Ulrich Weigand
2015-08-27 15:43 ` Pedro Alves
2015-08-27 16:23 ` Ulrich Weigand
2015-08-27 16:44 ` Pedro Alves
2015-08-27 17:40 ` Ulrich Weigand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox