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

["Philippe Waroquiers" <philippe.waroquiers@skynet.be>]

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

(copy of the below text also put in an attachment, as my mailer seems
to eat e.g. the tab characters of the ChangeLog).

A gdbserver has been integrated in Valgrind svn version.
(see https://bugs.kde.org/show_bug.cgi?id=214909 for details).

This gdbserver can provide 'unlimited' watchpoint length
using the Valgrind memcheck machinery. Such watchpoints
are slower than hardware watchpoints, but are faster
that gdb software watchpoints.
However, gdb remote.c hardcodes the length of a remote
watchpoint to 4 bytes (which looks like a bug).
The attached patch fixes this by adding a new variable
and the related commands to set/show it.
This follows the principles of similar variables
configuring e.g. the nr of remote watchpoints.

With the last svn Valgrind, and this patch, the following
watchpoint will be handled at a reasonable (i.e. "valgrind")
speed:
In the program being debugged:
  char s[1000];

in gdb:
  awatch s

as the hardware watchpoint will be accepted by the valgrind
gdbserver, and implemented using Valgrind validity bits
mechanism.


The change is small, so no paper signing might be needed.
But if deemed necessary, I can sign the needed papers.


Here is the change log entry for this change.

2011-05-14  Philippe Waroquiers <philippe.waroquiers@skynet.be>
 * remote.c (remote_region_ok_for_hw_watchpoint): New function.
 (remote_hw_watchpoint_length_limit): New variable.
 (_initialize_remote) add set,show cmds for this new variable.
 gdb.texinfo: document these new commands.

Index: gdb/remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.446
diff -c -p -r1.446 remote.c
*** gdb/remote.c 12 May 2011 12:09:16 -0000 1.446
--- gdb/remote.c 14 May 2011 14:40:08 -0000
*************** remote_remove_watchpoint (CORE_ADDR addr
*** 7756,7764 ****
--- 7756,7778 ----
  
  
  int remote_hw_watchpoint_limit = -1;
+ int remote_hw_watchpoint_length_limit = -1;
  int remote_hw_breakpoint_limit = -1;
  
  static int
+ remote_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
+ {
+   if (remote_hw_watchpoint_length_limit == 0)
+     return 0;
+   else if (remote_hw_watchpoint_length_limit < 0)
+     return 1;
+   else if (len <= remote_hw_watchpoint_length_limit)
+     return 1;
+   else
+     return 0;
+ }
+ 
+ static int
  remote_check_watch_resources (int type, int cnt, int ot)
  {
    if (type == bp_hardware_breakpoint)
*************** Specify the serial device it is connecte
*** 10326,10331 ****
--- 10340,10347 ----
    remote_ops.to_can_use_hw_breakpoint = remote_check_watch_resources;
    remote_ops.to_insert_hw_breakpoint = remote_insert_hw_breakpoint;
    remote_ops.to_remove_hw_breakpoint = remote_remove_hw_breakpoint;
+   remote_ops.to_region_ok_for_hw_watchpoint
+      = remote_region_ok_for_hw_watchpoint;
    remote_ops.to_insert_watchpoint = remote_insert_watchpoint;
    remote_ops.to_remove_watchpoint = remote_remove_watchpoint;
    remote_ops.to_kill = remote_kill;
*************** Specify a negative limit for unlimited."
*** 10735,10740 ****
--- 10751,10763 ----
          number of target hardware
          watchpoints is %s.  */
         &remote_set_cmdlist, &remote_show_cmdlist);
+   add_setshow_zinteger_cmd ("hardware-watchpoint-length-limit", no_class,
+        &remote_hw_watchpoint_length_limit, _("\
+ Set the maximum length of a target hardware watchpoint."), _("\
+ Show the maximum length of a target hardware watchpoint."), _("\
+ Specify a negative limit for unlimited."),
+        NULL, NULL, /* FIXME: i18n: The maximum length of a target hardware watchpoint is %s.  */
+        &remote_set_cmdlist, &remote_show_cmdlist);
    add_setshow_zinteger_cmd ("hardware-breakpoint-limit", no_class,
         &remote_hw_breakpoint_limit, _("\
  Set the maximum number of target hardware breakpoints."), _("\
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.838
diff -c -p -r1.838 gdb.texinfo
*** gdb/doc/gdb.texinfo 13 May 2011 22:36:07 -0000 1.838
--- gdb/doc/gdb.texinfo 14 May 2011 14:40:33 -0000
*************** responses.
*** 16578,16583 ****
--- 16578,16591 ----
  Restrict @value{GDBN} to using @var{limit} remote hardware breakpoint or
  watchpoints.  A limit of -1, the default, is treated as unlimited.
  
+ @cindex limit hardware watchpoints length
+ @cindex remote target, limit watchpoints length
+ @anchor{set remote hardware-watchpoint-length-limit}
+ @item set remote hardware-watchpoint-length-limit @var{limit}
+ Restrict @value{GDBN} to using @var{limit} for the maximum length of
+ remote hardware watchpoints.  A limit of -1, the default, is treated
+ as unlimited.
+ 
  @item set remote exec-file @var{filename}
  @itemx show remote exec-file
  @anchor{set remote exec-file}

[-- Attachment #2: set-length-limit-desc.txt --]
[-- Type: text/plain, Size: 4869 bytes --]


A gdbserver has been integrated in Valgrind svn version.
(see https://bugs.kde.org/show_bug.cgi?id=214909 for details).

This gdbserver can provide 'unlimited' watchpoint length
using the Valgrind memcheck machinery. Such watchpoints
are slower than hardware watchpoints, but are faster
that gdb software watchpoints.
However, gdb remote.c hardcodes the length of a remote
watchpoint to 4 bytes (which looks like a bug).
The attached patch fixes this by adding a new variable
and the related commands to set/show it.
This follows the principles of similar variables
configuring e.g. the nr of remote watchpoints.

With the last svn Valgrind, and this patch, the following
watchpoint will be handled at a reasonable (i.e. "valgrind")
speed:
In the program being debugged:
  char s[1000];

in gdb:
  awatch s

as the hardware watchpoint will be accepted by the valgrind
gdbserver, and implemented using Valgrind validity bits
mechanism.


The change is small, so no paper signing might be needed.
But if deemed necessary, I can sign the needed papers.


Here is the change log entry for this change.

2011-05-14  Philippe Waroquiers <philippe.waroquiers@skynet.be>
	* remote.c (remote_region_ok_for_hw_watchpoint): New function.
	(remote_hw_watchpoint_length_limit): New variable.
	(_initialize_remote) add set,show cmds for this new variable.
	gdb.texinfo: document these new commands.

Index: gdb/remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.446
diff -c -p -r1.446 remote.c
*** gdb/remote.c	12 May 2011 12:09:16 -0000	1.446
--- gdb/remote.c	14 May 2011 14:40:08 -0000
*************** remote_remove_watchpoint (CORE_ADDR addr
*** 7756,7764 ****
--- 7756,7778 ----
  
  
  int remote_hw_watchpoint_limit = -1;
+ int remote_hw_watchpoint_length_limit = -1;
  int remote_hw_breakpoint_limit = -1;
  
  static int
+ remote_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
+ {
+   if (remote_hw_watchpoint_length_limit == 0)
+     return 0;
+   else if (remote_hw_watchpoint_length_limit < 0)
+     return 1;
+   else if (len <= remote_hw_watchpoint_length_limit)
+     return 1;
+   else
+     return 0;
+ }
+ 
+ static int
  remote_check_watch_resources (int type, int cnt, int ot)
  {
    if (type == bp_hardware_breakpoint)
*************** Specify the serial device it is connecte
*** 10326,10331 ****
--- 10340,10347 ----
    remote_ops.to_can_use_hw_breakpoint = remote_check_watch_resources;
    remote_ops.to_insert_hw_breakpoint = remote_insert_hw_breakpoint;
    remote_ops.to_remove_hw_breakpoint = remote_remove_hw_breakpoint;
+   remote_ops.to_region_ok_for_hw_watchpoint
+      = remote_region_ok_for_hw_watchpoint;
    remote_ops.to_insert_watchpoint = remote_insert_watchpoint;
    remote_ops.to_remove_watchpoint = remote_remove_watchpoint;
    remote_ops.to_kill = remote_kill;
*************** Specify a negative limit for unlimited."
*** 10735,10740 ****
--- 10751,10763 ----
  					   number of target hardware
  					   watchpoints is %s.  */
  			    &remote_set_cmdlist, &remote_show_cmdlist);
+   add_setshow_zinteger_cmd ("hardware-watchpoint-length-limit", no_class,
+ 			    &remote_hw_watchpoint_length_limit, _("\
+ Set the maximum length of a target hardware watchpoint."), _("\
+ Show the maximum length of a target hardware watchpoint."), _("\
+ Specify a negative limit for unlimited."),
+ 			    NULL, NULL, /* FIXME: i18n: The maximum length of a target hardware watchpoint is %s.  */
+ 			    &remote_set_cmdlist, &remote_show_cmdlist);
    add_setshow_zinteger_cmd ("hardware-breakpoint-limit", no_class,
  			    &remote_hw_breakpoint_limit, _("\
  Set the maximum number of target hardware breakpoints."), _("\
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.838
diff -c -p -r1.838 gdb.texinfo
*** gdb/doc/gdb.texinfo	13 May 2011 22:36:07 -0000	1.838
--- gdb/doc/gdb.texinfo	14 May 2011 14:40:33 -0000
*************** responses.
*** 16578,16583 ****
--- 16578,16591 ----
  Restrict @value{GDBN} to using @var{limit} remote hardware breakpoint or
  watchpoints.  A limit of -1, the default, is treated as unlimited.
  
+ @cindex limit hardware watchpoints length
+ @cindex remote target, limit watchpoints length
+ @anchor{set remote hardware-watchpoint-length-limit}
+ @item set remote hardware-watchpoint-length-limit @var{limit}
+ Restrict @value{GDBN} to using @var{limit} for the maximum length of
+ remote hardware watchpoints.  A limit of -1, the default, is treated
+ as unlimited.
+ 
  @item set remote exec-file @var{filename}
  @itemx show remote exec-file
  @anchor{set remote exec-file}