From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cagney To: Mark Salter Cc: gdb-patches@sources.redhat.com Subject: Re: remote watchpoint support Date: Thu, 28 Jun 2001 15:09:00 -0000 Message-id: <3B3BAB07.4050203@cygnus.com> References: <200011012131.eA1LVa514840@deneb.localdomain> X-SW-Source: 2001-06/msg00480.html Mark, Did you ever get a chance to check this (after fixes) in? Eli was ok with the doc changes and I think you addressed my questions. Andrew > Ok. Here's a patch that allows a remote target to pass watchpoint > information back to gdb using the T packet. It also fixes the T > packet handling to ignore other optional info which may not be > recognized. This changes the behavior to match the documentation. > > While testing this, I discovered that GDB tries to read data from > the watched address before the watchpoints are removed. This causes > another watchpoint exception so the stub returns an error response > to the read memory request. > > --Mark > > > Index: ChangeLog > =================================================================== > RCS file: /cvs/src/src/gdb/ChangeLog,v > retrieving revision 1.744 > diff -p -r1.744 ChangeLog > *** ChangeLog 2000/10/31 19:35:03 1.744 > --- ChangeLog 2000/11/01 21:06:26 > *************** > *** 1,3 **** > --- 1,10 ---- > + 2000-11-01 Mark Salter > + > + * remote.c (remote_wait): Add support to extract optional > + watchpoint information from T-packet. Ignore unrecognized > + optional info in T-packet. > + (remote_async_wait): Ditto. > + > 2000-10-30 Michael Snyder > * config/sh/tm-linux.h: New file. Include generic tm-linux.h, > Index: remote.c > =================================================================== > RCS file: /cvs/src/src/gdb/remote.c,v > retrieving revision 1.26 > diff -p -r1.26 remote.c > *** remote.c 2000/10/23 22:49:28 1.26 > --- remote.c 2000/11/01 21:06:29 > *************** void _initialize_remote (void); > *** 204,209 **** > --- 204,216 ---- > > /* */ > > + /* This is set to the data address of the access causing the target > + * to stop for a watchpoint. */ > + static CORE_ADDR remote_watch_data_address; > + > + /* This is non-zero if taregt stopped for a watchpoint. */ > + static int remote_stopped_by_watchpoint_p; > + > static struct target_ops remote_ops; > > static struct target_ops extended_remote_ops; > *************** struct gdb_ext_thread_info > *** 963,969 **** > > #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2) > > ! char *unpack_varlen_hex (char *buff, int *result); > > static char *unpack_nibble (char *buf, int *val); > > --- 970,976 ---- > > #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2) > > ! char *unpack_varlen_hex (char *buff, ULONGEST *result); > > static char *unpack_nibble (char *buf, int *val); > > *************** stub_unpack_int (char *buff, int fieldle > *** 1084,1090 **** > > char * > unpack_varlen_hex (char *buff, /* packet to parse */ > ! int *result) > { > int nibble; > int retval = 0; > --- 1091,1097 ---- > > char * > unpack_varlen_hex (char *buff, /* packet to parse */ > ! ULONGEST *result) > { > int nibble; > int retval = 0; > *************** remote_wait (int pid, struct target_wait > *** 2593,2598 **** > --- 2600,2606 ---- > { > unsigned char *buf = alloca (PBUFSIZ); > int thread_num = -1; > + ULONGEST ul; > > status->kind = TARGET_WAITKIND_EXITED; > status->value.integer = 0; > *************** remote_wait (int pid, struct target_wait > *** 2610,2615 **** > --- 2618,2625 ---- > if (target_wait_loop_hook) > (*target_wait_loop_hook) (); > > + remote_stopped_by_watchpoint_p = 0; > + > switch (buf[0]) > { > case 'E': /* Error of some sort */ > *************** Packet: '%s'\n", > *** 2647,2656 **** > p, buf); > if (strncmp ((const char *) p, "thread", p1 - p) == 0) > { > ! p_temp = unpack_varlen_hex (++p1, &thread_num); > record_currthread (thread_num); > p = (unsigned char *) p_temp; > } > } > else > { > --- 2657,2682 ---- > p, buf); > if (strncmp ((const char *) p, "thread", p1 - p) == 0) > { > ! p_temp = unpack_varlen_hex (++p1, &ul); > ! thread_num = ul; > record_currthread (thread_num); > p = (unsigned char *) p_temp; > } > + else if ((strncmp ((const char *) p, "watch", p1 - p) == 0) > + || (strncmp ((const char *) p, "rwatch", p1 - p) == 0) > + || (strncmp ((const char *) p, "awatch", p1 - p) == 0)) > + { > + remote_stopped_by_watchpoint_p = 1; > + p = unpack_varlen_hex (++p1, &ul); > + remote_watch_data_address = (CORE_ADDR)ul; > + } > + else > + { > + /* silently skip unknown optional info */ > + p_temp = (unsigned char *) strchr ((const char *) p1+1, ';'); > + if (p_temp) > + p = p_temp; > + } > } > else > { > *************** remote_async_wait (int pid, struct targe > *** 2808,2813 **** > --- 2834,2840 ---- > { > unsigned char *buf = alloca (PBUFSIZ); > int thread_num = -1; > + ULONGEST ul; > > status->kind = TARGET_WAITKIND_EXITED; > status->value.integer = 0; > *************** remote_async_wait (int pid, struct targe > *** 2831,2836 **** > --- 2858,2865 ---- > if (target_wait_loop_hook) > (*target_wait_loop_hook) (); > > + remote_stopped_by_watchpoint_p = 0; > + > switch (buf[0]) > { > case 'E': /* Error of some sort */ > *************** Packet: '%s'\n", > *** 2868,2877 **** > p, buf); > if (strncmp ((const char *) p, "thread", p1 - p) == 0) > { > ! p_temp = unpack_varlen_hex (++p1, &thread_num); > record_currthread (thread_num); > p = (unsigned char *) p_temp; > } > } > else > { > --- 2897,2922 ---- > p, buf); > if (strncmp ((const char *) p, "thread", p1 - p) == 0) > { > ! p_temp = unpack_varlen_hex (++p1, &ul); > ! thread_num = ul; > record_currthread (thread_num); > p = (unsigned char *) p_temp; > } > + else if ((strncmp ((const char *) p, "watch", p1 - p) == 0) > + || (strncmp ((const char *) p, "rwatch", p1 - p) == 0) > + || (strncmp ((const char *) p, "awatch", p1 - p) == 0)) > + { > + remote_stopped_by_watchpoint_p = 1; > + p = unpack_varlen_hex (++p1, &ul); > + remote_watch_data_address = (CORE_ADDR)ul; > + } > + else > + { > + /* silently skip unknown optional info */ > + p_temp = (unsigned char *) strchr ((const char *) p1+1, ';'); > + if (p_temp) > + p = p_temp; > + } > } > else > { > *************** remote_remove_hw_breakpoint (CORE_ADDR a > *** 4484,4489 **** > --- 4529,4551 ---- > } > internal_error ("remote_remove_watchpoint: reached end of function"); > } > + > + /* Returns data address if stopped by watchpoint. > + Zero otherwise. */ > + CORE_ADDR > + remote_stopped_data_address (void) > + { > + if (remote_stopped_by_watchpoint_p) > + return remote_watch_data_address; > + return (CORE_ADDR)0; > + } > + > + int > + remote_stopped_by_watchpoint(void) > + { > + return remote_stopped_by_watchpoint_p; > + } > + > > /* Some targets are only capable of doing downloads, and afterwards > they switch to the remote serial protocol. This function provides > Index: doc/ChangeLog > =================================================================== > RCS file: /cvs/src/src/gdb/doc/ChangeLog,v > retrieving revision 1.51 > diff -p -r1.51 ChangeLog > *** ChangeLog 2000/10/16 07:34:02 1.51 > --- ChangeLog 2000/11/01 21:06:31 > *************** > *** 1,3 **** > --- 1,8 ---- > + 2000-11-01 Mark Salter > + > + * gdb.texinfo (Protocol): Document T packet extension to > + allow watchpoint address reporting. > + > 2000-10-16 Eli Zaretskii > * gdb.texinfo (Contributors, MIPS Embedded): Minor spelling > Index: doc/gdb.texinfo > =================================================================== > RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v > retrieving revision 1.28 > diff -p -r1.28 gdb.texinfo > *** gdb.texinfo 2000/10/16 07:34:02 1.28 > --- gdb.texinfo 2000/11/01 21:06:40 > *************** conventions is used. > *** 9390,9399 **** > @var{AA} = two hex digit signal number; @var{n...} = register number > (hex), @var{r...} = target byte ordered register contents, size defined > by @code{REGISTER_RAW_SIZE}; @var{n...} = @samp{thread}, @var{r...} = > ! thread process ID, this is a hex integer; @var{n...} = other string not > ! starting with valid hex digit. @value{GDBN} should ignore this > ! @var{n...}, @var{r...} pair and go on to the next. This way we can > ! extend the protocol. > > @item @code{W}@var{AA} > @tab > --- 9390,9400 ---- > @var{AA} = two hex digit signal number; @var{n...} = register number > (hex), @var{r...} = target byte ordered register contents, size defined > by @code{REGISTER_RAW_SIZE}; @var{n...} = @samp{thread}, @var{r...} = > ! thread process ID, this is a hex integer; @var{n...} = (@samp{watch} | > ! @samp{rwatch} | @samp{awatch}, @var{r...} = data address, this is a hex > ! integer; @var{n...} = other string not starting with valid hex digit. > ! @value{GDBN} should ignore this @var{n...}, @var{r...} pair and go on > ! to the next. This way we can extend the protocol. > > @item @code{W}@var{AA} > @tab > >