From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23251 invoked by alias); 2 Jan 2003 20:08:48 -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 23244 invoked from network); 2 Jan 2003 20:08:45 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by 209.249.29.67 with SMTP; 2 Jan 2003 20:08:45 -0000 Received: from int-mx2.corp.redhat.com (nat-pool-rdu-dmz.redhat.com [172.16.52.200]) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id h02JfIB22935 for ; Thu, 2 Jan 2003 14:41:18 -0500 Received: from potter.sfbay.redhat.com (potter.sfbay.redhat.com [172.16.27.15]) by int-mx2.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h02K8Vn28190; Thu, 2 Jan 2003 15:08:31 -0500 Received: from redhat.com (reddwarf.sfbay.redhat.com [172.16.24.50]) by potter.sfbay.redhat.com (8.11.6/8.11.6) with ESMTP id h02K8Un19160; Thu, 2 Jan 2003 12:08:30 -0800 Message-ID: <3E149C3E.5EE135BB@redhat.com> Date: Thu, 02 Jan 2003 20:08:00 -0000 From: Michael Snyder Organization: Red Hat, Inc. X-Accept-Language: en MIME-Version: 1.0 To: Daniel Jacobowitz CC: gdb-patches@sources.redhat.com, jimb@redhat.com Subject: Re: [RFA/breakpoint] Fix errors from disabled watchpoints References: <20021228180954.GA17387@nevyn.them.org> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2003-01/txt/msg00020.txt.bz2 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? > > -- > 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)