From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cagney To: Fernando Nasser Cc: GDB Patches Subject: Runtiming hw watchpoints; Was: Split up Z packet enable/disable cmd Date: Wed, 02 Aug 2000 20:21:00 -0000 Message-id: <3988E54B.39646794@cygnus.com> References: <3987DD9A.21F2B29A@cygnus.com> <39884DFA.12DD4043@cygnus.com> X-SW-Source: 2000-08/msg00059.html (I'm going to think out loud...) > > There are 3 new target vector entries: > to_Z_packet_supported_p > to_insert_watchpoint > to_remove_watchpoint >From memory watchpoints can be implemented at two levels: o GDB directly pokes watchpoint registers o GDB asks the target to take care of the problem First thing to keep in mind is that everytime this is pointed out the discussion goes off on a religious tangent where people argue how one or the other is the only correct solution ... I think they both are equally correct :-) Something less target specific than to_Z_packet_supported_p() is going to be needed :-) > > PS: It doesn't fix the compile time test for remote watchpoints but it > > does enable the code always (so that I've a better chance of not > > breaking it :-). > > > Well, my patch takes care of that part. This is a good opportunity to ask what > people think of my solution. It is working on a beta version and a candidate > for being submitted as a complement to Andrews patch. > > I have set TARGET_CAN_USE_HARDWARE_WATCHPOINT to a function which will call > target_Z_packet_supported_p(). remote_Z_packet_supported_p() sends a "probe" > packet if the support is set to auto and we haven't asserted if the target > accepts or not the Z packet (OK, I will have to make it Z4 or something). > > If the Z-packet support is in auto and we have not connected to the target yet, > we cannot access if watchpoints are allowed (if it is set to enabled or disabled > we assume the user knows). In this case I print: > "Cannot set watchpoints before connecting to remote target: Z-packet support unknown." > > That is the only real restriction. As I don't meddle at all with the breakpoint.c > code it results in a very simple fix. I don't think this is reasonable. I think a better model would involve: (gdb) watch c Watchpoint 1: c (gdb) watch b Watchpoint 2: b (gdb) target xxx (gdb) try-watchpoints-for-size OR run Watchpoint 1 hardware Watchpoint 2 doesn't fit (gdb) run I.E. delay assigning watchpoints to hardware/software until the last moment. Instead of having watch_command_1() try to select a hw/sw watchpoint before it knows the answer, the code inserting the watchpoint could do it and just set ->hw if it succeeded. >From memory, GDB refuses to mix hardware and software watchpoints which should greatly simplify the implementation. Andrew >From eliz@delorie.com Wed Aug 02 23:02:00 2000 From: Eli Zaretskii To: ac131313@cygnus.com Cc: fnasser@cygnus.com, gdb-patches@sourceware.cygnus.com Subject: Re: Runtiming hw watchpoints; Was: Split up Z packet enable/disable cmd Date: Wed, 02 Aug 2000 23:02:00 -0000 Message-id: <200008030602.CAA26491@indy.delorie.com> References: <3987DD9A.21F2B29A@cygnus.com> <39884DFA.12DD4043@cygnus.com> <3988E54B.39646794@cygnus.com> X-SW-Source: 2000-08/msg00060.html Content-length: 1697 > Date: Thu, 03 Aug 2000 13:21:47 +1000 > From: Andrew Cagney > > I.E. delay assigning watchpoints to hardware/software until the last > moment. Instead of having watch_command_1() try to select a hw/sw > watchpoint before it knows the answer, the code inserting the watchpoint > could do it and just set ->hw if it succeeded. Yes, I think this is the only reasonable way, eventually. The problem is that the code which is run when the user says "watch foo" doesn't know how many other watchpoints, and of what kind, will be needed when the inferior is resumed. In particular, all kinds of breakpoints with commands that set, reset, or enable watchpoints may be lying around; a target may be able to combine several watchpoints into one, but only under some conditions; etc. You don't know all that stuff until it's time to resume, and only the target code knows the entire truth. Automatic fallback to software watchpoints might be useful, but IMHO it must be a user option, because in many cases, if I know that my resources for hardware watchpoints are not enough to cover all of the watches, I might wish to reconsider instead of letting the program to crawl... However, with the current machinery, it is very hard to delay the hw/sw decision to the point where the inferior is resumed. This is because high-level code in GDB needs to know whether there are any software watchpoints, to arrange for single-stepping. Since the decision can only be reliably made by the target-specific code, by the time it knows that, it's too late. We need some way of communicating that information back to GDB's application code, before resume() and its brethren is called. >From ac131313@cygnus.com Thu Aug 03 01:47:00 2000 From: Andrew Cagney To: GDB Patches Subject: [patch] remote-mips.c param fixes Date: Thu, 03 Aug 2000 01:47:00 -0000 Message-id: <398931B2.1FD60AA2@cygnus.com> X-SW-Source: 2000-08/msg00061.html Content-length: 2086 FYI, More fallout from the K&R->ISO conversion. Andrew Thu Aug 3 15:02:23 2000 Andrew Cagney * remote-mips.c (mips_expect, mips_expect_timeout, common_open, fputs_readable): Make string pointer arguments constant. Index: remote-mips.c =================================================================== RCS file: /cvs/cvsfiles/devo/gdb/remote-mips.c,v retrieving revision 2.105 diff -p -r2.105 remote-mips.c *** remote-mips.c 2000/07/30 01:49:39 2.105 --- remote-mips.c 2000/08/03 07:56:52 *************** fputc_readable (int ch, struct ui_file * *** 526,532 **** ^x notation or in hex. */ static void ! fputs_readable (char *string, struct ui_file *file) { int c; --- 526,532 ---- ^x notation or in hex. */ static void ! fputs_readable (const char *string, struct ui_file *file) { int c; *************** fputs_readable (char *string, struct ui_ *** 540,548 **** */ int ! mips_expect_timeout (char *string, int timeout) { ! char *p = string; if (remote_debug) { --- 540,548 ---- */ int ! mips_expect_timeout (const char *string, int timeout) { ! const char *p = string; if (remote_debug) { *************** mips_expect_timeout (char *string, int t *** 596,602 **** */ int ! mips_expect (char *string) { return mips_expect_timeout (string, 2); } --- 596,602 ---- */ int ! mips_expect (const char *string) { return mips_expect_timeout (string, 2); } *************** mips_initialize (void) *** 1499,1505 **** /* Open a connection to the remote board. */ static void common_open (struct target_ops *ops, char *name, int from_tty, ! enum mips_monitor_type new_monitor, char *new_monitor_prompt) { char *ptype; char *serial_port_name; --- 1499,1506 ---- /* Open a connection to the remote board. */ static void common_open (struct target_ops *ops, char *name, int from_tty, ! enum mips_monitor_type new_monitor, ! const char *new_monitor_prompt) { char *ptype; char *serial_port_name; >From taylor@cygnus.com Thu Aug 03 07:45:00 2000 From: David Taylor To: Jimmy Guo Cc: gdb-patches@sourceware.cygnus.com Subject: Re: [PATCH] language support: case sensitivity Date: Thu, 03 Aug 2000 07:45:00 -0000 Message-id: <200008031433.KAA04437@texas.cygnus.com> References: X-SW-Source: 2000-08/msg00062.html Content-length: 2019 Date: Wed, 2 Aug 2000 09:55:23 -0700 (PDT) From: Jimmy Guo I don't quite understand yet the need for having a new show command routine ... command.c (do_setshow_command) seems to be the one that handle the normal output of settings. Also, show_language_command has similar setup as show_case_command (). Do you have an example on the kind of behavior you'd like to see but not supported currently by this patch? - Jimmy Guo, guo@cup.hp.com You added two commands: set case-sensitivity show case-sensitivity And if the user types "show case-sensitivity", nothing will print unless the sensitivity differs from that of the current language. I feel that if the user *ASKED* to be shown the case sensitivity, then the user should be *SHOWN* the case sensitivity. If the user explicitily asks what it is, then it should not be silent. So, you could have the command "show case-sensitivity" print something like: Case sensitivity is on. warning: current language, fortran, is case insensitive. Where the first line is printed by a new function (to be called when the user types "show case-sensitivity"), and the second line is printed by the function (renamed) that currently implements "show case-sensitivity". [...] >Your new function show_case_command plays double duty -- it is both >invoked by other functions / commands and it is invoked by the user in >response to the 'show case' command. And while it is quite reasonable >for it to be silent when it is *NOT* invoked by the user, it should >not be silent when it is invoked by the user. > >My suggestion is to define two functions: > >. one, the new show_case_command, which is never silent -- it always >tells you the setting. > >. the other, the current show_case_command, prints a warning if >appropriate and is called by the current callers of show_case_command. > >Otherwise it looks fine to me. Thanks for submitting this.