This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[RFC] checking the Z-packet suppport on gdbserver
- From: Emi SUZUKI <emi-suzuki at tjsys dot co dot jp>
- To: gdb-patches at sourceware dot org
- Date: Thu, 13 Sep 2007 21:08:22 +0900
- Subject: [RFC] checking the Z-packet suppport on gdbserver
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
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)