This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA/breakpoint] Fix errors from disabled watchpoints
- From: Daniel Jacobowitz <drow at mvista dot com>
- To: Michael Snyder <msnyder at redhat dot com>
- Cc: gdb-patches at sources dot redhat dot com, jimb at redhat dot com
- Date: Thu, 2 Jan 2003 16:22:30 -0500
- Subject: Re: [RFA/breakpoint] Fix errors from disabled watchpoints
- References: <20021228180954.GA17387@nevyn.them.org> <3E149C3E.5EE135BB@redhat.com>
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 <drow@mvista.com>
> >
> > * 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