From patchwork Thu Sep 18 02:30:40 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 2903 Received: (qmail 7353 invoked by alias); 18 Sep 2014 02:34:59 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 7332 invoked by uid 89); 18 Sep 2014 02:34:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 18 Sep 2014 02:34:52 +0000 Received: from svr-orw-fem-04.mgc.mentorg.com ([147.34.97.41]) by relay1.mentorg.com with esmtp id 1XURYX-0001zY-4n from Yao_Qi@mentor.com ; Wed, 17 Sep 2014 19:34:49 -0700 Received: from GreenOnly (147.34.91.1) by svr-orw-fem-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server id 14.3.181.6; Wed, 17 Sep 2014 19:34:48 -0700 From: Yao Qi To: Pedro Alves CC: Subject: Re: [PATCH] Honour SIGILL and SIGSEGV in cancel breakpoint References: <1410696393-29327-1-git-send-email-yao@codesourcery.com> <54182945.7090300@redhat.com> Date: Thu, 18 Sep 2014 10:30:40 +0800 In-Reply-To: <54182945.7090300@redhat.com> (Pedro Alves's message of "Tue, 16 Sep 2014 13:12:53 +0100") Message-ID: <87mw9xzmlr.fsf@codesourcery.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Pedro Alves 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 , a new function lp_status_is_sigtrap_like_event is added. 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. I post the updated patch to fix the issue we've seen on canceling breakpoint. 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))