From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28824 invoked by alias); 29 May 2011 13:01:35 -0000 Received: (qmail 28796 invoked by uid 22791); 29 May 2011 13:01:32 -0000 X-SWARE-Spam-Status: No, hits=0.3 required=5.0 tests=AWL,BAYES_50,MIME_QP_LONG_LINE,RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from mailrelay002.isp.belgacom.be (HELO mailrelay002.isp.belgacom.be) (195.238.6.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 29 May 2011 13:01:13 +0000 X-Belgacom-Dynamic: yes X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AhQFAJdB4k1tgHac/2dsb2JhbAA3HooXm0FkeIhqB7B5jCsOhhAEn2s Received: from 156.118-128-109.adsl-dyn.isp.belgacom.be (HELO soleil) ([109.128.118.156]) by relay.skynet.be with SMTP; 29 May 2011 15:01:11 +0200 Message-ID: From: "Philippe Waroquiers" To: Cc: References: Subject: Re: ping: Re: PATCH : allow to set length of hw watchpoints (e.g. for Valgrind gdbserver) Date: Sun, 29 May 2011 13:01:00 -0000 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_019A_01CC1E11.3F350B60" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-05/txt/msg00664.txt.bz2 This is a multi-part message in MIME format. ------=_NextPart_000_019A_01CC1E11.3F350B60 Content-Type: text/plain; format=flowed; charset="iso-8859-1"; reply-type=original Content-Transfer-Encoding: 7bit Content-length: 2826 On Fri, 27 May 2011 11:24:31 +0800 Yao Qi wrote > If I understand you correctly, this commands are only useful in > gdbserver+valgrind. If gdb is running in normal remote target, using > `set remote hardware-watchpoint-length-limit' will make GDB work > incorrectly. Interesting remark. Waiting for the legal papers to be signed, it made me work a little bit more, and allowed me to find a bug in the current gdbserver hw watchpoint handling on x86 : linux-x86-low.c:511: A problem internal to GDBserver has been detected. Assertion `DR_FIRSTADDR <= regnum && regnum < DR_LASTADDR' failed. I am attaching a new version of the patch which fixes this problem + attaching the test program. The patch is supposed to be useful also for 'non Valgrind' gdbserver. I think the current behaviour is inconsistent (whatever the gdbserver) and the patch is making it more consistent. Currently: the user can configure the nr of remote hw watchpoints (default unlimited). GDB tries to place the watchpoints, and if it fails, the user can then decide what to do (e.g. delete some watchpoints, switch to software watchpoints, etc). For a too long watchpoint, GDB silently switches to software watchpoints, which is I find surprising : it will then run (hundreds of times?) slower without warning. With the patch, a too long watchpoint will be handled similarly to too many watchpoints. The user can then decide what to do for too long watchpoints e.g. switch to software watchpoints, disable or delete some watchpoints, ... 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 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. If you enable remote hw points debugging, you will see that after having deleted all watchpoints, some debug registers are still set. The patch fixes this by adding a rollback. Question: is there somewhere a 'how to test gdbserver for dummies' ? I read the gdb internal doc, and could only find a reference to setting GDBSERVER=... variable when doing make check, but that does not run the gdbserver tests (I believe). I took a look at dejagnu documentation, but that is not for a dummy like me :). In the meantime, I have only done limited manual tests. Philippe ------=_NextPart_000_019A_01CC1E11.3F350B60 Content-Type: application/octet-stream; name="s.c" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="s.c" Content-length: 597 char s1[1]; char sep1[5000];=0A= char s2[2]; char sep2[5000];=0A= char s3[3]; char sep3[5000];=0A= char s4[4]; char sep4[5000];=0A= char s5[5]; char sep5[5000];=0A= char s6[6]; char sep6[5000];=0A= char s7[7]; char sep7[5000];=0A= char s8[8]; char sep8[5000];=0A= =0A= char s16[16]; char sep16[5000];=0A= =0A= char s32[32]; char sep32[5000];=0A= =0A= char s64[64]; char sep64[5000];=0A= =0A= char s128[128]; char sep128[5000];=0A= =0A= char s1000[1000]; char spe1[5000];=0A= main()=0A= {=0A= int i;=0A= char * p =3D s1000;=0A= for (i =3D 0; i < 1000; i++)=0A= p[i] =3D 1;=0A= }=0A= ------=_NextPart_000_019A_01CC1E11.3F350B60 Content-Type: text/plain; format=flowed; name="set-lenght-limit-5.txt"; reply-type=original Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="set-lenght-limit-5.txt" Content-length: 7939 Index: gdb/NEWS =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/src/src/gdb/NEWS,v retrieving revision 1.441 diff -c -p -r1.441 NEWS *** gdb/NEWS 13 May 2011 22:36:06 -0000 1.441 --- gdb/NEWS 29 May 2011 12:39:19 -0000 *************** *** 3,8 **** --- 3,18 ---- =20=20 *** Changes since GDB 7.3 =20=20 + * GDB has two new commands: "set remote hardware-watchpoint-length-limit" + and "show remote hardware-watchpoint-length-limit". These allows to + set or show the maximum length limit (in bytes) of a remote + target hardware watchpoint. +=20 + This allows e.g. to use "unlimited" hardware watchpoints with the + gdbserver integrated in Valgrind version >=3D 3.7.0. Such Valgrind + watchpoints are slower than real hardware watchpoints but are + significantly faster than gdb software watchpoints. +=20 * libthread-db-search-path now supports two special values: $sdir and $pd= ir. $sdir specifies the default system locations of shared libraries. $pdir specifies the directory where the libpthread used by the applicat= ion Index: gdb/remote.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D 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 29 May 2011 12:39:20 -0000 *************** remote_remove_watchpoint (CORE_ADDR addr *** 7756,7764 **** --- 7756,7778 ---- =20=20 =20=20 int remote_hw_watchpoint_limit =3D -1; + int remote_hw_watchpoint_length_limit =3D -1; int remote_hw_breakpoint_limit =3D -1; =20=20 static int + remote_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) + { + if (remote_hw_watchpoint_length_limit =3D=3D 0) + return 0; + else if (remote_hw_watchpoint_length_limit < 0) + return 1; + else if (len <=3D remote_hw_watchpoint_length_limit) + return 1; + else + return 0; + } +=20 + static int remote_check_watch_resources (int type, int cnt, int ot) { if (type =3D=3D bp_hardware_breakpoint) *************** Specify the serial device it is connecte *** 10326,10331 **** --- 10340,10347 ---- remote_ops.to_can_use_hw_breakpoint =3D remote_check_watch_resources; remote_ops.to_insert_hw_breakpoint =3D remote_insert_hw_breakpoint; remote_ops.to_remove_hw_breakpoint =3D remote_remove_hw_breakpoint; + remote_ops.to_region_ok_for_hw_watchpoint + =3D remote_region_ok_for_hw_watchpoint; remote_ops.to_insert_watchpoint =3D remote_insert_watchpoint; remote_ops.to_remove_watchpoint =3D remote_remove_watchpoint; remote_ops.to_kill =3D remote_kill; *************** Specify a negative limit for unlimited." *** 10735,10740 **** --- 10751,10765 ---- 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 (in bytes) of a target hardware watchpoint."), _("\ + Show the maximum length (in bytes) of a target hardware watchpoint."), _(= "\ + Specify a negative limit for unlimited."), + NULL, NULL, /* FIXME: i18n: The maximum + length (in bytes) 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 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v retrieving revision 1.840 diff -c -p -r1.840 gdb.texinfo *** gdb/doc/gdb.texinfo 17 May 2011 13:29:38 -0000 1.840 --- gdb/doc/gdb.texinfo 29 May 2011 12:39:24 -0000 *************** responses. *** 16578,16583 **** --- 16578,16595 ---- Restrict @value{GDBN} to using @var{limit} remote hardware breakpoint or watchpoints. A limit of -1, the default, is treated as unlimited. =20=20 + @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} bytes for the maximum length of + a remote hardware watchpoint. A limit of -1, the default, is treated + as unlimited. +=20 + @item show remote hardware-watchpoint-length-limit + Show the current limit (in bytes) of the maximum length of + a remote hardware watchpoint. +=20 @item set remote exec-file @var{filename} @itemx show remote exec-file @anchor{set remote exec-file} Index: gdb/gdbserver/ChangeLog =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/src/src/gdb/gdbserver/ChangeLog,v retrieving revision 1.483 diff -c -p -r1.483 ChangeLog *** gdb/gdbserver/ChangeLog 26 May 2011 15:49:25 -0000 1.483 --- gdb/gdbserver/ChangeLog 29 May 2011 12:39:24 -0000 *************** *** 1,3 **** --- 1,10 ---- + 2011-05-29 Philippe Waroquiers +=20 + * i386-low.c (i386_low_insert_watchpoint): rollback a partially + inserted hw watchpoint. In i386-nat.c, this rollback is ensured + by breakpoint.c In i386-low.c (copied from i386-nat.c), the + rollback must be done by gdbserver. +=20 2011-05-26 Yao Qi =20=20 * Makefile.in (thread-db.o): Track dependence to Index: gdb/gdbserver/i386-low.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/src/src/gdb/gdbserver/i386-low.c,v retrieving revision 1.6 diff -c -p -r1.6 i386-low.c *** gdb/gdbserver/i386-low.c 27 Feb 2011 21:33:10 -0000 1.6 --- gdb/gdbserver/i386-low.c 29 May 2011 12:39:24 -0000 *************** Invalid value %d of operation in i386_ha *** 412,418 **** in which case we just increment the reference counts of occupied debug registers). If we break out of the loop too early, we could cause those addresses watched by ! other watchpoints to be disabled when breakpoint.c reacts to our failure to insert this watchpoint and tries to remove it. */ if (status) --- 412,418 ---- in which case we just increment the reference counts of occupied debug registers). If we break out of the loop too early, we could cause those addresses watched by ! other watchpoints to be disabled when our caller reacts to our failure to insert this watchpoint and tries to remove it. */ if (status) *************** i386_low_insert_watchpoint (struct i386_ *** 468,473 **** --- 468,481 ---- { retval =3D i386_handle_nonaligned_watchpoint (state, WP_INSERT, addr, len, type); + if (retval) + { + if (debug_hw_points) + i386_show_dr (state, "rolling back FAILED insert_watchpoin= t", + addr, len, type); + i386_handle_nonaligned_watchpoint (state, WP_REMOVE, + addr, len, type); + } } else { ------=_NextPart_000_019A_01CC1E11.3F350B60--