Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[Pedro Alves <pedro@codesourcery.com>]

On Friday 23 July 2010 23:19:35, Jan Kratochvil wrote:

> This SIGTRAP->SIGILL case happens only on ia64 and ia64 does not use any
> set_gdbarch_decr_pc_after_break at all, PC stays on the breakpoint bundle+slot
> in both the SIGTRAP and SIGILL case.
> 
> You are right it is arch-specific.  On i386 I checked SIGILL is never
> generated (only in some fpu-emulated code).  So I checked s390x-linux-gnu::
> 	SIGILL on opcode 0xb29e
> 	si_addr = 0x800009a4
> 	.psw.addr = 0x800009a8
> 	instr at = 0x800009a4
> 	.psw.addr - instr == 4

Oh?  Does this mean the PC is left pointing _after_ the instruction
that caused the SIGILL?

I think you still need to audit other bits in linux-nat.c for SIGTRAP bkpts
handling.  E.g., see stop_wait_callback.  (For extra correctness,
count_events_callback, and the select_event_lwp_callback functions would
be relaxed too.)  I wasn't previously suggesting to make this ia64 arch
specific, which made fixing these other places too easier.  Notice how
gdbserver/linux-low.c also considers non-sigtrap bkpts (and it was a recent
change, needed for ARM thumb2 kernels that hadn't learned about the
breakpoint insns yet, IIRC).

> --- a/gdb/ia64-linux-nat.c
> +++ b/gdb/ia64-linux-nat.c
> @@ -809,6 +809,26 @@ ia64_linux_xfer_partial (struct target_ops *ops,
>  			     offset, len);
>  }
>  
> +/* For break.b instruction ia64 CPU forgets the immediate value and generates
> +   SIGILL with ILL_ILLOPC instead of more common SIGTRAP with TRAP_BRKPT.  */
> +
> +static int
> +ia64_linux_cancel_breakpoint (struct lwp_info *lp)
> +{
> +  /* We check for lp->waitstatus in addition to lp->status, because we can
> +     have pending process exits recorded in lp->status
> +     and W_EXITCODE(0,0) == 0.  We should probably have an additional
> +     lp->status_p flag.  */
> +
> +  if (! (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
> +	 && WIFSTOPPED (lp->status)
> +	 && (WSTOPSIG (lp->status) == SIGTRAP
> +	     || WSTOPSIG (lp->status) == SIGILL)))
> +    return 0;
> +
> +  return linux_nat_cancel_breakpoint_when_signalled (lp);

If this stays, please add a comment above this call mentioning that
we can safely call this function even for SIGILL, since
decr_pc_after_break is 0 on ia64.  (Alternatively, recode to avoid
the assumption)  The point I'm making is that PC adjustment is
always only necessary for sigtraps, never other signals (and is in fact
wrong for other signals).

-- 
Pedro Alves