| Message ID | 87mw9xzmlr.fsf@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
On 09/18/2014 03:30 AM, Yao Qi wrote: > Pedro Alves <palves@redhat.com> writes: > >> 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). > > I take a look at linux_nat_lp_status_is_event and email discussions on > adding this function <https://sourceware.org/ml/gdb-patches/2010-07/msg00275.html>, > a new function lp_status_is_sigtrap_like_event is added. I think something with "breakpoint" in the name, like lp_status_maybe_breakpoint would be even clearer. The event is SIGTRAP-like only in the sense that it may signal a breakpoint like SIGTRAP does. A SIGILL is not sigtrap-like for single-steps, for example. > I don't use > the same name because I feel linux_nat_lp_status_is_event isn't clear > enough. Secondly, I don't use "waitstatus.kind == TARGET_WAITKIND_IGNORE" > condition check inside lp_status_is_sigtrap_like_event, because IMO it > was used in linux_nat_lp_status_is_event due to lack of lp->status_p > flag, as the comments described. However, in GDBserver, we have > status_pending_p flag, so "waitstatus.kind == TARGET_WAITKIND_IGNORE" is > not needed. > > count_events_callback and select_event_lwp_callback in GDBServer need to > honour SIGILL and SIGSEGV too. I write a patch to call > lp_status_is_sigtrap_like_event in them, but regression test result > isn't changed, which is a surprise to me. I thought some fails should > be fixed. I'll look into it deeply. Maybe you're getting lucky with scheduling. pthreads.exp and schedlock.exp I think are the most sensitive to this. See: https://www.sourceware.org/ml/gdb-patches/2001-06/msg00250.html Thanks, Pedro Alves
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index ec3260e..9c9a303 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -1739,6 +1739,20 @@ cancel_breakpoint (struct lwp_info *lwp) return 0; } +/* Check for SIGTRAP-like events in LP. */ + +static int +lp_status_is_sigtrap_like_event (struct lwp_info *lp) +{ + return (lp->status_pending_p + && WIFSTOPPED (lp->status_pending) + && (WSTOPSIG (lp->status_pending) == SIGTRAP + /* SIGILL and SIGSEGV are also treated as traps in case a + breakpoint is inserted at the current PC. */ + || WSTOPSIG (lp->status_pending) == SIGILL + || WSTOPSIG (lp->status_pending) == SIGSEGV)); +} + /* Do low-level handling of the event, and check if we should go on and pass it to caller code. Return the affected lwp if we are, or NULL otherwise. */ @@ -1936,7 +1950,7 @@ 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 + && lp_status_is_sigtrap_like_event (child) && cancel_breakpoint (child)) { /* Throw away the SIGTRAP. */ @@ -2271,9 +2285,7 @@ cancel_breakpoints_callback (struct inferior_list_entry *entry, void *data) if (thread->last_resume_kind != resume_stop && thread->last_status.kind == TARGET_WAITKIND_IGNORE - && lp->status_pending_p - && WIFSTOPPED (lp->status_pending) - && WSTOPSIG (lp->status_pending) == SIGTRAP + && lp_status_is_sigtrap_like_event (lp) && !lp->stepping && !lp->stopped_by_watchpoint && cancel_breakpoint (lp))