From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eli Zaretskii To: fnasser@cygnus.com Cc: ac131313@cygnus.com, gdb-patches@sourceware.cygnus.com Subject: Re: Runtiming hw watchpoints; Was: Split up Z packet enable/disable cmd Date: Fri, 04 Aug 2000 23:57:00 -0000 Message-id: <200008050656.CAA29256@indy.delorie.com> References: <3987DD9A.21F2B29A@cygnus.com> <39884DFA.12DD4043@cygnus.com> <3988E54B.39646794@cygnus.com> <200008030602.CAA26491@indy.delorie.com> <39898EA6.4683E9F2@cygnus.com> <200008031820.OAA27128@indy.delorie.com> <3989C2A4.C5322586@cygnus.com> <200008040922.FAA28046@indy.delorie.com> <398ACA49.8C933672@cygnus.com> <200008041810.OAA28510@indy.delorie.com> <398B1590.27E611B8@cygnus.com> X-SW-Source: 2000-08/msg00120.html > Date: Fri, 04 Aug 2000 15:12:16 -0400 > From: Fernando Nasser > > Eli Zaretskii wrote: > > > > t could do this if it gets the info about all > > the watchpoints and hardware breakpoints defined previously. > > Currently, it only gets information about one single watchpoint at a > > time, and that is not enough, at least on x86. > > > You are absolutely right. I missed that. We can add a field in the breakpoint > structure and store the number of hw registers used by a hw watchpoint. > Then we can fix hw_watchpoint_used_count() to add them up. > Does it sound right? This would be a huge step in the right direction, but it is still not enough. Imagine a situation where a user sets 2 different watchpoints at the same address. (This might make sense if these watchpoints have different conditions, for example.) DJGPP currently can insert such watchpoints using a single debug register, by implementing reference counts for debug registers. I believe that eventually, other x86 platforms will use a similar technique, since the number of debug registers is so small. (We had a discussion some time ago where everybody agreed that all x86 targets should share the code which handles watchpoints.) In situations like this, it is not enough to know how many debug registers are already taken up. You need to know everything about those other watchpoints. (In the x86 case, you need to know only the type of the watchpoint, and the address and the size of the watched region, but other platforms might want to know more.) So I think we have two alternatives to solving this: - Make can_use_hardware_watchpoint loop over all the watchpoints before you ask it about the additional one, so that the target end could get the entire picture. - Invent a new API call whereby the target will add and remove the info about the watchpoints and breakpoints to some internal storage at the target end. The latter seems like a cleaner solution, and also has a benefit that it might remove the need for inserting the breakpoints and watchpoints one by one when resuming: you could simply tell the target to insert all active ones in one go. But it does mean more extensive changes... > But I guess this will only happen if we try to use it for good. > No pain, no gain :-) Sure. Personally, I use watchpoints a lot, and would be glad to see any improvements in their handling. The current code in go32-nat.c works well enough for me, but it is messy and full of pitfalls. I will welcome any changes in higher-level APIs that would make this simpler and cleaner. > I am merging my Pentium III changes to the current sourceware code. > I will retest things and post some patches as soon as I get them to > work again. We can start from that. Great! >From eliz@delorie.com Fri Aug 04 23:58:00 2000 From: Eli Zaretskii To: kevinb@cygnus.com Cc: gdb-patches@sourceware.cygnus.com Subject: Re: [PATCH RFC] Protoize ch-exp.c, core-regset.c Date: Fri, 04 Aug 2000 23:58:00 -0000 Message-id: <200008050658.CAA29260@indy.delorie.com> References: <1000803190941.ZM2697@ocotillo.lan> <200008040931.FAA28059@indy.delorie.com> <1000804151730.ZM4041@ocotillo.lan> <200008041810.OAA28507@indy.delorie.com> <1000805000138.ZM5802@ocotillo.lan> X-SW-Source: 2000-08/msg00121.html Content-length: 759 > Date: Fri, 4 Aug 2000 17:01:38 -0700 > From: Kevin Buettner > > #if 0 > +/* Parse the name of an option string. If ALLOW_ALL is 1, ALL is > + allowed as a postfix. */ > + > static tree > -parse_opt_name_string (allow_all) > - int allow_all; /* 1 if ALL is allowed as a postfix */ > +parse_opt_name_string (int allow_all) Err... now that I actually *look* at ch-exp.c, it seems that this function should be described simply as "Parse name_string." I cannot figure out what does the "opt" part stand for; perhaps it hints at the optional ALLOW_ALL parameter. Sorry, I should have told that the description I showed in my previous message was meant to be an example, not suggest that this is the actual function description. >From geoffk@cygnus.com Sat Aug 05 01:15:00 2000 From: Geoff Keating To: guo@cup.hp.com Cc: gdb-patches@sourceware.cygnus.com Subject: Re: recent dejagnu changes Date: Sat, 05 Aug 2000 01:15:00 -0000 Message-id: <200008050810.BAA11524@localhost.cygnus.com> References: X-SW-Source: 2000-08/msg00122.html Content-length: 792 > Date: Fri, 4 Aug 2000 23:33:47 -0700 (PDT) > From: Jimmy Guo > The problem is that I thought a dejagnu test tree contains > ./ as a proper suite name, anywhere. However apparently > the GCC suite just requires the top level suite name be such. I think, in the absence of documentation, you have to assume that the original code was correct and the way you're using it was wrong :-(. > Actually, the dir_to_run and cmdline_dir_to_run change is OK ... that > allows specification of a list of test suites, instead of one at a time. Could you post these as a separate patch, and test them separately? They were committed together, and it's difficult to know which part of the commit belongs with which change. -- - Geoffrey Keating >From kettenis@wins.uva.nl Sat Aug 05 01:54:00 2000 From: Mark Kettenis To: kevinb@cygnus.com Cc: gdb-patches@sourceware.cygnus.com Subject: Re: [PATCH RFC] Protoize ch-exp.c, core-regset.c Date: Sat, 05 Aug 2000 01:54:00 -0000 Message-id: <200008050854.e758sNh00394@delius.kettenis.local> References: <1000803190941.ZM2697@ocotillo.lan> <200008040931.FAA28059@indy.delorie.com> <1000804151730.ZM4041@ocotillo.lan> <200008041810.OAA28507@indy.delorie.com> <1000805000138.ZM5802@ocotillo.lan> X-SW-Source: 2000-08/msg00123.html Content-length: 967 Date: Fri, 4 Aug 2000 17:01:38 -0700 From: Kevin Buettner Index: core-regset.c =================================================================== RCS file: /cvs/src/src/gdb/core-regset.c,v retrieving revision 1.5 diff -u -r1.5 core-regset.c --- core-regset.c 2000/07/30 01:48:25 1.5 +++ core-regset.c 2000/08/04 23:35:40 @@ -72,7 +72,8 @@ Read the values of either the general register set (WHICH equals 0) or the floating point register set (WHICH equals 2) from the core file data (pointed to by CORE_REG_SECT), and update gdb's idea of - their current values. The CORE_REG_SIZE parameter is ignored. + their current values. The CORE_REG_SIZE and REG_ADDR parameters are + ignored. NOTES If you look at the actual code, you'll see that CORE_REG_SIZE isn't really ignored, but is used to check if the structure size matches the amount of data in the core file. Mark >From fnasser@cygnus.com Sat Aug 05 09:29:00 2000 From: Fernando Nasser To: Geoff Keating Cc: guo@cup.hp.com, gdb-patches@sourceware.cygnus.com Subject: Re: recent dejagnu changes Date: Sat, 05 Aug 2000 09:29:00 -0000 Message-id: <398C3E91.D0EDB934@cygnus.com> References: <200008050810.BAA11524@localhost.cygnus.com> X-SW-Source: 2000-08/msg00124.html Content-length: 910 Geoff Keating wrote: > > > Actually, the dir_to_run and cmdline_dir_to_run change is OK ... that > > allows specification of a list of test suites, instead of one at a time. > We do want to retain this functionality. > Could you post these as a separate patch, and test them separately? > They were committed together, and it's difficult to know which part > of the commit belongs with which change. > Better than that would be if Jimmy submitted a patch commenting out (with the explanation why) the code that he believes is the offending one. If that passes the gcc testsuite tests we commit it. I wonder if there isn't a way to accomodate both the gcc tree structure and the HP one... -- Fernando Nasser Red Hat Canada Ltd. E-Mail: fnasser@cygnus.com 2323 Yonge Street, Suite #300 Tel: 416-482-2661 ext. 311 Toronto, Ontario M4P 2C9 Fax: 416-482-6299 >From kevinb@cygnus.com Sat Aug 05 11:01:00 2000 From: Kevin Buettner To: gdb-patches@sourceware.cygnus.com Subject: Re: [PATCH RFC] Protoize ch-exp.c, core-regset.c Date: Sat, 05 Aug 2000 11:01:00 -0000 Message-id: <1000805180139.ZM7354@ocotillo.lan> References: <1000803190941.ZM2697@ocotillo.lan> <200008040931.FAA28059@indy.delorie.com> <1000804151730.ZM4041@ocotillo.lan> <200008041810.OAA28507@indy.delorie.com> <1000805000138.ZM5802@ocotillo.lan> X-SW-Source: 2000-08/msg00125.html Content-length: 2348 Let's try this (yet) again... The previous versions of this patch at http://sources.redhat.com/ml/gdb-patches/2000-08/msg00073.html and http://sources.redhat.com/ml/gdb-patches/2000-08/msg00113.html are withdrawn. Thanks to Mark Kettenis and Eli Zaretskii for pointing out problems with the previous patch. * ch-exp.c (parse_opt_name_string): Protoize. [Thanks to Eli Zaretskii for the prefatory comment.] * core-regset.c (fetch_core_registers): Protoize; revise comment. Index: ch-exp.c =================================================================== RCS file: /cvs/src/src/gdb/ch-exp.c,v retrieving revision 1.4 diff -u -r1.4 ch-exp.c --- ch-exp.c 2000/07/30 01:48:24 1.4 +++ ch-exp.c 2000/08/05 17:56:35 @@ -301,9 +301,10 @@ } #if 0 +/* Parse a name string. If ALLOW_ALL is 1, ALL is allowed as a postfix. */ + static tree -parse_opt_name_string (allow_all) - int allow_all; /* 1 if ALL is allowed as a postfix */ +parse_opt_name_string (int allow_all) { int token = PEEK_TOKEN (); tree name; Index: core-regset.c =================================================================== RCS file: /cvs/src/src/gdb/core-regset.c,v retrieving revision 1.5 diff -u -r1.5 core-regset.c --- core-regset.c 2000/07/30 01:48:25 1.5 +++ core-regset.c 2000/08/05 17:56:37 @@ -72,20 +72,16 @@ Read the values of either the general register set (WHICH equals 0) or the floating point register set (WHICH equals 2) from the core file data (pointed to by CORE_REG_SECT), and update gdb's idea of - their current values. The CORE_REG_SIZE parameter is ignored. + their current values. The CORE_REG_SIZE parameter is compared to + the size of the gregset or fpgregset structures (as appropriate) to + validate the size of the structure from the core file. The + REG_ADDR parameter is ignored. - NOTES - - Use the indicated sizes to validate the gregset and fpregset - structures. */ static void -fetch_core_registers (core_reg_sect, core_reg_size, which, reg_addr) - char *core_reg_sect; - unsigned core_reg_size; - int which; - CORE_ADDR reg_addr; /* Unused in this version */ +fetch_core_registers (char *core_reg_sect, unsigned core_reg_size, int which, + CORE_ADDR reg_addr) { #if defined (HAVE_GREGSET_T) && defined (HAVE_FPREGSET_T) gregset_t gregset;