[Bug breakpoints/27423] New: Watchpoint could be hw but is sw

[Bug breakpoints/27423] New: Watchpoint could be hw but is sw

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27423

            Bug ID: 27423
           Summary: Watchpoint could be hw but is sw
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: breakpoints
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

Consider the same test-case as in PR27422.

Now consider the following 4 scenarios.


I. b 20, watch address:
...
$ gdb -iex "set trace-commands on" -batch a.out -ex "b 20" -ex run -ex "watch
*(int *)0x602260" -ex cont -ex "info break"
+b 20
Breakpoint 1 at 0x4005c5: file test.c, line 20.
+run

Breakpoint 1, main () at test.c:20
20        printf ("%p\n", ap);
+watch *(int *)0x602260
Hardware watchpoint 2: *(int *)0x602260
+cont
0x602260

Hardware watchpoint 2: *(int *)0x602260

Old value = 1
New value = 0
main () at test.c:22
22        a = 0;
+info break
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x00000000004005c5 in main at test.c:20
        breakpoint already hit 1 time
2       hw watchpoint  keep y                      *(int *)0x602260
        breakpoint already hit 1 time
...


II. b 20, watch expression:
...
$ gdb -iex "set trace-commands on" -batch a.out -ex "b 20" -ex run -ex "watch
*(int *)0x602260 == 0" -ex cont -ex "info break"
+b 20
Breakpoint 1 at 0x4005c5: file test.c, line 20.
+run

Breakpoint 1, main () at test.c:20
20        printf ("%p\n", ap);
+watch *(int *)0x602260 == 0
Hardware watchpoint 2: *(int *)0x602260 == 0
+cont
0x602260

Hardware watchpoint 2: *(int *)0x602260 == 0

Old value = 0
New value = 1
main () at test.c:22
22        a = 0;
+info break
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x00000000004005c5 in main at test.c:20
        breakpoint already hit 1 time
2       hw watchpoint  keep y                      *(int *)0x602260 == 0
        breakpoint already hit 1 time
...


III. b 18, watch address:
...
$ gdb -iex "set trace-commands on" -batch a.out -ex "b 18" -ex run -ex "watch
*(int *)0x602260" -ex cont -ex "info break"       
+b 18
Breakpoint 1 at 0x4005ad: file test.c, line 18.
+run

Breakpoint 1, main () at test.c:18
18        int *ap = malloc (sizeof(int));
+watch *(int *)0x602260
Hardware watchpoint 2: *(int *)0x602260
+cont

Hardware watchpoint 2: *(int *)0x602260

Old value = <unreadable>
New value = 1
main () at test.c:20
20        printf ("%p\n", ap);
+info break
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x00000000004005ad in main at test.c:18
        breakpoint already hit 1 time
2       hw watchpoint  keep y                      *(int *)0x602260
        breakpoint already hit 1 time
...


IV. b 18, watch expression:
...
$ gdb -iex "set trace-commands on" -batch a.out -ex "b 18" -ex run -ex "watch
*(int *)0x602260 == 0" -ex cont -ex "info break"
+b 18
Breakpoint 1 at 0x4005ad: file test.c, line 18.
+run

Breakpoint 1, main () at test.c:18
18        int *ap = malloc (sizeof(int));
+watch *(int *)0x602260 == 0
Watchpoint 2: *(int *)0x602260 == 0
+cont

Watchpoint 2: *(int *)0x602260 == 0

Old value = <unreadable>
New value = 1
0x00007ffff7b0e9a9 in __brk (addr=addr@entry=0x623000) at
../sysdeps/unix/sysv/linux/x86_64/brk.c:31
31        __curbrk = newbrk = (void *) INLINE_SYSCALL (brk, 1, addr);
+info break
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x00000000004005ad in main at test.c:18
        breakpoint already hit 1 time
2       watchpoint     keep y                      *(int *)0x602260 == 0
        breakpoint already hit 1 time
...

We have a hw breakpoint in all cases, except case IV.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/27423] Watchpoint could be hw but is sw

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27423

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
The behaviour can be observed in can_use_hardware_watchpoint.

In all cases expect IV, we hit target_region_ok_for_hw_watchpoint, where true
is returned.

