Re: [gdbserver/patch] Z packet support

[Orjan Friberg <orjan.friberg@axis.com>]

Daniel Jacobowitz wrote:
> 
> I'm fine with the approach.  The implementation needs a bit of work,
> though.

There's a new ChangeLog entry (date change only) and patch at the end.  A couple 
of comments to what you wrote first through:

>>+	  addr = (*the_target->stopped_data_address) ();
>>+
>>+	  *buf++ = tohex ((addr >> 28) & 0xf);
>>+	  *buf++ = tohex ((addr >> 24) & 0xf);
>>+	  *buf++ = tohex ((addr >> 20) & 0xf);
>>+	  *buf++ = tohex ((addr >> 16) & 0xf);
>>+
>>+	  *buf++ = tohex ((addr >> 12) & 0xf);
>>+	  *buf++ = tohex ((addr >> 8) & 0xf);
>>+	  *buf++ = tohex ((addr >> 4) & 0xf);
>>+	  *buf++ = tohex (addr & 0xf);
>>+
>>+	  *buf++ = ';';
>>+	}
> 
> 
> This is broken on 64-bit targets.  This will require a certain amount
> of shuffling to figure out the proper size of an address.

Living comfortably in a 32-bit world, I may have missed what you mean by "a 
certain amount of shuffling".  I imagined a simple sizeof (addr) would do.

>   - Some of the z/Z commands are breakpoints, not watchpoints.  Please
>     only invoke watchpoint methods for watchpoint commands.

I assume you mean that I should exclude memory breakpoint and hardware 
breakpoint.  Anyway, that's what I did.


2005-01-12  Orjan Friberg  <orjanf@axis.com>

	* target.h (struct target_ops): Add insert_watchpoint,
	remove_watchpoint, stopped_by_watchpoint, stopped_data_address function
	pointers for hardware watchpoint support.
	* linux-low.h (struct linux_target_ops): Ditto.
	* linux-low.c (linux_insert_watchpoint, linux_remove_watchpoint)
	(linux_stopped_by_watchpoint, linux_stopped_data_address): New.  Add
	to linux_target_ops.
	* remote-utils.c (prepare_resume_reply): Add watchpoint information to
	reply packet.
	* server.c (main): Recognize 'Z' and 'z' packets.


Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/target.h,v
retrieving revision 1.11
diff -u -p -r1.11 target.h
--- target.h    5 Mar 2004 03:43:19 -0000       1.11
+++ target.h    12 Jan 2005 13:23:55 -0000
@@ -133,6 +133,27 @@ struct target_ops
       Read LEN bytes at OFFSET into a buffer at MYADDR.  */

    int (*read_auxv) (CORE_ADDR offset, char *myaddr, unsigned int len);
+
+  /* Insert and remove a hardware watchpoint.
+     Returns 0 on success, -1 on failure and 1 on unsupported.
+     The type is coded as follows:
+       2 = write watchpoint
+       3 = read watchpoint
+       4 = access watchpoint
+  */
+
+  int (*insert_watchpoint) (char type, CORE_ADDR addr, int len);
+  int (*remove_watchpoint) (char type, CORE_ADDR addr, int len);
+
+  /* Returns 1 if target was stopped due to a watchpoint hit, 0 otherwise.  */
+
+  int (*stopped_by_watchpoint) (void);
+
+  /* Returns the address associated with the watchpoint that hit, if any;
+     returns 0 otherwise.  */
+
+  CORE_ADDR (*stopped_data_address) (void);
+
  };

  extern struct target_ops *the_target;
Index: linux-low.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.h,v
retrieving revision 1.7
diff -u -p -r1.7 linux-low.h
--- linux-low.h 31 Jan 2004 22:19:32 -0000      1.7
+++ linux-low.h 12 Jan 2005 13:23:55 -0000
@@ -57,6 +57,13 @@ struct linux_target_ops

    int decr_pc_after_break;
    int (*breakpoint_at) (CORE_ADDR pc);
