From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2054 invoked by alias); 2 Jan 2003 21:22:36 -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 2047 invoked from network); 2 Jan 2003 21:22:35 -0000 Received: from unknown (HELO crack.them.org) (65.125.64.184) by 209.249.29.67 with SMTP; 2 Jan 2003 21:22:35 -0000 Received: from nevyn.them.org ([66.93.61.169] ident=mail) by crack.them.org with asmtp (Exim 3.12 #1 (Debian)) id 18UEfx-0000q8-00; Thu, 02 Jan 2003 17:22:57 -0600 Received: from drow by nevyn.them.org with local (Exim 3.36 #1 (Debian)) id 18UCnO-00068z-00; Thu, 02 Jan 2003 16:22:30 -0500 Date: Thu, 02 Jan 2003 21:22:00 -0000 From: Daniel Jacobowitz To: Michael Snyder Cc: gdb-patches@sources.redhat.com, jimb@redhat.com Subject: Re: [RFA/breakpoint] Fix errors from disabled watchpoints Message-ID: <20030102212230.GA23599@nevyn.them.org> Mail-Followup-To: Michael Snyder , gdb-patches@sources.redhat.com, jimb@redhat.com References: <20021228180954.GA17387@nevyn.them.org> <3E149C3E.5EE135BB@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3E149C3E.5EE135BB@redhat.com> User-Agent: Mutt/1.5.1i X-SW-Source: 2003-01/txt/msg00033.txt.bz2 On Thu, Jan 02, 2003 at 12:08:30PM -0800, Michael Snyder wrote: > Daniel Jacobowitz wrote: > > > > Right now, if you set and then disable a watchpoint, you'll still memory > > errors from it in two places. One is fatal, and comes from > > insert_breakpoints (); the other is just noisy, and comes from > > breakpoint_re_set_one (). Neither really serves any purpose. If a > > watchpoint is disabled, we don't need to check what its value is; we'll > > check when we insert it. > > > > It would be nice to do the equivalet of a bp_shlib_disabled for watchpoints > > on memory that isn't currently accessible but that's not really practical on > > any OS I know of, so the user still has to hand-disable and hand-enable the > > watchpoints. But at least they don't have to _delete_ the watchpoints now. > > > > Is this OK? No surprises in the testsuite on i386-linux. > > I'm not surprised that watchpoints were broken in this way, > but after looking at your changes, I am surprised that the > problem didn't show up in any other context. > > My only concern with your change is that you've changed > the code from explicitly listing the excluded states, to > assuming that they are all excluded except for one. The > problem that concerns me with that is, what if future states > are added? We were already being pretty inconsistent about which we checked; see how half the checks I deleted were inclusive and the other half were exclusive? If we start adding states I suspect we'll need BREAKPOINT_ENABLED (bp->state), or something along those lines. > > > > > -- > > Daniel Jacobowitz > > MontaVista Software Debian GNU/Linux Developer > > > > 2002-12-28 Daniel Jacobowitz > > > > * breakpoint.c (insert_breakpoints): Skip disabled breakpoints > > entirely. > > (breakpoint_re_set_one): Don't fetch the value for a disabled > > watchpoint. > > > > Index: breakpoint.c > > =================================================================== > > RCS file: /cvs/src/src/gdb/breakpoint.c,v > > retrieving revision 1.104 > > diff -u -p -r1.104 breakpoint.c > > --- breakpoint.c 17 Dec 2002 17:27:44 -0000 1.104 > > +++ breakpoint.c 28 Dec 2002 17:59:57 -0000 > > @@ -735,9 +735,11 @@ insert_breakpoints (void) > > > > ALL_BREAKPOINTS_SAFE (b, temp) > > { > > - if (b->enable_state == bp_permanent) > > - /* Permanent breakpoints cannot be inserted or removed. */ > > + /* Permanent breakpoints cannot be inserted or removed. Disabled > > + breakpoints should not be inserted. */ > > + if (b->enable_state != bp_enabled) > > continue; > > + > > if ((b->type == bp_watchpoint > > || b->type == bp_hardware_watchpoint > > || b->type == bp_read_watchpoint > > @@ -759,9 +761,6 @@ insert_breakpoints (void) > > && b->type != bp_catch_exec > > && b->type != bp_catch_throw > > && b->type != bp_catch_catch > > - && b->enable_state != bp_disabled > > - && b->enable_state != bp_shlib_disabled > > - && b->enable_state != bp_call_disabled > > && !b->inserted > > && !b->duplicate) > > { > > @@ -880,9 +879,6 @@ insert_breakpoints (void) > > return_val = val; /* remember failure */ > > } > > else if (ep_is_exception_catchpoint (b) > > - && b->enable_state != bp_disabled > > - && b->enable_state != bp_shlib_disabled > > - && b->enable_state != bp_call_disabled > > && !b->inserted > > && !b->duplicate) > > > > @@ -940,7 +936,6 @@ insert_breakpoints (void) > > else if ((b->type == bp_hardware_watchpoint || > > b->type == bp_read_watchpoint || > > b->type == bp_access_watchpoint) > > - && b->enable_state == bp_enabled > > && b->disposition != disp_del_at_next_stop > > && !b->inserted > > && !b->duplicate) > > @@ -1059,7 +1054,6 @@ insert_breakpoints (void) > > else if ((b->type == bp_catch_fork > > || b->type == bp_catch_vfork > > || b->type == bp_catch_exec) > > - && b->enable_state == bp_enabled > > && !b->inserted > > && !b->duplicate) > > { > > @@ -7049,7 +7043,7 @@ breakpoint_re_set_one (PTR bint) > > value_free (b->val); > > b->val = evaluate_expression (b->exp); > > release_value (b->val); > > - if (VALUE_LAZY (b->val)) > > + if (VALUE_LAZY (b->val) && b->enable_state == bp_enabled) > > value_fetch_lazy (b->val); > > > > if (b->cond_string != NULL) > -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer