From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13304 invoked by alias); 16 Sep 2014 12:13:01 -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 13293 invoked by uid 89); 16 Sep 2014 12:13:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 16 Sep 2014 12:12:59 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8GCCuxg013236 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 16 Sep 2014 08:12:56 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8GCCrs6004365; Tue, 16 Sep 2014 08:12:54 -0400 Message-ID: <54182945.7090300@redhat.com> Date: Tue, 16 Sep 2014 12:13:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.0 MIME-Version: 1.0 To: Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH] Honour SIGILL and SIGSEGV in cancel breakpoint References: <1410696393-29327-1-git-send-email-yao@codesourcery.com> In-Reply-To: <1410696393-29327-1-git-send-email-yao@codesourcery.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-09/txt/msg00532.txt.bz2 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 > > * 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