[RFA] Fix hw watchpoint count in update_watchpoint

[RFA] Fix hw watchpoint count in update_watchpoint

[Thiago Jung Bauermann]

[-- Attachment #1: Type: Text/Plain, Size: 752 bytes --]

Hi,

update_watchpoint counts the number of existing hardware watchpoints to decide 
whether there is room for another one. The problem is that it miscounts the 
number of existing hardware watchpoints. The reason is that the given 
watchpoint can already be a hardware watchpoint and thus it will be counted as 
such by hw_watchpoint_used_count, which may make update_watchpoint decide that 
it should downgrade the hardware watchpoint to a software one.

Ok to commit?
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


20090-12-30  Jan Kratochvil  <jan.kratochvil@redhat.com>
	     Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* breakpoint.c (update_watchpoint): Change b->type to
	bp_watchpoint before calling hw_watchpoint_used_count.

[-- Attachment #2: watchpoint-count-fix.diff --]
[-- Type: text/x-patch, Size: 675 bytes --]

Index: gdb.git/gdb/breakpoint.c
===================================================================
--- gdb.git.orig/gdb/breakpoint.c	2009-12-28 14:00:45.000000000 -0200
+++ gdb.git/gdb/breakpoint.c	2009-12-30 12:39:12.000000000 -0200
@@ -1152,6 +1152,10 @@ update_watchpoint (struct breakpoint *b,
 	  {
 	    int i, mem_cnt, other_type_used;
 
+	    /* Mark as software watchpoint to ensure that this watchpoint
+	       will not be counted in the hw_watchpoint_used_count call
+	       below.  */
+	    b->type = bp_watchpoint;
 	    i = hw_watchpoint_used_count (bp_hardware_watchpoint,
 					  &other_type_used);
 	    mem_cnt = can_use_hardware_watchpoint (val_chain);

Re: [RFA] Fix hw watchpoint count in update_watchpoint

[Thiago Jung Bauermann]

On Wed 30 Dec 2009 14:46:48 Thiago Jung Bauermann wrote:
> Ok to commit?

I forgot to mention that there were no regressions on i686-linux.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFA] Fix hw watchpoint count in update_watchpoint

[Joel Brobecker]

> 20090-12-30  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 	     Thiago Jung Bauermann  <bauerman@br.ibm.com>
> 
> 	* breakpoint.c (update_watchpoint): Change b->type to
> 	bp_watchpoint before calling hw_watchpoint_used_count.

Look good, please go ahead and commit.  If I could just expand a little
on your comment, if you don't mind:

> +	    /* Mark as software watchpoint to ensure that this watchpoint
> +	       will not be counted in the hw_watchpoint_used_count call
> +	       below.  */

            /* We need to determine how many resources are already used
               for all other hardware watchpoints to see if we still have 
               enough resources to also fit this watchpoint in as well.
               To avoid the hw_watchpoint_used_count call below from counting
               this watchpoint, make sure that it is marked as a software
               watchpoint.  */

This is only a suggestion, so if you don't like it, please commit your
version, and I'll see if I can suggest an update separately.

Thanks for submitting the patch! (and welcome back, it's been a while)
-- 
Joel

Re: [RFA] Fix hw watchpoint count in update_watchpoint

[Thiago Jung Bauermann]

[-- Attachment #1: Type: Text/Plain, Size: 1203 bytes --]

On Wed 30 Dec 2009 17:06:26 Joel Brobecker wrote:
> > +	    /* Mark as software watchpoint to ensure that this watchpoint
> > +	       will not be counted in the hw_watchpoint_used_count call
> > +	       below.  */
> 
>             /* We need to determine how many resources are already used
>                for all other hardware watchpoints to see if we still have
>                enough resources to also fit this watchpoint in as well.
>                To avoid the hw_watchpoint_used_count call below from
>  counting this watchpoint, make sure that it is marked as a software
>  watchpoint.  */
> 
> This is only a suggestion, so if you don't like it, please commit your
> version, and I'll see if I can suggest an update separately.

Your version is much better. I used it.
 
> Thanks for submitting the patch! (and welcome back, it's been a while)
 
Thanks! It's nice to be back. :-)

I committed the attached.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


20090-12-30  Jan Kratochvil  <jan.kratochvil@redhat.com>
	     Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* breakpoint.c (update_watchpoint): Change b->type to
	bp_watchpoint before calling hw_watchpoint_used_count.

[-- Attachment #2: watchpoint-count-fix.diff --]
[-- Type: text/x-patch, Size: 874 bytes --]

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2009-12-30 15:41:58.000000000 -0200
+++ src/gdb/breakpoint.c	2009-12-30 18:04:57.000000000 -0200
@@ -1152,6 +1152,13 @@ update_watchpoint (struct breakpoint *b,
 	  {
 	    int i, mem_cnt, other_type_used;
 
+	    /* We need to determine how many resources are already used
+	       for all other hardware watchpoints to see if we still have
+	       enough resources to also fit this watchpoint in as well.
+	       To avoid the hw_watchpoint_used_count call below from counting
+	       this watchpoint, make sure that it is marked as a software
+	       watchpoint.  */
+	    b->type = bp_watchpoint;
 	    i = hw_watchpoint_used_count (bp_hardware_watchpoint,
 					  &other_type_used);
 	    mem_cnt = can_use_hardware_watchpoint (val_chain);