+
+  /* 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 (*stopped_by_watchpoint) (void);
+  CORE_ADDR (*stopped_data_address) (void);
+
  };

  extern struct linux_target_ops the_low_target;
Index: linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.33
diff -u -p -r1.33 linux-low.c
--- linux-low.c 16 Oct 2004 17:42:00 -0000      1.33
+++ linux-low.c 12 Jan 2005 13:23:55 -0000
@@ -1466,7 +1466,47 @@ linux_read_auxv (CORE_ADDR offset, char
    return n;
  }

-

+/* These watchpoint related wrapper functions simply pass on the function call
+   if the target has registered a corresponding function.  */
+
+static int
+linux_insert_watchpoint (char type, CORE_ADDR addr, int len)
+{
+  if (the_low_target.insert_watchpoint != NULL)
+    return the_low_target.insert_watchpoint (type, addr, len);
+  else
+    /* Unsupported (see target.h).  */
+    return 1;
+}
+
+static int
+linux_remove_watchpoint (char type, CORE_ADDR addr, int len)
+{
+  if (the_low_target.remove_watchpoint != NULL)
+    return the_low_target.remove_watchpoint (type, addr, len);
+  else
+    /* Unsupported (see target.h).  */
+    return 1;
+}
+
+static int
+linux_stopped_by_watchpoint (void)
+{
+  if (the_low_target.stopped_by_watchpoint != NULL)
+    return the_low_target.stopped_by_watchpoint ();
+  else
+    return 0;
+}
+
+static CORE_ADDR
+linux_stopped_data_address (void)
+{
+  if (the_low_target.stopped_data_address != NULL)
+    return the_low_target.stopped_data_address ();
+  else
+    return 0;
+}
+
  static struct target_ops linux_target_ops = {
    linux_create_inferior,
    linux_attach,
@@ -1482,6 +1522,10 @@ static struct target_ops linux_target_op
    linux_look_up_symbols,
    linux_send_signal,
    linux_read_auxv,
+  linux_insert_watchpoint,
+  linux_remove_watchpoint,
+  linux_stopped_by_watchpoint,
+  linux_stopped_data_address,
  };

  static void
Index: remote-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/remote-utils.c,v
retrieving revision 1.22
diff -u -p -r1.22 remote-utils.c
--- remote-utils.c      16 Oct 2004 17:42:00 -0000      1.22
+++ remote-utils.c      12 Jan 2005 13:23:56 -0000
@@ -639,6 +639,26 @@ prepare_resume_reply (char *buf, char st
    if (status == 'T')
      {
        const char **regp = gdbserver_expedite_regs;
+
+      if (the_target->stopped_by_watchpoint != NULL
+         && (*the_target->stopped_by_watchpoint) ())
+       {
+         CORE_ADDR addr;
+         int i;
+
+         strncpy (buf, "watch:", 6);
+         buf += 6;
+
+         addr = (*the_target->stopped_data_address) ();
+
+         /* Convert each byte of the address into two hexadecimal chars.  */
+         for (i = sizeof (addr) * 2; i > 0; i--)
+           {
+             *buf++ = tohex ((addr >> (i - 1) * 4) & 0xf);
+           }
+         *buf++ = ';';
+       }
+
        while (*regp)
         {
           buf = outreg (find_regno (*regp), buf);
Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.22
diff -u -p -r1.22 server.c
--- server.c    5 Mar 2004 03:44:27 -0000       1.22
+++ server.c    12 Jan 2005 13:23:56 -0000
@@ -508,6 +508,66 @@ main (int argc, char *argv[])
               signal = mywait (&status, 1);
               prepare_resume_reply (own_buf, status, signal);
               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';
+                 }
+               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;
+             }
+           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';
+                 }
+               else
+                 {
+                   int res;
+
+                   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);
+                 }
+               break;
+             }
             case 'k':
               fprintf (stderr, "Killing inferior\n");
               kill_inferior ();

-- 
Orjan Friberg
Axis Communications