From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26543 invoked by alias); 31 Jan 2004 17:49:53 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 26506 invoked from network); 31 Jan 2004 17:49:51 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 31 Jan 2004 17:49:51 -0000 Received: from drow by nevyn.them.org with local (Exim 4.30 #1 (Debian)) id 1AmzFd-0001VO-T8; Sat, 31 Jan 2004 12:49:49 -0500 Date: Sat, 31 Jan 2004 17:49:00 -0000 From: Daniel Jacobowitz To: Eli Zaretskii , gdb-patches@sources.redhat.com Subject: Re: RFC: Centralize DECR_PC_AFTER_BREAK handling from infrun Message-ID: <20040131174949.GA5154@nevyn.them.org> Mail-Followup-To: Eli Zaretskii , gdb-patches@sources.redhat.com References: <20040117222007.GA23563@nevyn.them.org> <20040118151909.GA17039@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20040118151909.GA17039@nevyn.them.org> User-Agent: Mutt/1.5.1i X-SW-Source: 2004-01/txt/msg00777.txt.bz2 On Sun, Jan 18, 2004 at 10:19:10AM -0500, Daniel Jacobowitz wrote: > > > + /* If we've hit a breakpoint, we'll be stopped with SIGTRAP. */ > > > + if (ecs->ws.kind != TARGET_WAITKIND_STOPPED) > > > + return; > > > + > > > + if (ecs->ws.value.sig != TARGET_SIGNAL_TRAP) > > > + return; > > > > The original code didn't check these conditions, right? So why add > > them here? (Also, the comment doesn't seem to describe the two > > tests, only the second one.) > > The comment does describe both tests; if != TARGET_WAITKIND_STOPPED, > then we aren't stopped by a signal. The other waitkinds correspond to > things like exiting and catchpoints, and with the exception of some > complications in the FORKED/EXECD cases, stop_signal will not get set > to SIGTRAP. Also, the original code did check these conditions, though > somewhat indirectly: > > if (stop_signal == TARGET_SIGNAL_TRAP) > { > /* Check if a regular breakpoint has been hit before checking > for a potential single step breakpoint. Otherwise, GDB will > not see this breakpoint hit when stepping onto breakpoints. */ > > Oh, hum, that's for the first set (thread hit thread-specific BP for a > different thread). The second thread does this: > > if (stop_signal == TARGET_SIGNAL_TRAP > || (breakpoints_inserted && > (stop_signal == TARGET_SIGNAL_ILL > || stop_signal == TARGET_SIGNAL_EMT)) > || stop_soon == STOP_QUIETLY > || stop_soon == STOP_QUIETLY_NO_SIGSTOP) > > The stop_soon's aren't relevant here, since they're handled before > DECR_PC_AFTER_BREAK, but the ILL/EMT are relevant. They should be > added to adjust_pc_after_break - thanks! Actually, I'm not sure they should. The question is, on an operating system such that hitting a breakpoint causes SIGILL, should DECR_PC_AFTER_BREAK be applied? I don't have a machine such that breakpoints cause SIGILL, but I tested x86 and alpha (both DECR_PC_AFTER_BREAK targets). On x86 a SIGILL leaves the PC pointing at the beginning of the illegal instruction, and on Alpha a SIGILL leaves the PC pointing to the second instruction after the illegal one. In neither case does that match the behavior of DECR_PC_AFTER_BREAK. So this will become a comment also. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer