x86 watchpoints bug (Re: ping: Re: PATCH : allow to set length of hw watchpoints (e.g. for Valgrind gdbserver))

[Pedro Alves <pedro@codesourcery.com>]

On Sunday 29 May 2011 14:01:11, Philippe Waroquiers wrote:
> For what concerns the bug: the problem is that the code in i386-low.c
> can partially place a watchpoint, but to the contrary of i386-nat.c,
> such a partial watchpoint will not be rolled back by breakpoint.c

Not sure I understand what is different between GDB and GDBserver
here.  A watchpoint, from breakpoint.c's perpective can be composed
of several low-level watchpoints.  E.g., if the expression the user
wants to watch requires trapping accesses to two disjoint memory
regions for changes, each of those memory regions will correspond
to one low-level hardware watchpoint.  In GDBserver's or i386-nat.c's
perpective, there will be two watchpoints.  If the second fails to
insert, then breakpoint.c in GDB rolls back the first.  This applies
to GDBserver as well.

> The patch fixes this (as this bug is much easier to trigger with long
> breakpoints which are accepted by gdb, but have to be rolled back
> by gdbserver). 
> To reproduce the bug, compile the attached s.c, and do the following:
>       gdbserver :1234 ./s
> gdb ./s
> tar rem :1234
> set breakpoint always-inserted on
> watch s1
> watch s2
> watch s4
> watch s3
> del
> y
> break s.c:24
> c
> p p = &s3
> c
>         linux-x86-low.c:511: A problem internal to GDBserver has been detected.
>         Assertion `DR_FIRSTADDR <= regnum && regnum < DR_LASTADDR' failed.

First things first.  This assertion is actually bogus ( and I'm to blame
for it :-) ).  Patch below.  We get here with regnum == 3, which is quite
valid.  This means gdbserver is asserting whenever a watchpoint on DR3
triggers.  Vis:

(gdb) watch s1
(gdb) watch s2
(gdb) watch s3

(gdb) b 24
(gdb) c
...

on gdbserver side we have:

stopped_data_addr:
        CONTROL (DR7): 51150155          STATUS (DR6): 00000000
        DR0: addr=0x603768, ref.count=1  DR1: addr=0x60376a, ref.count=1
        DR2: addr=0x609a08, ref.count=1  DR3: addr=0x60d8e8, ref.count=1
                                                   ^^^^^^^^

so:

(gdb) p p = 0x60d8e8
(gdb) c

... puff!

../../../src/gdb/gdbserver/linux-x86-low.c:511: A problem internal to GDBserver has been detected.
Assertion `DR_FIRSTADDR <= regnum && regnum < DR_LASTADDR' failed.

Program exited with code 01.
(gdb) 

Pedro Alves

2011-05-31  Pedro Alves  <pedro@codesourcery.com>

	gdb/gdbserver/
	* linux-x86-low.c (i386_dr_low_get_addr): Fix off by one in
	assertion.
	* win32-i386-low.c (i386_dr_low_get_addr): Ditto.

---
 gdb/gdbserver/linux-x86-low.c  |    2 +-
 gdb/gdbserver/win32-i386-low.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: src/gdb/gdbserver/linux-x86-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-x86-low.c	2011-04-28 17:17:30.000000000 +0100
+++ src/gdb/gdbserver/linux-x86-low.c	2011-05-31 19:55:00.924959503 +0100
@@ -508,7 +508,7 @@ i386_dr_low_get_addr (int regnum)
   ptid_t ptid = ptid_of (lwp);
 
   /* DR6 and DR7 are retrieved with some other way.  */
-  gdb_assert (DR_FIRSTADDR <= regnum && regnum < DR_LASTADDR);
+  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
 
   return x86_linux_dr_get (ptid, regnum);
 }
Index: src/gdb/gdbserver/win32-i386-low.c
===================================================================
--- src.orig/gdb/gdbserver/win32-i386-low.c	2011-01-13 15:07:54.000000000 +0000
+++ src/gdb/gdbserver/win32-i386-low.c	2011-05-31 19:56:14.414959478 +0100
@@ -61,7 +61,7 @@ i386_dr_low_set_addr (const struct i386_
 CORE_ADDR
 i386_dr_low_get_addr (int regnum)
 {
-  gdb_assert (DR_FIRSTADDR <= regnum && regnum < DR_LASTADDR);
+  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
 
   return debug_reg_state.dr_mirror[regnum];
 }