From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1705 invoked by alias); 13 Sep 2007 12:09:53 -0000 Received: (qmail 1694 invoked by uid 22791); 13 Sep 2007 12:09:51 -0000 X-Spam-Check-By: sourceware.org Received: from imx12.toshiba.co.jp (HELO imx12.toshiba.co.jp) (61.202.160.132) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 13 Sep 2007 12:09:42 +0000 Received: from arc11.toshiba.co.jp ([133.199.90.127]) by imx12.toshiba.co.jp with ESMTP id l8DC9dr0020249 for ; Thu, 13 Sep 2007 21:09:39 +0900 (JST) Received: (from root@localhost) by arc11.toshiba.co.jp id l8DC9cRO000448 for gdb-patches@sourceware.org; Thu, 13 Sep 2007 21:09:38 +0900 (JST) Received: from ovp11.toshiba.co.jp [133.199.90.148] by arc11.toshiba.co.jp with ESMTP id XAA00447; Thu, 13 Sep 2007 21:09:38 +0900 Received: from mx12.toshiba.co.jp (localhost [127.0.0.1]) by ovp11.toshiba.co.jp with ESMTP id l8DC9cMR002564 for ; Thu, 13 Sep 2007 21:09:38 +0900 (JST) Received: from mx.tjsys.co.jp by toshiba.co.jp id l8DC9cwW007994; Thu, 13 Sep 2007 21:09:38 +0900 (JST) Received: from voltage-out.tjsys.co.jp (voltage-out.tjsys.co.jp [157.79.3.51]) by mx.tjsys.co.jp (8.12.11/8.12.11) with ESMTP id l8DC9bM4029553 for ; Thu, 13 Sep 2007 21:09:37 +0900 (JST) Received: from is-com10 ([157.79.3.71]) by voltage-out.tjsys.co.jp (8.13.1/8.13.1) with SMTP id l8DC9WN3021346 for ; Thu, 13 Sep 2007 21:09:32 +0900 Received: from localhost ([157.79.51.49]) by ims.tjsys.co.jp (iPlanet Messaging Server 5.2 HotFix 2.10 (built Dec 26 2005)) with ESMTP id <0JOB00JWW33UDK@ims.tjsys.co.jp> for gdb-patches@sourceware.org; Thu, 13 Sep 2007 21:09:30 +0900 (JST) Date: Thu, 13 Sep 2007 12:09:00 -0000 From: Emi SUZUKI Subject: [RFC] checking the Z-packet suppport on gdbserver To: gdb-patches@sourceware.org Message-id: <20070913.210822.19763360.emi-suzuki@tjsys.co.jp> MIME-version: 1.0 X-Mailer: Mew version 5.2.51 on Emacs 22.1 / Mule 5.0 (SAKAKI) Content-type: Multipart/Mixed; boundary="--Next_Part(Thu_Sep_13_21_08_22_2007_127)--" Content-transfer-encoding: 7bit X-WAuditID: 0709132109300000014486 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: 2007-09/txt/msg00171.txt.bz2 ----Next_Part(Thu_Sep_13_21_08_22_2007_127)-- Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Content-length: 4687 Hello members, Now I've noticed that GDB always tries to use hardware watchpoints while remote debugging, even the target doesn't have the hardware watchpoint support. ----- $ ./gdb hello_target GNU gdb 6.6.50.20070904-cvs Copyright (C) 2007 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type "show copying" to see the conditions. There is absolutely no warranty for GDB. Type "show warranty" for details. This GDB was configured as "--host=i686-pc-linux-gnu --target=powerpc64-linux"... (gdb) set solib-absolute-prefix /target/sysroot (gdb) target remote remote_server:2015 Remote debugging using remote_server:2015 0x0ffd6bf4 in _start () from /target/sysroot/lib/ld.so.1 (gdb) set debug remote 1 (gdb) watch glob Sending packet: $m10010b30,4#84...Ack Packet received: 00000003 Hardware watchpoint 1: glob (gdb) c Continuing. Sending packet: $Z0,ffcf654,4#4a...Ack Packet received: Packet Z0 (software-breakpoint) is NOT supported Sending packet: $mffcf654,4#01...Ack Packet received: 893a0000 Sending packet: $Xffcf654,0:#22...Ack Packet received: OK binary downloading suppported by target Sending packet: $Xffcf654,4:}]\202\020\b#9a...Ack Packet received: OK Sending packet: $m10010b30,4#84...Ack Packet received: 00000003 Sending packet: $Z2,10010b30,4#cf...Ack Packet received: Packet Z2 (write-watchpoint) is NOT supported warning: Could not remove hardware watchpoint 1. Warning: Could not insert hardware watchpoint 1. Could not insert hardware breakpoints: You may have requested too many hardware breakpoints/watchpoints. (gdb) ------ GDB decides which type of watchpoints should be set when giving the command like "watch foo". And hardware watchpoints can be used when gdbserver has the Z-packet support. However, as the session log above has shown, GDB does not check the Z-packet support on gdbserver when deciding the type of the watchpoint but when actually setting the watchpoint to the target. It has a simple workaround for this problem: delete the watchpoint and setting "remote hardware-watchpoint-limit" to 0, and set it again. But I thought that the timing when checking the Z-packet support should be fxied and made a fix for this as attached. The concept is: - gdbserver: Allows "Z#" packets (not "Z#,addr,length"). Responds "OK" if hardware breakpoints/watchpoints are supported or responds "". - gdb: Sends a "Z#" packet for checking if hardware breakpoints/watchpoints can be handled on the target. And two things which I feel indecisive for this fix are there: - I defined a new function pointer "watchpoint_support" in target_ops for gdbserver, because I thought we cannot decide the common address value for all the architectures supported by gdbserver at which we should not set a hardware watchpoint. If we can, we can use the existing format of Z-packet for this purpose. - The newly defined packet format might cause problems like exceptions or setting a hardware watchpoint at the unexpected location when working with older versions of gdbserver, because in which the Z-packet format is not checked at all. The codes in CVS head are below: case 'Z': { char *lenptr; char *dataptr; CORE_ADDR addr = strtoul (&own_buf[3], &lenptr, 16); int len = strtol (lenptr + 1, &dataptr, 16); char type = own_buf[1]; if (the_target->insert_watchpoint == NULL || (type < '2' || type > '4')) { /* No watchpoint support or not a watchpoint command; unrecognized either way. */ own_buf[0] = '\0'; } else { int res; res = (*the_target->insert_watchpoint) (type, addr, len); if (res == 0) write_ok (own_buf); else if (res == 1) /* Unsupported. */ own_buf[0] = '\0'; else write_enn (own_buf); } break; } Would anyone give me any comments how it should be treated as a whole? Defines another packet for it? Applies as proposed and notes "it might not work with older versions of gdbserver" ? By the way, now I have *finally* got the copyright assignment for FSF. But the problem I have addressed for the first time here has already fixed by Luis Machado and many active members in the list. I would like to thank all of them, and apologize for not giving enough comments for it. Best regards, -- Emi SUZUKI / emi-suzuki at tjsys.co.jp ----Next_Part(Thu_Sep_13_21_08_22_2007_127)-- Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="zpacket-support.diff" Content-length: 8937 Index: gdb/gdbserver/server.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/server.c,v retrieving revision 1.56 diff -u -r1.56 server.c --- gdb/gdbserver/server.c 23 Aug 2007 18:08:48 -0000 1.56 +++ gdb/gdbserver/server.c 13 Sep 2007 10:51:07 -0000 @@ -1097,21 +1097,30 @@ break; case 'Z': { - char *lenptr; - char *dataptr; - CORE_ADDR addr = strtoul (&own_buf[3], &lenptr, 16); - int len = strtol (lenptr + 1, &dataptr, 16); char type = own_buf[1]; - if (the_target->insert_watchpoint == NULL || (type < '2' || type > '4')) { /* No watchpoint support or not a watchpoint command; unrecognized either way. */ own_buf[0] = '\0'; + break; + } + + if (own_buf[2] == '\0') + { + if ((*the_target->watchpoint_support) (type)) + write_ok (own_buf); + else + /* Unsupported. */ + own_buf[0] = '\0'; } else { + char *lenptr; + char *dataptr; + CORE_ADDR addr = strtoul (&own_buf[3], &lenptr, 16); + int len = strtol (lenptr + 1, &dataptr, 16); int res; res = (*the_target->insert_watchpoint) (type, addr, len); Index: gdb/gdbserver/linux-low.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v retrieving revision 1.61 diff -u -r1.61 linux-low.c --- gdb/gdbserver/linux-low.c 4 Sep 2007 21:30:23 -0000 1.61 +++ gdb/gdbserver/linux-low.c 13 Sep 2007 10:50:59 -0000 @@ -1635,6 +1635,16 @@ } static int +linux_watchpoint_support (char type) +{ + if (the_low_target.watchpoint_support != NULL) + return the_low_target.watchpoint_support (type); + else + /* Unsupported. */ + return 0; +} + +static int linux_stopped_by_watchpoint (void) { if (the_low_target.stopped_by_watchpoint != NULL) @@ -1721,6 +1731,7 @@ linux_read_auxv, linux_insert_watchpoint, linux_remove_watchpoint, + linux_watchpoint_support, linux_stopped_by_watchpoint, linux_stopped_data_address, #if defined(__UCLIBC__) && defined(HAS_NOMMU) Index: gdb/gdbserver/linux-low.h =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/linux-low.h,v retrieving revision 1.18 diff -u -r1.18 linux-low.h --- gdb/gdbserver/linux-low.h 23 Aug 2007 18:08:48 -0000 1.18 +++ gdb/gdbserver/linux-low.h 13 Sep 2007 10:50:59 -0000 @@ -63,6 +63,7 @@ /* Watchpoint related functions. See target.h for comments. */ int (*insert_watchpoint) (char type, CORE_ADDR addr, int len); int (*remove_watchpoint) (char type, CORE_ADDR addr, int len); + int (*watchpoint_support) (char type); int (*stopped_by_watchpoint) (void); CORE_ADDR (*stopped_data_address) (void); Index: gdb/gdbserver/target.h =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/target.h,v retrieving revision 1.25 diff -u -r1.25 target.h --- gdb/gdbserver/target.h 23 Aug 2007 18:08:48 -0000 1.25 +++ gdb/gdbserver/target.h 13 Sep 2007 10:51:11 -0000 @@ -155,6 +155,11 @@ int (*insert_watchpoint) (char type, CORE_ADDR addr, int len); int (*remove_watchpoint) (char type, CORE_ADDR addr, int len); + + /* Check if hardware watchpoints are supported. + Returns 0 on unsupported, else on supported. + The type is coded as mentioned above. */ + int (*watchpoint_support) (char type); /* Returns 1 if target was stopped due to a watchpoint hit, 0 otherwise. */ Index: gdb/gdbserver/win32-low.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v retrieving revision 1.15 diff -u -r1.15 win32-low.c --- gdb/gdbserver/win32-low.c 3 Sep 2007 22:17:27 -0000 1.15 +++ gdb/gdbserver/win32-low.c 13 Sep 2007 10:51:16 -0000 @@ -1522,6 +1522,7 @@ NULL, NULL, NULL, + NULL, win32_arch_string }; Index: gdb/gdbserver/linux-x86-64-low.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/linux-x86-64-low.c,v retrieving revision 1.16 diff -u -r1.16 linux-x86-64-low.c --- gdb/gdbserver/linux-x86-64-low.c 23 Aug 2007 18:08:48 -0000 1.16 +++ gdb/gdbserver/linux-x86-64-low.c 13 Sep 2007 10:51:00 -0000 @@ -174,6 +174,7 @@ NULL, NULL, NULL, + NULL, 0, "i386:x86-64", }; Index: gdb/gdbserver/linux-crisv32-low.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/linux-crisv32-low.c,v retrieving revision 1.5 diff -u -r1.5 linux-crisv32-low.c --- gdb/gdbserver/linux-crisv32-low.c 23 Aug 2007 18:08:48 -0000 1.5 +++ gdb/gdbserver/linux-crisv32-low.c 13 Sep 2007 10:50:51 -0000 @@ -307,6 +307,21 @@ } static int +cris_watchpoint_support (char type) +{ + /* Breakpoint/watchpoint types is described in the comment for + cris_insert_watchpoint. */ + + if (type < '2' || type > '4') + { + /* Unsupported. */ + return 0; + } + /* Supported. */ + return 1; +} + +static int cris_stopped_by_watchpoint (void) { unsigned long exs; @@ -373,6 +388,7 @@ cris_breakpoint_at, cris_insert_watchpoint, cris_remove_watchpoint, + cris_watchpoint_support, cris_stopped_by_watchpoint, cris_stopped_data_address, }; Index: gdb/gdbserver/linux-i386-low.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/linux-i386-low.c,v retrieving revision 1.13 diff -u -r1.13 linux-i386-low.c --- gdb/gdbserver/linux-i386-low.c 23 Aug 2007 18:08:48 -0000 1.13 +++ gdb/gdbserver/linux-i386-low.c 13 Sep 2007 10:50:51 -0000 @@ -200,6 +200,7 @@ NULL, NULL, NULL, + NULL, 0, "i386" }; Index: gdb/gdbserver/linux-ppc64-low.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/linux-ppc64-low.c,v retrieving revision 1.8 diff -u -r1.8 linux-ppc64-low.c --- gdb/gdbserver/linux-ppc64-low.c 23 Aug 2007 18:08:48 -0000 1.8 +++ gdb/gdbserver/linux-ppc64-low.c 13 Sep 2007 10:51:00 -0000 @@ -130,5 +130,6 @@ NULL, NULL, NULL, + NULL, 1 }; Index: gdb/gdbserver/spu-low.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/spu-low.c,v retrieving revision 1.12 diff -u -r1.12 spu-low.c --- gdb/gdbserver/spu-low.c 23 Aug 2007 18:08:48 -0000 1.12 +++ gdb/gdbserver/spu-low.c 13 Sep 2007 10:51:09 -0000 @@ -596,6 +596,7 @@ NULL, NULL, NULL, + NULL, spu_arch_string, spu_proc_xfer_spu, }; Index: gdb/gdbserver/linux-cris-low.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/linux-cris-low.c,v retrieving revision 1.5 diff -u -r1.5 linux-cris-low.c --- gdb/gdbserver/linux-cris-low.c 23 Aug 2007 18:08:48 -0000 1.5 +++ gdb/gdbserver/linux-cris-low.c 13 Sep 2007 10:50:49 -0000 @@ -116,8 +116,4 @@ cris_reinsert_addr, 0, cris_breakpoint_at, - 0, - 0, - 0, - 0, }; Index: gdb/remote.c =================================================================== RCS file: /cvs/src/src/gdb/remote.c,v retrieving revision 1.266 diff -u -r1.266 remote.c --- gdb/remote.c 23 Aug 2007 18:08:36 -0000 1.266 +++ gdb/remote.c 13 Sep 2007 10:50:49 -0000 @@ -5374,6 +5374,53 @@ _("remote_remove_watchpoint: reached end of function")); } +static int +hardware_resouce_to_Z_packet (int type) +{ + switch (type) + { + case bp_hardware_breakpoint: + return Z_PACKET_HARDWARE_BP; + break; + case bp_hardware_watchpoint: + return Z_PACKET_WRITE_WP; + break; + case bp_read_watchpoint: + return Z_PACKET_READ_WP; + break; + case bp_access_watchpoint: + return Z_PACKET_ACCESS_WP; + break; + default: + internal_error (__FILE__, __LINE__, + _("hardware_resouce_to_Z_packet: bad watchpoint type %d"), type); + } +} + +static int +remote_check_Zpacket_support (int type) +{ + struct remote_state *rs = get_remote_state (); + enum Z_packet_type packet = hardware_resouce_to_Z_packet (type); + + if (remote_protocol_packets[PACKET_Z0 + packet].support == PACKET_DISABLE) + return 0; + + sprintf (rs->buf, "Z%x", packet); + putpkt (rs->buf); + getpkt (&rs->buf, &rs->buf_size, 0); + + switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z0 + packet])) + { + case PACKET_ERROR: + case PACKET_UNKNOWN: + return 0; + case PACKET_OK: + return 1; + } + internal_error (__FILE__, __LINE__, + _("remote_check_Zpacket_supported: reached end of function")); +} int remote_hw_watchpoint_limit = -1; int remote_hw_breakpoint_limit = -1; @@ -5381,6 +5428,9 @@ static int remote_check_watch_resources (int type, int cnt, int ot) { + if (! remote_check_Zpacket_support (type)) + return 0; + if (type == bp_hardware_breakpoint) { if (remote_hw_breakpoint_limit == 0) ----Next_Part(Thu_Sep_13_21_08_22_2007_127)----