[PATCH] Watchpoints + conditionals problem

[PATCH] Watchpoints + conditionals problem

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]

Hi folks,

There is an unexpected behavior when we use software watchpoints (also
hardware in some cases) attached to conditional expressions, such as:
"watch x if x == <number>", considering "x" a simple "int" type
variable.

When that watchpoint goes out of scope, it's marked for disposal at the
next stop as obviously there's no way to evaluate the expression "x"
anymore. But GDB is still trying to evaluate the conditional part of the
watchpoint, even though "x" is out of scope already. Thus, GDB is
failing to find correct frame information and ends up in an internal
error.

I've tracked down this issue and it seems we need to check for the
disposition field of the watchpoint whenever we check for the return
value of a conditional expression.

Attached is a simple patch of what i have in mind to fix this issue. The
testcase can be a simple "Hello world"-like binary with a few variables
to watch.

Any other ideas?

Regards,
Luis

[-- Attachment #2: watch_condition_fix.diff --]
[-- Type: text/x-patch, Size: 555 bytes --]

Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2007-11-28 09:47:26.000000000 -0800
+++ gdb/breakpoint.c	2007-11-28 09:49:47.000000000 -0800
@@ -2969,7 +2969,7 @@
 	if (b->type == bp_watchpoint_scope)
 	  b->related_breakpoint->watchpoint_triggered = watch_triggered_yes;
 
-	if (bl->cond)
+	if (bl->cond && bl->owner->disposition != disp_del_at_next_stop)
 	  {
 	    /* Need to select the frame, with all that implies
 	       so that the conditions will have the right context.  */

Re: [PATCH] Watchpoints + conditionals problem

[Vladimir Prus]

Luis Machado wrote:

> Hi folks,
> 
> There is an unexpected behavior when we use software watchpoints (also
> hardware in some cases) attached to conditional expressions, such as:
> "watch x if x == <number>", considering "x" a simple "int" type
> variable.
> 
> When that watchpoint goes out of scope, it's marked for disposal at the
> next stop as obviously there's no way to evaluate the expression "x"
> anymore. But GDB is still trying to evaluate the conditional part of the
> watchpoint, even though "x" is out of scope already. Thus, GDB is
> failing to find correct frame information and ends up in an internal
> error.

I've run into same problem yesterday.

> I've tracked down this issue and it seems we need to check for the
> disposition field of the watchpoint whenever we check for the return
> value of a conditional expression.
> 
> Attached is a simple patch of what i have in mind to fix this issue. The
> testcase can be a simple "Hello world"-like binary with a few variables
> to watch.
> 
> Any other ideas?

I guess ideally we need to:

1. Don't check condition at all if bs->stop is already 0.
2. Immediately delete watchpoint, don't bother with del_at_next_stop

But for now, your approach is probably fine -- can anybody approve it?

- Volodya

Re: [PATCH] Watchpoints + conditionals problem

[Luis Machado]

Thanks Vladimir,

Either way is fine with me. The simple fix or the alternate one.

Anybody has any comments?

Regards,
Luis

On Thu, 2007-11-29 at 11:53 +0300, Vladimir Prus wrote:
> Luis Machado wrote:
> 
> > Hi folks,
> > 
> > There is an unexpected behavior when we use software watchpoints (also
> > hardware in some cases) attached to conditional expressions, such as:
> > "watch x if x == <number>", considering "x" a simple "int" type
> > variable.
> > 
> > When that watchpoint goes out of scope, it's marked for disposal at the
> > next stop as obviously there's no way to evaluate the expression "x"
> > anymore. But GDB is still trying to evaluate the conditional part of the
> > watchpoint, even though "x" is out of scope already. Thus, GDB is
> > failing to find correct frame information and ends up in an internal
> > error.
> 
> I've run into same problem yesterday.
> 
> > I've tracked down this issue and it seems we need to check for the
> > disposition field of the watchpoint whenever we check for the return
> > value of a conditional expression.
> > 
> > Attached is a simple patch of what i have in mind to fix this issue. The
> > testcase can be a simple "Hello world"-like binary with a few variables
> > to watch.
> > 
> > Any other ideas?
> 
> I guess ideally we need to:
> 
> 1. Don't check condition at all if bs->stop is already 0.
> 2. Immediately delete watchpoint, don't bother with del_at_next_stop
> 
> But for now, your approach is probably fine -- can anybody approve it?
> 
> - Volodya
> 

Re: [PATCH] Watchpoints + conditionals problem

[Daniel Jacobowitz]

On Wed, Nov 28, 2007 at 03:59:15PM -0200, Luis Machado wrote:
> Index: gdb/breakpoint.c
> ===================================================================
> --- gdb.orig/breakpoint.c	2007-11-28 09:47:26.000000000 -0800
> +++ gdb/breakpoint.c	2007-11-28 09:49:47.000000000 -0800
> @@ -2969,7 +2969,7 @@
>  	if (b->type == bp_watchpoint_scope)
>  	  b->related_breakpoint->watchpoint_triggered = watch_triggered_yes;
>  
> -	if (bl->cond)
> +	if (bl->cond && bl->owner->disposition != disp_del_at_next_stop)
>  	  {
>  	    /* Need to select the frame, with all that implies
>  	       so that the conditions will have the right context.  */

Seems OK to me.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Watchpoints + conditionals problem

[Luis Machado]

Thanks Daniel.

On Sun, 2007-12-16 at 16:45 -0500, Daniel Jacobowitz wrote:
> On Wed, Nov 28, 2007 at 03:59:15PM -0200, Luis Machado wrote:
> > Index: gdb/breakpoint.c
> > ===================================================================
> > --- gdb.orig/breakpoint.c	2007-11-28 09:47:26.000000000 -0800
> > +++ gdb/breakpoint.c	2007-11-28 09:49:47.000000000 -0800
> > @@ -2969,7 +2969,7 @@
> >  	if (b->type == bp_watchpoint_scope)
> >  	  b->related_breakpoint->watchpoint_triggered = watch_triggered_yes;
> >  
> > -	if (bl->cond)
> > +	if (bl->cond && bl->owner->disposition != disp_del_at_next_stop)
> >  	  {
> >  	    /* Need to select the frame, with all that implies
> >  	       so that the conditions will have the right context.  */
> 
> Seems OK to me.
> 

-- 
Luis Machado
Software Engineer 
IBM Linux Technology Center
LoP Toolchain/Debuggers' team
Phone: +55 19-2132-2218
T/L: 839-2218
e-mail: luisgpm@linux.vnet.ibm.com

Re: [PATCH] Watchpoints + conditionals problem

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 119 bytes --]

> Seems OK to me.

I've checked this in now.

Regards,
-- 
Luis Machado
Software Engineer 
IBM Linux Technology Center

[-- Attachment #2: watch_condition_fix.diff --]
[-- Type: text/x-patch, Size: 713 bytes --]

2007-12-17  Luis Machado  <luisgpm@br.ibm.com>

	* breakpoint.c (bpstat_stop_status): Check an additional 
	condition before evaluating an expression value.

Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2007-12-17 03:18:27.000000000 -0800
+++ gdb/breakpoint.c	2007-12-17 03:18:50.000000000 -0800
@@ -2823,7 +2823,7 @@
 	if (b->type == bp_watchpoint_scope)
 	  b->related_breakpoint->watchpoint_triggered = watch_triggered_yes;
 
-	if (bl->cond)
+	if (bl->cond && bl->owner->disposition != disp_del_at_next_stop)
 	  {
 	    /* Need to select the frame, with all that implies
 	       so that the conditions will have the right context.  */