In case IV, we hit instead the ';' here:
...
      if (VALUE_LVAL (v) == lval_memory)
        {
          if (v != head && value_lazy (v))
            /* A lazy memory lvalue in the chain is one that GDB never          
               needed to fetch; we either just used its address (e.g.,          
               `a' in `a.b') or we never needed it at all (e.g., `a'            
               in `a,b').  This doesn't apply to HEAD; if that is               
               lazy then it was not readable, but watch it anyway.  */
            ;
          else
            {
              /* Ahh, memory we actually used!  Check if we can cover          
              ...

I can force choosing the else clause, and then observe a hw watchpoint instead.
 But it never triggers.  So, either more fixes are required, or this is the
wrong way to fix it and instead the value chain needs fixing.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/27423] Watchpoint could be hw but is sw

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27423

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
OK, so with this patch it works:
...
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index e9aba79e40f..2d5de0dbb9c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1932,8 +1932,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
             must watch it.  If the first value returned is
             still lazy, that means an error occurred reading it;
             watch it anyway in case it becomes readable.  */
-         if (VALUE_LVAL (v) == lval_memory
-             && (v == val_chain[0] || ! value_lazy (v)))
+         if (VALUE_LVAL (v) == lval_memory)
            {
              struct type *vtype = check_typedef (value_type (v));

@@ -11063,14 +11062,6 @@ can_use_hardware_watchpoint (const
std::vector<value_ref_ptr>
 &vals)

       if (VALUE_LVAL (v) == lval_memory)
        {
-         if (v != head && value_lazy (v))
-           /* A lazy memory lvalue in the chain is one that GDB never
-              needed to fetch; we either just used its address (e.g.,
-              `a' in `a.b') or we never needed it at all (e.g., `a'
-              in `a,b').  This doesn't apply to HEAD; if that is
-              lazy then it was not readable, but watch it anyway.  */
-           ;
-         else
            {
              /* Ahh, memory we actually used!  Check if we can cover
                 it with hardware watchpoints.  */
...
such that we have:
...
$ gdb -iex "set trace-commands on" -batch a.out -ex "b 18" -ex run -ex "watch
*(int *)0x602260 == 0" -ex cont -ex "info break"
+b 18
Breakpoint 1 at 0x4005ad: file test.c, line 18.
+run

Breakpoint 1, main () at test.c:18
18        int *ap = malloc (sizeof(int));
+watch *(int *)0x602260 == 0
Hardware watchpoint 2: *(int *)0x602260 == 0
+cont

Hardware watchpoint 2: *(int *)0x602260 == 0

Old value = <unreadable>
New value = 0
main () at test.c:20
20        printf ("%p\n", ap);
+info break
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x00000000004005ad in main at test.c:18
        breakpoint already hit 1 time
2       hw watchpoint  keep y                      *(int *)0x602260 == 0
        breakpoint already hit 1 time
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/27423] Watchpoint could be hw but is sw

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27423

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
WORKAROUND: Flip the watch expression to "0 == *(int *)0x602260", and you have
a hw watchpoint instead:
...
$ gdb -iex "set trace-commands on" -batch a.out -ex "b 18" -ex run -ex "watch 0
== *(int *)0x602260" -ex cont -ex "info break"     
+b 18
Breakpoint 1 at 0x4005ad: file test.c, line 18.
+run

Breakpoint 1, main () at test.c:18
18        int *ap = malloc (sizeof(int));
+watch 0 == *(int *)0x602260
Hardware watchpoint 2: 0 == *(int *)0x602260
+cont

Hardware watchpoint 2: 0 == *(int *)0x602260

Old value = <unreadable>
New value = 0
main () at test.c:20
20        printf ("%p\n", ap);
+info break
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x00000000004005ad in main at test.c:18
        breakpoint already hit 1 time
2       hw watchpoint  keep y                      0 == *(int *)0x602260
        breakpoint already hit 1 time
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/27423] Watchpoint could be hw but is sw

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=27423

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #2)
> OK, so with this patch it works:
> ...
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index e9aba79e40f..2d5de0dbb9c 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -1932,8 +1932,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
>              must watch it.  If the first value returned is
>              still lazy, that means an error occurred reading it;
>              watch it anyway in case it becomes readable.  */
> -         if (VALUE_LVAL (v) == lval_memory
> -             && (v == val_chain[0] || ! value_lazy (v)))
> +         if (VALUE_LVAL (v) == lval_memory)
>             {
>               struct type *vtype = check_typedef (value_type (v));
>  
> @@ -11063,14 +11062,6 @@ can_use_hardware_watchpoint (const
> std::vector<value_ref_ptr>
>  &vals)
>  
>        if (VALUE_LVAL (v) == lval_memory)
>         {
> -         if (v != head && value_lazy (v))
> -           /* A lazy memory lvalue in the chain is one that GDB never
> -              needed to fetch; we either just used its address (e.g.,
> -              `a' in `a.b') or we never needed it at all (e.g., `a'
> -              in `a,b').  This doesn't apply to HEAD; if that is
> -              lazy then it was not readable, but watch it anyway.  */
> -           ;
> -         else
>             {
>               /* Ahh, memory we actually used!  Check if we can cover
>                  it with hardware watchpoints.  */
> ...


Tested this patch. 287 FAILs in total, in gdb.base/watchpoint-unaligned.exp and
gdb.base/watchpoint-reuse-slot.exp.

-- 
You are receiving this mail because:
You are on the CC list for the bug.