This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Somewhat sanitize watchpoints with conditions on local expressions
On Thursday 04 March 2010 05:59:26, Joel Brobecker wrote:
> > (gdb) cond *foo > 1000
> > Bad breakpoint argument: '*foo > 1000'
>
> GDB does not have bugs, GDB is perfect. Only the operator has bugs.
> You forgot the breakpoint number in this example.
>
> (sorry, just couldn't resist)
Heh! Thanks, it's official, I'm dumb. :-)
What I was trying to show does happen, but of course, silly
mistakes happen when trying to show pretty examples. :-)
If will not-error on non-MMU targets much more easily.
>
>
> > IMO, GBB could be smarted, and check if it makes sense
> > to evaluate the condition in the current frame before
> > actualy trying, and stop if it doesn't, with an short blurb:
> >
> > (gdb) c
> > Continuing.
> > warning: Watchpoint condition can't tested in the current scope
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> That, or maybe just disable the watchpoint and keep going?
> It's unclear
> what the user intent is, but maybe he really meant for the watchpoint
> to be local to the scope where the condition applies? Maybe your approach
> is safer, though - perhaps it's better to over hit a watchpoint rather
> than miss some hits...
Yeah, one can't really tell, so IMO better stop (which leaves the option
of simply continuing), than not stopping and leave the user wondering
what's changing the memory, and why isn't GDB helping. It's a one line
change to change this behaviour, so, if usage in the field indicates the
other way around would be better, it's easy to change.
>
> I agree with your second case.
>
> > 2010-03-04 Pedro Alves <pedro@codesourcery.com>
> >
> > * breakpoint.c (condition_command): Handle watchpoint conditions.
> > (is_watchpoint): New.
> >
> > (update_watchpoint): Don't reparse the watchpoint's condition
> > unless necessary.
> > (WP_IGNORE): New.
> > (watchpoint_check): Use it.
> > (bpstat_check_watchpoint): Handle it.
> > (bpstat_check_breakpoint_conditions):
> >
> > 2010-03-04 Pedro Alves <pedro@codesourcery.com>
> >
> > * gdb.base/watch-cond.c, gdb.base/watch-cond.exp: New.
>
> I only skimmed through the patch, just noticed a couple of nits.
>
> > + /* For local watchpoint expressions, which particular
> > + instance of a local is being watched matters, so we
> > + keep track of the frame to evaluate the expression
> > + in. To evaluate the condition however, it doesn't
> > + really matter which instantiation of the function
> > + where the condition makes sense triggers the
> > + watchpoint. This allows an expression like "watch
> > + global if q > 10" set in `func', catch writes to
> > + global on all threads that call `func', or catch
> > + writes on all recursive calls of `func' by a single
> > + thread. We simple always evaluate the condition in
> ^^^^^^
> simply?
> > + warning (_("Watchpoint condition can't tested "
> ^^^^^^^^^^^^
> cannot be tested
>
> > + "in the current scope"));
>
> Note: I personally would prefer if we kept contractions away from
> our error messages (hence the suggestion to use "cannot"), but I'm OK
> if others think it's fine.
I'm a convert. I fully agree. Both fixed locally. Thanks!
--
Pedro Alves