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

[Philippe Waroquiers]

> I think nothing else changed in the patch.

I looked at the new patch and re-tested on f12/x86 and debian5/amd64, using 7.3.
Behaviour looks ok to me regarding the handling of debug registers.
(note I tested with the patch allowing to change the remote hw watchpoint length,
which I believe could be committed soon : FSF papers ok, waiting for a user now).

During the testing, I however found something else slightly strange.
With reference to the previous s.c test program, watching a string length 1000
is ok at the start (handled as a sw breakpoint), but this watchpoint cannot be disabled
then re-enabled:
   (gdb) watch s1000
   Hardware watchpoint 1: s1000
   (gdb) start     <<<<<<<<<<<<<<<<<<<<<<<<<< this runs slowly as s1000 is sw-watched
   Temporary breakpoint 2 at 0x400480: file s.c, line 22.
   Starting program: /home/philippe/gdb/s 
   Error in re-setting breakpoint 1: Expression cannot be implemented with read/access watchpoint.
   Error in re-setting breakpoint 1: Expression cannot be implemented with read/access watchpoint.
   Error in re-setting breakpoint 1: Expression cannot be implemented with read/access watchpoint.

   Temporary breakpoint 2, main () at s.c:22
   22    char * p = s1000;
   (gdb) dis 1
   (gdb) ena 1
   Cannot enable watchpoint 1: Expression cannot be implemented with read/access watchpoint.
   (gdb) 
At this point, if the watchpoint is deleted then re-created, then the watchpoint is again 'sw-accepted'.
Note that this looks to be a regression in 7.3.50.20110722-cvs, as I do not see the same problem on 7.2.
This regression is not linked with the DR patch (occurs both with the patched/non patched 7.3.50).

So, in summary:
  * the patch for the i386 debug register fix looks ok.
The following strange behaviours/bugs have still to be fixed or looked at:
  * handling of duplicate locations across disabled breakpoints
     (resulting in wrongly duplicated z packets and/or missing active debug registers in native)
  * watch s1000 then run then disable then enable impossible

 There was also a 'nice to have' which could be looked at:
  * ensure that the insertion of watchpoint is done using the order of breakpoints
     (so as to not have a new watchpoint causing an error/rejection on a previously accepted
      watchpoint).

Thanks for all the work

Philippe