This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] gdbserver: Add support for Z0/Z1 packets
Doug Evans wrote:
On Tue, Jun 16, 2009 at 6:31 PM, Aleksandar Ristovski<aristovski@qnx.com> wrote:
Aleksandar Ristovski wrote:
Hello,
This adds support for Z0 and Z1 (and z0 and z1) packets.
I apologize, the patch should be this one. Change log corrected as well.
--
Aleksandar Ristovski
QNX Software Systems
ChangeLog:
* target.h (insert_breakpoint, remove_breakpoint): New target functions.
* server.c (process_serial_event): 'Z' and 'z' packet - add support
for [Zz]0 and [Zz]1 - software and hardware breakpoints.
* linux-low.c (linux_target_ops): Add new fields.
* spu-low.c (spu_target_ops): Likewise.
* win32-low.c (win32_target_ops): Likewise.
Hi.
Minor nits (fwiw):
Inserting the insert_breakpoint/remove_breakpoint fields *between* the
stopped_by_watchpoint/stopped_data_address fields is odd.
Can they go before insert_watchpoint?
I think the code in server.c that watches for 0/1/2/3/4 should use a switch.
The code previously caught invalid values (e.g. type == '5'), but you
accidentally removed that (IIUC).
The code in server.c does this:
+ if (the_target->insert_watchpoint == NULL)
+ {
+ /* No watchpoint support or not a watchpoint command;
+ unrecognized either way. */
+ own_buf[0] = '\0';
+ res = 1;
+ }
and later does this:
+ if (res == 0)
+ write_ok (own_buf);
+ else if (res == 1)
+ /* Unsupported. */
+ own_buf[0] = '\0';
+ else
+ write_enn (own_buf);
How about just setting res = 1 in the first code snippet above?
[This appears for both 'z' and 'Z' packets.]
Ok, I made changes accordingly. Note that since the code in
'Z' and 'z' differs only in whether "insert_" or "remove_"
version of function is called, I merged the two with the
distinction being made at the calling point.
--
Aleksandar Ristovski
QNX Software Systems
ChangeLog:
* target.h (insert_breakpoint, remove_breakpoint): New
target functions.
* server.c (process_serial_event): 'Z' and 'z' packet - add
support
for [Zz]0 and [Zz]1 - software and hardware breakpoints.
* linux-low.c (linux_target_ops): Add new fields.
* spu-low.c (spu_target_ops): Likewise.
* win32-low.c (win32_target_ops): Likewise.
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/target.h,v
retrieving revision 1.36
diff -u -p -r1.36 target.h
--- target.h 1 Apr 2009 22:50:24 -0000 1.36
+++ target.h 19 Jun 2009 13:47:44 -0000
@@ -213,6 +213,14 @@ struct target_ops
int (*read_auxv) (CORE_ADDR offset, unsigned char *myaddr,
unsigned int len);
+ /* Insert and remove breakpoint. Argument TYPE can be:
+ '0' = software-breakpoint
+ '1' = hardware-breakpoint
+ Returns 0 if successful, 1 otherwise. */
+
+ int (*insert_breakpoint) (char type, CORE_ADDR addr);
+ int (*remove_breakpoint) (char type, CORE_ADDR addr);
+
/* Insert and remove a hardware watchpoint.
Returns 0 on success, -1 on failure and 1 on unsupported.
The type is coded as follows:
Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.97
diff -u -p -r1.97 server.c
--- server.c 24 May 2009 21:06:53 -0000 1.97
+++ server.c 19 Jun 2009 13:47:44 -0000
@@ -2366,66 +2370,64 @@ process_serial_event (void)
signal = 0;
myresume (own_buf, 1, signal);
break;
- case 'Z':
+ case 'Z': /* insert_ ... */
+ /* Fallthrough. */
+ case 'z': /* remove_ ... */
{
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];
+ int res;
+ const int insert_ = ch == 'Z';
- 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;
+ /* Type: '0' - software-breakpoint
+ '1' - hardware-breakpoint
+ '2' - write watchpoint
+ '3' - read watchpoint
+ '4' - access watchpoint */
- require_running (own_buf);
- res = (*the_target->insert_watchpoint) (type, addr, len);
- if (res == 0)
- write_ok (own_buf);
- else if (res == 1)
- /* Unsupported. */
- own_buf[0] = '\0';
+ switch (type)
+ {
+ case '0':
+ /* Fallthrough. */
+ case '1':
+ if ((insert_ ? the_target->insert_breakpoint
+ : the_target->remove_breakpoint) == NULL)
+ res = 1; /* Not supported. */
else
- write_enn (own_buf);
+ res = insert_ ? (*the_target->insert_breakpoint) (type, addr)
+ : (*the_target->remove_breakpoint) (type, addr);
+ break;
+ case '2':
+ /* Fallthrough. */
+ case '3':
+ /* Fallthrough. */
+ case '4':
+ if ((insert_ ? the_target->insert_watchpoint
+ : the_target->remove_watchpoint) == NULL)
+ res = 1; /* Not supported. */
+ else
+ {
+ require_running (own_buf);
+ res = insert_ ? (*the_target->insert_watchpoint) (type, addr,
+ len)
+ : (*the_target->remove_watchpoint) (type, addr,
+ len);
+ }
+ break;
+ default:
+ res = -1; /* Unrecognized type. */
}
- 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->remove_watchpoint == NULL
- || (type < '2' || type > '4'))
- {
- /* No watchpoint support or not a watchpoint command;
- unrecognized either way. */
- own_buf[0] = '\0';
- }
+ if (res == 0)
+ write_ok (own_buf);
+ else if (res == 1)
+ /* Unsupported. */
+ own_buf[0] = '\0';
else
- {
- int res;
-
- require_running (own_buf);
- res = (*the_target->remove_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);
- }
+ write_enn (own_buf);
break;
}
case 'k':
Index: linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.105
diff -u -p -r1.105 linux-low.c
--- linux-low.c 24 May 2009 17:44:19 -0000 1.105
+++ linux-low.c 19 Jun 2009 13:47:45 -0000
@@ -3024,6 +3030,8 @@ static struct target_ops linux_target_op
linux_look_up_symbols,
linux_request_interrupt,
linux_read_auxv,
+ NULL, /* insert_breakpoint */
+ NULL, /* remove_breakpoint */
linux_insert_watchpoint,
linux_remove_watchpoint,
linux_stopped_by_watchpoint,
Index: spu-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/spu-low.c,v
retrieving revision 1.24
diff -u -p -r1.24 spu-low.c
--- spu-low.c 3 Apr 2009 14:38:39 -0000 1.24
+++ spu-low.c 19 Jun 2009 13:47:45 -0000
@@ -612,6 +612,8 @@ static struct target_ops spu_target_ops
spu_look_up_symbols,
spu_request_interrupt,
NULL,
+ NULL, /* insert_breakpoint */
+ NULL, /* remove_breakpoint */
NULL,
NULL,
NULL,
Index: win32-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v
retrieving revision 1.35
diff -u -p -r1.35 win32-low.c
--- win32-low.c 1 Apr 2009 22:50:24 -0000 1.35
+++ win32-low.c 19 Jun 2009 13:47:45 -0000
@@ -1697,6 +1697,8 @@ static struct target_ops win32_target_op
NULL,
win32_request_interrupt,
NULL,
+ NULL, /* insert_breakpoint */
+ NULL, /* remove_breakpoint */
NULL,
NULL,
NULL,