From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Salter To: ac131313@cygnus.com Cc: gdb-patches@sources.redhat.com Subject: Re: remote watchpoint support Date: Thu, 28 Jun 2001 16:17:00 -0000 Message-id: <200106282317.f5SNHH011029@deneb.localdomain> References: <200011012131.eA1LVa514840@deneb.localdomain> <3B3BAB07.4050203@cygnus.com> X-SW-Source: 2001-06/msg00488.html No. There was some concern about the globals (remote_watch_data_address and remote_stopped_by_watchpoint_p) and I never got the green light. The globals are ugly and I think we could get rid of them with a little more effort. I just haven't had time to pursue that. --Mark > 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 >> >>