From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Ingham To: gdb-patches@sourceware.cygnus.com Subject: Re: Resizing the to_sections target vector field. Date: Wed, 22 Sep 1999 17:16:00 -0000 Message-id: References: <14313.20900.407612.412542@leda.cygnus.com> X-SW-Source: 1999-q3/msg00263.html Oops, I hadn't saved all my Xemacs buffers before making the diff. In the osfsolib.c & irix5-nat.c files, the line target_resize_to_sections (target, count); should really be: old = target_resize_to_sections (target, count); Sorry 'bout that. Jim -- ++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++ Jim Ingham jingham@cygnus.com Cygnus Solutions Inc. >From jimb@cygnus.com Thu Sep 23 13:18:00 1999 From: Jim Blandy To: jtc@redback.com, David Taylor Cc: gdb-patches@sourceware.cygnus.com Subject: Re: patch to write_dollar_variable() Date: Thu, 23 Sep 1999 13:18:00 -0000 Message-id: References: <5maerlncv8.fsf@jtc.redbacknetworks.com> X-SW-Source: 1999-q3/msg00264.html Content-length: 5476 > As previously discussed on the gdb list, this patch is necessary. At > least until symbol table handling is greatly improved... :-) > > --jtc > > > 1999-08-20 J.T. Conklin > > * parse.c (write_dollar_variable): If HPUXHPPA is not defined, > don't search for $ variables in the symbol table. Worst case > symbol table lookup performance is extremely poor. This causes > GDB scripts that use convenience variables to execute so slowly > to be almost unusable. Testing HPUXHPPA is a mess, though. How about this? 1999-09-23 Jim Blandy * parse.c (write_dollar_variable): Don't check the symbol table for identifiers beginning with `$' unless IDENTIFIERS_CAN_START_WITH_DOLLAR is #defined. * config/pa/tm-hppa.h (IDENTIFIERS_CAN_START_WITH_DOLLAR): Define. * doc/gdbint.texinfo (IDENTIFIERS_CAN_START_WITH_DOLLAR): Document. Index: parse.c =================================================================== RCS file: /cvs/cvsfiles/devo/gdb/parse.c,v retrieving revision 2.67 diff -c -r2.67 parse.c *** parse.c 1999/09/01 20:50:45 2.67 --- parse.c 1999/09/23 20:14:52 *************** *** 460,468 **** /* Handle the tokens $digits; also $ (short for $0) and $$ (short for $$1) and $$digits (equivalent to $<-digits> if you could type that). */ - struct symbol *sym = NULL; - struct minimal_symbol *msym = NULL; - int negate = 0; int i = 1; /* Double dollar means negate the number and add -1 as well. --- 460,465 ---- *************** *** 496,523 **** if (i >= 0) goto handle_register; ! /* On HP-UX, certain system routines (millicode) have names beginning ! with $ or $$, e.g. $$dyncall, which handles inter-space procedure ! calls on PA-RISC. Check for those, first. */ ! sym = lookup_symbol (copy_name (str), (struct block *) NULL, ! VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL); ! if (sym) ! { ! write_exp_elt_opcode (OP_VAR_VALUE); ! write_exp_elt_block (block_found); /* set by lookup_symbol */ ! write_exp_elt_sym (sym); ! write_exp_elt_opcode (OP_VAR_VALUE); ! return; ! } ! msym = lookup_minimal_symbol (copy_name (str), NULL, NULL); ! if (msym) ! { ! write_exp_msymbol (msym, ! lookup_function_type (builtin_type_int), ! builtin_type_int); ! return; ! } /* Any other names starting in $ are debugger internal variables. */ --- 493,530 ---- if (i >= 0) goto handle_register; ! #if defined(IDENTIFIERS_CAN_START_WITH_DOLLAR) ! { ! struct symbol *sym = NULL; ! struct minimal_symbol *msym = NULL; ! /* On HP-UX, certain system routines (millicode) have names beginning ! with $ or $$, e.g. $$dyncall, which handles inter-space procedure ! calls on PA-RISC. Check for those, first. */ ! ! /* This code is not enabled on non HP-UX systems, since worst case ! symbol table lookup performance is awful, to put it mildly. */ ! ! sym = lookup_symbol (copy_name (str), (struct block *) NULL, ! VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL); ! if (sym) ! { ! write_exp_elt_opcode (OP_VAR_VALUE); ! write_exp_elt_block (block_found); /* set by lookup_symbol */ ! write_exp_elt_sym (sym); ! write_exp_elt_opcode (OP_VAR_VALUE); ! return; ! } ! msym = lookup_minimal_symbol (copy_name (str), NULL, NULL); ! if (msym) ! { ! write_exp_msymbol (msym, ! lookup_function_type (builtin_type_int), ! builtin_type_int); ! return; ! } ! } ! #endif /* Any other names starting in $ are debugger internal variables. */ Index: config/pa/tm-hppa.h =================================================================== RCS file: /cvs/cvsfiles/devo/gdb/config/pa/tm-hppa.h,v retrieving revision 1.76 diff -c -r1.76 tm-hppa.h *** tm-hppa.h 1999/09/17 16:12:32 1.76 --- tm-hppa.h 1999/09/23 20:14:53 *************** *** 799,801 **** --- 799,807 ---- /* Here's how to step off a permanent breakpoint. */ #define SKIP_PERMANENT_BREAKPOINT (hppa_skip_permanent_breakpoint) extern void hppa_skip_permanent_breakpoint (void); + + /* On HP-UX, certain system routines (millicode) have names beginning + with $ or $$, e.g. $$dyncall, which handles inter-space procedure + calls on PA-RISC. Tell the expression parser to check for those + when parsing tokens that begin with "$". */ + #define IDENTIFIERS_CAN_START_WITH_DOLLAR Index: doc/gdbint.texinfo =================================================================== RCS file: /cvs/cvsfiles/devo/gdb/doc/gdbint.texinfo,v retrieving revision 1.136 diff -c -r1.136 gdbint.texinfo *** gdbint.texinfo 1999/09/15 21:27:39 1.136 --- gdbint.texinfo 1999/09/23 20:14:58 *************** *** 1444,1449 **** --- 1444,1458 ---- feature-specific macros. It was introduced in haste and we are repenting at leisure. + @item IDENTIFIERS_CAN_START_WITH_DOLLAR + Some systems have routines whose names start with @samp{$}. Defining + this macro tells GDB's expression parser to check for such routines when + parsing tokens that begin with @samp{$}. + + On HP-UX, certain system routines (millicode) have names beginning with + @samp{$} or @samp{$$}. For example, @code{$$dyncall} is a millicode + routine that handles inter-space procedure calls on PA-RISC. + @item IEEE_FLOAT Define this if the target system uses IEEE-format floating point numbers. >From shebs@cygnus.com Thu Sep 23 13:53:00 1999 From: Stan Shebs To: jimb@cygnus.com Cc: jtc@redback.com, taylor@cygnus.com, gdb-patches@sourceware.cygnus.com Subject: Re: patch to write_dollar_variable() Date: Thu, 23 Sep 1999 13:53:00 -0000 Message-id: <199909232052.NAA02680@andros.cygnus.com> References: X-SW-Source: 1999-q3/msg00265.html Content-length: 1612 From: Jim Blandy Date: 23 Sep 1999 15:17:00 -0500 > As previously discussed on the gdb list, this patch is necessary. At > least until symbol table handling is greatly improved... :-) > > --jtc > > > 1999-08-20 J.T. Conklin > > * parse.c (write_dollar_variable): If HPUXHPPA is not defined, > don't search for $ variables in the symbol table. Worst case > symbol table lookup performance is extremely poor. This causes > GDB scripts that use convenience variables to execute so slowly > to be almost unusable. Testing HPUXHPPA is a mess, though. How about this? 1999-09-23 Jim Blandy * parse.c (write_dollar_variable): Don't check the symbol table for identifiers beginning with `$' unless IDENTIFIERS_CAN_START_WITH_DOLLAR is #defined. * config/pa/tm-hppa.h (IDENTIFIERS_CAN_START_WITH_DOLLAR): Define. * doc/gdbint.texinfo (IDENTIFIERS_CAN_START_WITH_DOLLAR): Document. This is going in the right direction, but to keep in line with our future, I'd prefer to see a runtime test: if (IDENTIFIERS_CAN_START_WITH_DOLLAR) that is defaulted to 0 somewhere convenient (like at the top of parse.c). This shouldn't cost any more, because GCC can optimize out the whole block on non-HPs, and will always optimize in on HPs. If this ends up going through a gdbarch op in the future, there will be a small extra cost, although I'm sure that there are many better opportunities for script optimization before this test becomes an issue. Stan >From jimb@cygnus.com Thu Sep 23 16:46:00 1999 From: Jim Blandy To: Stan Shebs Cc: jtc@redback.com, taylor@cygnus.com, gdb-patches@sourceware.cygnus.com Subject: Re: patch to write_dollar_variable() Date: Thu, 23 Sep 1999 16:46:00 -0000 Message-id: References: <199909232052.NAA02680@andros.cygnus.com> X-SW-Source: 1999-q3/msg00266.html Content-length: 6101 > This is going in the right direction, but to keep in line with our > future, I'd prefer to see a runtime test: > > if (IDENTIFIERS_CAN_START_WITH_DOLLAR) > > that is defaulted to 0 somewhere convenient (like at the top of > parse.c). Duh. I knew that... 1999-09-23 Jim Blandy * parse.c (IDENTIFIERS_CAN_START_WITH_DOLLAR): New macro, whose value can be overridden by target files. (write_dollar_variable): Don't check the symbol table for identifiers beginning with `$' unless IDENTIFIERS_CAN_START_WITH_DOLLAR is non-zero. * config/pa/tm-hppa.h (IDENTIFIERS_CAN_START_WITH_DOLLAR): Define. * doc/gdbint.texinfo (IDENTIFIERS_CAN_START_WITH_DOLLAR): Document. Index: parse.c =================================================================== RCS file: /cvs/cvsfiles/devo/gdb/parse.c,v retrieving revision 2.67 diff -c -r2.67 parse.c *** parse.c 1999/09/01 20:50:45 2.67 --- parse.c 1999/09/23 23:39:00 *************** *** 44,49 **** --- 44,64 ---- #include "gdbcmd.h" #include "symfile.h" /* for overlay functions */ + /* Symbols which architectures can redefine. */ + + /* Some systems have routines whose names start with `$'. Giving this + macro a non-zero value tells GDB's expression parser to check for + such routines when parsing tokens that begin with `$'. + + On HP-UX, certain system routines (millicode) have names beginning + with `$' or `$$'. For example, `$$dyncall' is a millicode routine + that handles inter-space procedure calls on PA-RISC. */ + #ifndef IDENTIFIERS_CAN_START_WITH_DOLLAR + #define IDENTIFIERS_CAN_START_WITH_DOLLAR (0) + #endif + + + /* Global variables declared in parser-defs.h (and commented there). */ struct expression *expout; int expout_size; *************** *** 460,468 **** /* Handle the tokens $digits; also $ (short for $0) and $$ (short for $$1) and $$digits (equivalent to $<-digits> if you could type that). */ - struct symbol *sym = NULL; - struct minimal_symbol *msym = NULL; - int negate = 0; int i = 1; /* Double dollar means negate the number and add -1 as well. --- 475,480 ---- *************** *** 496,522 **** if (i >= 0) goto handle_register; ! /* On HP-UX, certain system routines (millicode) have names beginning ! with $ or $$, e.g. $$dyncall, which handles inter-space procedure ! calls on PA-RISC. Check for those, first. */ ! ! sym = lookup_symbol (copy_name (str), (struct block *) NULL, ! VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL); ! if (sym) ! { ! write_exp_elt_opcode (OP_VAR_VALUE); ! write_exp_elt_block (block_found); /* set by lookup_symbol */ ! write_exp_elt_sym (sym); ! write_exp_elt_opcode (OP_VAR_VALUE); ! return; ! } ! msym = lookup_minimal_symbol (copy_name (str), NULL, NULL); ! if (msym) { ! write_exp_msymbol (msym, ! lookup_function_type (builtin_type_int), ! builtin_type_int); ! return; } /* Any other names starting in $ are debugger internal variables. */ --- 508,543 ---- if (i >= 0) goto handle_register; ! if (IDENTIFIERS_CAN_START_WITH_DOLLAR) { ! struct symbol *sym = NULL; ! struct minimal_symbol *msym = NULL; ! ! /* On HP-UX, certain system routines (millicode) have names beginning ! with $ or $$, e.g. $$dyncall, which handles inter-space procedure ! calls on PA-RISC. Check for those, first. */ ! ! /* This code is not enabled on non HP-UX systems, since worst case ! symbol table lookup performance is awful, to put it mildly. */ ! ! sym = lookup_symbol (copy_name (str), (struct block *) NULL, ! VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL); ! if (sym) ! { ! write_exp_elt_opcode (OP_VAR_VALUE); ! write_exp_elt_block (block_found); /* set by lookup_symbol */ ! write_exp_elt_sym (sym); ! write_exp_elt_opcode (OP_VAR_VALUE); ! return; ! } ! msym = lookup_minimal_symbol (copy_name (str), NULL, NULL); ! if (msym) ! { ! write_exp_msymbol (msym, ! lookup_function_type (builtin_type_int), ! builtin_type_int); ! return; ! } } /* Any other names starting in $ are debugger internal variables. */ Index: config/pa/tm-hppa.h =================================================================== RCS file: /cvs/cvsfiles/devo/gdb/config/pa/tm-hppa.h,v retrieving revision 1.76 diff -c -r1.76 tm-hppa.h *** tm-hppa.h 1999/09/17 16:12:32 1.76 --- tm-hppa.h 1999/09/23 23:39:01 *************** *** 799,801 **** --- 799,807 ---- /* Here's how to step off a permanent breakpoint. */ #define SKIP_PERMANENT_BREAKPOINT (hppa_skip_permanent_breakpoint) extern void hppa_skip_permanent_breakpoint (void); + + /* On HP-UX, certain system routines (millicode) have names beginning + with $ or $$, e.g. $$dyncall, which handles inter-space procedure + calls on PA-RISC. Tell the expression parser to check for those + when parsing tokens that begin with "$". */ + #define IDENTIFIERS_CAN_START_WITH_DOLLAR Index: doc/gdbint.texinfo =================================================================== RCS file: /cvs/cvsfiles/devo/gdb/doc/gdbint.texinfo,v retrieving revision 1.136 diff -c -r1.136 gdbint.texinfo *** gdbint.texinfo 1999/09/15 21:27:39 1.136 --- gdbint.texinfo 1999/09/23 23:39:05 *************** *** 1444,1449 **** --- 1444,1458 ---- feature-specific macros. It was introduced in haste and we are repenting at leisure. + @item IDENTIFIERS_CAN_START_WITH_DOLLAR + Some systems have routines whose names start with @samp{$}. Giving this + macro a non-zero value tells GDB's expression parser to check for such + routines when parsing tokens that begin with @samp{$}. + + On HP-UX, certain system routines (millicode) have names beginning with + @samp{$} or @samp{$$}. For example, @code{$$dyncall} is a millicode + routine that handles inter-space procedure calls on PA-RISC. + @item IEEE_FLOAT Define this if the target system uses IEEE-format floating point numbers. >From shebs@cygnus.com Thu Sep 23 19:44:00 1999 From: Stan Shebs To: kevinb@cygnus.com Cc: jimb@cygnus.com, msnyder@cygnus.com, gdb-patches@sourceware.cygnus.com Subject: Re: RFA: breakpoint.c and infrun.c changes Date: Thu, 23 Sep 1999 19:44:00 -0000 Message-id: <199909240241.TAA03085@andros.cygnus.com> References: <990917194445.ZM3796@ocotillo.lan> X-SW-Source: 1999-q3/msg00267.html Content-length: 1665 Date: Fri, 17 Sep 1999 12:44:45 -0700 From: Kevin Buettner My patch for fixing this bug is below. The changes to breakpoint.c alone fix the bug (on the i386 anyway); the changes to infrun.c are cleanup. A very nice piece of analysis! I had to read through it about four times to understand it all, but the reasoning seems sound. As we discussed on the phone, testing on a small assortment of relevant platforms is still worthwhile, just in case practice is trickier than theory. :-) This will certainly help to shorten and straighten out the execution control code. As people have accreted hacks over the years, it becomes harder and harder to know why something is there, and one descends into voodoo-fixing (as old ChangeLog entries attest). I'd also like us to look into JimB's suggestion that the PC could be pre-adjusted according to DECR_PC_AFTER_BREAK before anything in execution control looks at it. The analysis for that would involve looking at all the places where the PC is saved away, there are quite a few of them. In any case, assuming testing doesn't report any regression, I'd like to get this patch in soon so I can do my next phase of infrun simplification - there are just three more gotos left in handle_inferior_event, but so far I can't see how to evaporate them validly without breaking the function into a number of smaller pieces. Also, it would be really great to add a testsuite case for temporary breakpoints and stepping that will always fail without your changes, and will pass otherwise, since it seems that there is currently no test to detect the failure you're fixing. Stan >From kevinb@cygnus.com Fri Sep 24 10:40:00 1999 From: Kevin Buettner To: Stan Shebs Cc: gdb-patches@sourceware.cygnus.com, jimb@cygnus.com, msnyder@cygnus.com Subject: Re: RFA: breakpoint.c and infrun.c changes Date: Fri, 24 Sep 1999 10:40:00 -0000 Message-id: <990924173707.ZM19747@ocotillo.lan> References: <199909240241.TAA03085@andros.cygnus.com> X-SW-Source: 1999-q3/msg00268.html Content-length: 2507 On Sep 23, 7:41pm, Stan Shebs wrote: > A very nice piece of analysis! I had to read through it about four > times to understand it all, but the reasoning seems sound. As we > discussed on the phone, testing on a small assortment of relevant > platforms is still worthwhile, just in case practice is trickier than > theory. :-) As I told you on the phone, I've tested on i386 linux and saw no regressions. I've tested with the d10v simulator and saw no regressions. I tried to test the i960, but the i960 simulator did not work well for me. I'll try against an actual board if you think it's important. (After looking at the nightly testing results for this target, I have doubts about being able to get it to work properly. The one recent test that did work properly had 1034 failures. But most of the recent and even not so recent tests show that the testsuite did not complete successfully.) I've tested on the alpha (running linux), but I'm not sure what to make of the results. On the one hand, I do see *fewer* failures after I apply my patch. But I've run the test suite a number of times and get different results each time I run it. The good thing is that there aren't any new failures in the breakpoint or single step tests. (I ran my tests on the machine called `dot'.) > I'd also like us to look into JimB's suggestion that the PC could be > pre-adjusted according to DECR_PC_AFTER_BREAK before anything in > execution control looks at it. The analysis for that would involve > looking at all the places where the PC is saved away, there are quite > a few of them. I'll make this a background task. > In any case, assuming testing doesn't report any regression, I'd like > to get this patch in soon so I can do my next phase of infrun > simplification - there are just three more gotos left in > handle_inferior_event, but so far I can't see how to evaporate them > validly without breaking the function into a number of smaller pieces. Okay. Let me know what you want me to do about testing the i960. Also, I'd like Michael Snyder's blessing for the breakpoint.c changes. > Also, it would be really great to add a testsuite case for temporary > breakpoints and stepping that will always fail without your changes, > and will pass otherwise, since it seems that there is currently no > test to detect the failure you're fixing. I'll construct such a test and have you take a look at it before committing it. Kevin -- Kevin Buettner kev@primenet.com, kevinb@cygnus.com >From shebs@cygnus.com Fri Sep 24 11:43:00 1999 From: Stan Shebs To: kevinb@cygnus.com Cc: gdb-patches@sourceware.cygnus.com, jimb@cygnus.com, msnyder@cygnus.com Subject: Re: RFA: breakpoint.c and infrun.c changes Date: Fri, 24 Sep 1999 11:43:00 -0000 Message-id: <199909241841.LAA03933@andros.cygnus.com> References: <990924173707.ZM19747@ocotillo.lan> X-SW-Source: 1999-q3/msg00269.html Content-length: 1628 Date: Fri, 24 Sep 1999 10:37:07 -0700 From: Kevin Buettner I've tested with the d10v simulator and saw no regressions. Great! I tried to test the i960, but the i960 simulator did not work well for me. I'll try against an actual board if you think it's important. (After looking at the nightly testing results for this target, I have doubts about being able to get it to work properly. The one recent test that did work properly had 1034 failures. But most of the recent and even not so recent tests show that the testsuite did not complete successfully.) I think we can safely skip over testing the i960 for this patch. I would like to know why the testing is having trouble, but that's not your problem... I've tested on the alpha (running linux), but I'm not sure what to make of the results. On the one hand, I do see *fewer* failures after I apply my patch. But I've run the test suite a number of times and get different results each time I run it. The good thing is that there aren't any new failures in the breakpoint or single step tests. (I ran my tests on the machine called `dot'.) Hmmm, when I ran tests on Alpha Linux a couple months ago I was getting pretty consistent results. Perhaps you could save a couple of the varying logs and point me at them? In any case, the results sound good, and the patch has been tested with a wider variety of targets than most patches get, so there's no reason not to check in now. If you get it in today, then it can be in Monday's snap for everybody to try out for themselves. Stan