Re: Somewhat sanitize watchpoints with conditions on local expressions

[Joel Brobecker <brobecker@adacore.com>]

>  (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)


> 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...

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.

-- 
Joel