[RFC] Fix SW breakpoint handling for Cell multi-arch
Commit Message
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
Comments
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
===================================================================
@@ -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;