Honour SIGILL and SIGSEGV in cancel breakpoint
Commit Message
I see the following fail on are-none-linux-gnueabi testing,
(gdb) continue^M
Continuing.^M
^M
Program received signal SIGILL, Illegal instruction.^M
[Switching to Thread 1003]^M
handler (signo=10) at
/scratch/yqi/arm-none-linux-gnueabi/src/gdb-trunk/gdb/testsuite/gdb.threads/sigstep-threads.c:33^M
33 tgkill (getpid (), gettid (), SIGUSR1); /* step-2 */^M
(gdb) FAIL: gdb.threads/sigstep-threads.exp: continue
the cause is that GDBserver doesn't cancel the breakpoint if the stop
signal is SIGILL. The kernel used here is a little old, 2.6.x, and
doesn't translate SIGILL to SIGTRAP when program hits breakpoint
instruction (which is an illegal instruction actually). GDB and
GDBserver can translate SIGILL to SIGTRAP under certain circumstance,
so it is not a problem here. See gdbserver/linux-low.c:linux_wait_1
/* If this event was not handled before, and is not a SIGTRAP, we
report it. SIGILL and SIGSEGV are also treated as traps in case
a breakpoint is inserted at the current PC. If this target does
not support internal breakpoints at all, we also report the
SIGTRAP without further processing; it's of no concern to us. */
maybe_internal_trap
= (supports_breakpoints ()
&& (WSTOPSIG (w) == SIGTRAP
|| ((WSTOPSIG (w) == SIGILL
|| WSTOPSIG (w) == SIGSEGV)
&& (*the_low_target.breakpoint_at) (event_child->stop_pc))));
However, SIGILL and SIGSEGV is not considered when cancelling
breakpoint, which causes the fail above. That is, when GDB is doing
software single step on address ADDR, both thread A and thread B hits the
software single step breakpoint, and get SIGILL. GDB selects the event
from thread A, removes the software single step breakpoint, and resume
the program. The event (SIGILL) from thread B is reported to GDB, but
GDB doesn't regard this SIGILL as SIGTRAP, because the breakpoint on
address ADDR was removed, so GDB reports "Program received signal
SIGILL".
The patch is to allow calling cancel_breakpoint if the signal is
SIGILL and SIGSEGV. This patch fixes the fail above.
Regression tested on arm-none-linux-gnueabi and x86_64-linux.
gdb/gdbserver:
2014-09-12 Yao Qi <yao@codesourcery.com>
* linux-low.c (cancel_breakpoints_callback): Allow calling
cancel_breakpoint if stop signal is SIGILL or SIGSEGV.
(linux_low_filter_event): Likewise.
---
gdb/gdbserver/linux-low.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
Comments
On 09/14/2014 01:06 PM, Yao Qi wrote:
> I see the following fail on are-none-linux-gnueabi testing,
>
> (gdb) continue^M
> Continuing.^M
> ^M
> Program received signal SIGILL, Illegal instruction.^M
> [Switching to Thread 1003]^M
> handler (signo=10) at
> /scratch/yqi/arm-none-linux-gnueabi/src/gdb-trunk/gdb/testsuite/gdb.threads/sigstep-threads.c:33^M
> 33 tgkill (getpid (), gettid (), SIGUSR1); /* step-2 */^M
> (gdb) FAIL: gdb.threads/sigstep-threads.exp: continue
>
> the cause is that GDBserver doesn't cancel the breakpoint if the stop
> signal is SIGILL. The kernel used here is a little old, 2.6.x, and
> doesn't translate SIGILL to SIGTRAP when program hits breakpoint
> instruction (which is an illegal instruction actually). GDB and
> GDBserver can translate SIGILL to SIGTRAP under certain circumstance,
> so it is not a problem here. See gdbserver/linux-low.c:linux_wait_1
>
> /* If this event was not handled before, and is not a SIGTRAP, we
> report it. SIGILL and SIGSEGV are also treated as traps in case
> a breakpoint is inserted at the current PC. If this target does
> not support internal breakpoints at all, we also report the
> SIGTRAP without further processing; it's of no concern to us. */
> maybe_internal_trap
> = (supports_breakpoints ()
> && (WSTOPSIG (w) == SIGTRAP
> || ((WSTOPSIG (w) == SIGILL
> || WSTOPSIG (w) == SIGSEGV)
> && (*the_low_target.breakpoint_at) (event_child->stop_pc))));
>
> However, SIGILL and SIGSEGV is not considered when cancelling
> breakpoint, which causes the fail above. That is, when GDB is doing
> software single step on address ADDR, both thread A and thread B hits the
> software single step breakpoint, and get SIGILL. GDB selects the event
> from thread A, removes the software single step breakpoint, and resume
> the program. The event (SIGILL) from thread B is reported to GDB, but
> GDB doesn't regard this SIGILL as SIGTRAP, because the breakpoint on
> address ADDR was removed, so GDB reports "Program received signal
> SIGILL".
>
> The patch is to allow calling cancel_breakpoint if the signal is
> SIGILL and SIGSEGV. This patch fixes the fail above.
>
> Regression tested on arm-none-linux-gnueabi and x86_64-linux.
>
> gdb/gdbserver:
>
> 2014-09-12 Yao Qi <yao@codesourcery.com>
>
> * linux-low.c (cancel_breakpoints_callback): Allow calling
> cancel_breakpoint if stop signal is SIGILL or SIGSEGV.
> (linux_low_filter_event): Likewise.
> ---
> gdb/gdbserver/linux-low.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index ec3260e..e2c9814 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -1936,7 +1936,12 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
> the core before this one is handled. All-stop always
> cancels breakpoint hits in all threads. */
> if (non_stop
> - && WSTOPSIG (wstat) == SIGTRAP
> + && (WSTOPSIG (wstat) == SIGTRAP
> + /* SIGILL and SIGSEGV are also treated as traps in
> + case a breakpoint is inserted at the current PC,
> + which is checked in cancel_breakpoints below. */
> + || WSTOPSIG (wstat) == SIGILL
> + || WSTOPSIG (wstat) == SIGSEGV)
> && cancel_breakpoint (child))
> {
> /* Throw away the SIGTRAP. */
> @@ -2273,7 +2278,12 @@ cancel_breakpoints_callback (struct inferior_list_entry *entry, void *data)
> && thread->last_status.kind == TARGET_WAITKIND_IGNORE
> && lp->status_pending_p
> && WIFSTOPPED (lp->status_pending)
> - && WSTOPSIG (lp->status_pending) == SIGTRAP
> + && (WSTOPSIG (lp->status_pending) == SIGTRAP
> + /* SIGILL and SIGSEGV are also treated as traps in case a
> + breakpoint is inserted at the current PC, which is checked
> + in cancel_breakpoints below. */
> + || WSTOPSIG (lp->status_pending) == SIGILL
> + || WSTOPSIG (lp->status_pending) == SIGSEGV)
> && !lp->stepping
> && !lp->stopped_by_watchpoint
> && cancel_breakpoint (lp))
>
Instead of duplicating the code and comments, please factor out
the SIGTRAP+SIGILL+SIGSEGVs checks to a helper function. On the GDB side,
we have linux_nat_lp_status_is_event, and we see that it's also used
on count_count_events_callback (which gdbserver also has), which makes
sense, as it's counting threads that had breakpoint SIGTRAP-ish
events (though I'm not sure why we only consider breakpoints when
selecting the event lwp).
Thanks,
Pedro Alves
@@ -1936,7 +1936,12 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
the core before this one is handled. All-stop always
cancels breakpoint hits in all threads. */
if (non_stop
- && WSTOPSIG (wstat) == SIGTRAP
+ && (WSTOPSIG (wstat) == SIGTRAP
+ /* SIGILL and SIGSEGV are also treated as traps in
+ case a breakpoint is inserted at the current PC,
+ which is checked in cancel_breakpoints below. */
+ || WSTOPSIG (wstat) == SIGILL
+ || WSTOPSIG (wstat) == SIGSEGV)
&& cancel_breakpoint (child))
{
/* Throw away the SIGTRAP. */
@@ -2273,7 +2278,12 @@ cancel_breakpoints_callback (struct inferior_list_entry *entry, void *data)
&& thread->last_status.kind == TARGET_WAITKIND_IGNORE
&& lp->status_pending_p
&& WIFSTOPPED (lp->status_pending)
- && WSTOPSIG (lp->status_pending) == SIGTRAP
+ && (WSTOPSIG (lp->status_pending) == SIGTRAP
+ /* SIGILL and SIGSEGV are also treated as traps in case a
+ breakpoint is inserted at the current PC, which is checked
+ in cancel_breakpoints below. */
+ || WSTOPSIG (lp->status_pending) == SIGILL
+ || WSTOPSIG (lp->status_pending) == SIGSEGV)
&& !lp->stepping
&& !lp->stopped_by_watchpoint
&& cancel_breakpoint (lp))