From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@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 02:34:00 -0000 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 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg00610.txt.bz2 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 =3D=3D TARGET_WAITKIND_IGNO= RE" 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 =3D=3D 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 breakpoin= t. --=20 Yao (=E9=BD=90=E5=B0=A7) From: Yao Qi Subject: [PATCH] Honour SIGILL and SIGSEGV in cancel breakpoint I see the following fail on arm-none-linux-gnueabi testing, (gdb) continue^M Continuing.^M ^M Program received signal SIGILL, Illegal instruction.^M [Switching to Thread 1003]^M handler (signo=3D10) 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 =3D (supports_breakpoints () && (WSTOPSIG (w) =3D=3D SIGTRAP || ((WSTOPSIG (w) =3D=3D SIGILL || WSTOPSIG (w) =3D=3D 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. gdb/gdbserver: 2014-09-18 Yao Qi * linux-low.c (lp_status_is_sigtrap_like_event): New function. (cancel_breakpoints_callback): Call lp_status_is_sigtrap_like_event. (linux_low_filter_event): Likewise. 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; } =20 +/* 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) =3D=3D SIGTRAP + /* SIGILL and SIGSEGV are also treated as traps in case a + breakpoint is inserted at the current PC. */ + || WSTOPSIG (lp->status_pending) =3D=3D SIGILL + || WSTOPSIG (lp->status_pending) =3D=3D 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) =3D=3D 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_ent= ry *entry, void *data) =20 if (thread->last_resume_kind !=3D resume_stop && thread->last_status.kind =3D=3D TARGET_WAITKIND_IGNORE - && lp->status_pending_p - && WIFSTOPPED (lp->status_pending) - && WSTOPSIG (lp->status_pending) =3D=3D SIGTRAP + && lp_status_is_sigtrap_like_event (lp) && !lp->stepping && !lp->stopped_by_watchpoint && cancel_breakpoint (lp))