From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Jacobowitz To: Elena Zannoni , gdb-patches@sources.redhat.com Subject: Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS Date: Thu, 11 Oct 2001 18:05:00 -0000 Message-id: <20011011210516.A24649@nevyn.them.org> References: <20011009123457.A28534@nevyn.them.org> <15302.12121.778853.77938@krustylu.cygnus.com> <20011011195450.A22256@nevyn.them.org> X-SW-Source: 2001-10/msg00160.html On Thu, Oct 11, 2001 at 07:54:50PM -0400, Daniel Jacobowitz wrote: > On Thu, Oct 11, 2001 at 07:46:33PM -0400, Elena Zannoni wrote: > > > > Daniel, > > Thanks so much for doing this. It makes it so much easier. > > > > Yes, I looked ths over and it seems to work, except that I would really > > prefer the change to printcmd.c split in two. The first bit to > > rationalize that "if (func)..." code. This would have with it all > > the indentation changes as well. The code as it is now doesn't really > > make much sense. So, that looks a good change to me. But it has nothing > > to do with the new macro. After that change is in, you can introduce > > the macro in printcmd.c w/o having all the indent changes. > > It also makes it easier to distinguish a no-op change (the macro) from > > the other one. > > OK. Would you prefer I resubmit this patch broken up further, then? > I could do that. Having reduced it to truly obvious, I'm going to commit the attached patch, unless someone objects strenuously in the near future. A diff ignoring whitespace shows that I am only moving a brace from before a for loop to after it; the for loop will never be executed unless the if is taken, anyway. This'll shrink the other patch quite a bit. -- Daniel Jacobowitz Carnegie Mellon University MontaVista Software Debian GNU/Linux Developer 2001-10-11 Daniel Jacobowitz * printcmd.c (print_frame_args): Move symbol iteration explicitly inside the func != NULL block. Index: printcmd.c =================================================================== RCS file: /cvs/src/src/gdb/printcmd.c,v retrieving revision 1.27 diff -u -p -r1.27 printcmd.c --- printcmd.c 2001/09/12 04:18:08 1.27 +++ printcmd.c 2001/10/12 00:59:44 @@ -1807,167 +1807,167 @@ print_frame_args (struct symbol *func, s { b = SYMBOL_BLOCK_VALUE (func); nsyms = BLOCK_NSYMS (b); - } - for (i = 0; i < nsyms; i++) - { - QUIT; - sym = BLOCK_SYM (b, i); - - /* Keep track of the highest stack argument offset seen, and - skip over any kinds of symbols we don't care about. */ - - switch (SYMBOL_CLASS (sym)) - { - case LOC_ARG: - case LOC_REF_ARG: - { - long current_offset = SYMBOL_VALUE (sym); - arg_size = TYPE_LENGTH (SYMBOL_TYPE (sym)); - - /* Compute address of next argument by adding the size of - this argument and rounding to an int boundary. */ - current_offset = - ((current_offset + arg_size + sizeof (int) - 1) - & ~(sizeof (int) - 1)); - - /* If this is the highest offset seen yet, set highest_offset. */ - if (highest_offset == -1 - || (current_offset > highest_offset)) - highest_offset = current_offset; - - /* Add the number of ints we're about to print to args_printed. */ - args_printed += (arg_size + sizeof (int) - 1) / sizeof (int); - } - - /* We care about types of symbols, but don't need to keep track of - stack offsets in them. */ - case LOC_REGPARM: - case LOC_REGPARM_ADDR: - case LOC_LOCAL_ARG: - case LOC_BASEREG_ARG: - break; - - /* Other types of symbols we just skip over. */ - default: - continue; - } + for (i = 0; i < nsyms; i++) + { + QUIT; + sym = BLOCK_SYM (b, i); + + /* Keep track of the highest stack argument offset seen, and + skip over any kinds of symbols we don't care about. */ - /* We have to look up the symbol because arguments can have - two entries (one a parameter, one a local) and the one we - want is the local, which lookup_symbol will find for us. - This includes gcc1 (not gcc2) on the sparc when passing a - small structure and gcc2 when the argument type is float - and it is passed as a double and converted to float by - the prologue (in the latter case the type of the LOC_ARG - symbol is double and the type of the LOC_LOCAL symbol is - float). */ - /* But if the parameter name is null, don't try it. - Null parameter names occur on the RS/6000, for traceback tables. - FIXME, should we even print them? */ - - if (*SYMBOL_NAME (sym)) - { - struct symbol *nsym; - nsym = lookup_symbol - (SYMBOL_NAME (sym), - b, VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL); - if (SYMBOL_CLASS (nsym) == LOC_REGISTER) + switch (SYMBOL_CLASS (sym)) { - /* There is a LOC_ARG/LOC_REGISTER pair. This means that - it was passed on the stack and loaded into a register, - or passed in a register and stored in a stack slot. - GDB 3.x used the LOC_ARG; GDB 4.0-4.11 used the LOC_REGISTER. - - Reasons for using the LOC_ARG: - (1) because find_saved_registers may be slow for remote - debugging, - (2) because registers are often re-used and stack slots - rarely (never?) are. Therefore using the stack slot is - much less likely to print garbage. - - Reasons why we might want to use the LOC_REGISTER: - (1) So that the backtrace prints the same value as - "print foo". I see no compelling reason why this needs - to be the case; having the backtrace print the value which - was passed in, and "print foo" print the value as modified - within the called function, makes perfect sense to me. - - Additional note: It might be nice if "info args" displayed - both values. - One more note: There is a case with sparc structure passing - where we need to use the LOC_REGISTER, but this is dealt with - by creating a single LOC_REGPARM in symbol reading. */ + case LOC_ARG: + case LOC_REF_ARG: + { + long current_offset = SYMBOL_VALUE (sym); + arg_size = TYPE_LENGTH (SYMBOL_TYPE (sym)); + + /* Compute address of next argument by adding the size of + this argument and rounding to an int boundary. */ + current_offset = + ((current_offset + arg_size + sizeof (int) - 1) + & ~(sizeof (int) - 1)); + + /* If this is the highest offset seen yet, set highest_offset. */ + if (highest_offset == -1 + || (current_offset > highest_offset)) + highest_offset = current_offset; + + /* Add the number of ints we're about to print to args_printed. */ + args_printed += (arg_size + sizeof (int) - 1) / sizeof (int); + } + + /* We care about types of symbols, but don't need to keep track of + stack offsets in them. */ + case LOC_REGPARM: + case LOC_REGPARM_ADDR: + case LOC_LOCAL_ARG: + case LOC_BASEREG_ARG: + break; + + /* Other types of symbols we just skip over. */ + default: + continue; + } + + /* We have to look up the symbol because arguments can have + two entries (one a parameter, one a local) and the one we + want is the local, which lookup_symbol will find for us. + This includes gcc1 (not gcc2) on the sparc when passing a + small structure and gcc2 when the argument type is float + and it is passed as a double and converted to float by + the prologue (in the latter case the type of the LOC_ARG + symbol is double and the type of the LOC_LOCAL symbol is + float). */ + /* But if the parameter name is null, don't try it. + Null parameter names occur on the RS/6000, for traceback tables. + FIXME, should we even print them? */ - /* Leave sym (the LOC_ARG) alone. */ - ; + if (*SYMBOL_NAME (sym)) + { + struct symbol *nsym; + nsym = lookup_symbol + (SYMBOL_NAME (sym), + b, VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL); + if (SYMBOL_CLASS (nsym) == LOC_REGISTER) + { + /* There is a LOC_ARG/LOC_REGISTER pair. This means that + it was passed on the stack and loaded into a register, + or passed in a register and stored in a stack slot. + GDB 3.x used the LOC_ARG; GDB 4.0-4.11 used the LOC_REGISTER. + + Reasons for using the LOC_ARG: + (1) because find_saved_registers may be slow for remote + debugging, + (2) because registers are often re-used and stack slots + rarely (never?) are. Therefore using the stack slot is + much less likely to print garbage. + + Reasons why we might want to use the LOC_REGISTER: + (1) So that the backtrace prints the same value as + "print foo". I see no compelling reason why this needs + to be the case; having the backtrace print the value which + was passed in, and "print foo" print the value as modified + within the called function, makes perfect sense to me. + + Additional note: It might be nice if "info args" displayed + both values. + One more note: There is a case with sparc structure passing + where we need to use the LOC_REGISTER, but this is dealt with + by creating a single LOC_REGPARM in symbol reading. */ + + /* Leave sym (the LOC_ARG) alone. */ + ; + } + else + sym = nsym; } - else - sym = nsym; - } #ifdef UI_OUT - /* Print the current arg. */ - if (!first) - ui_out_text (uiout, ", "); - ui_out_wrap_hint (uiout, " "); - - annotate_arg_begin (); - - list_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL); - fprintf_symbol_filtered (stb->stream, SYMBOL_SOURCE_NAME (sym), - SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI); - ui_out_field_stream (uiout, "name", stb); - annotate_arg_name_end (); - ui_out_text (uiout, "="); + /* Print the current arg. */ + if (!first) + ui_out_text (uiout, ", "); + ui_out_wrap_hint (uiout, " "); + + annotate_arg_begin (); + + list_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL); + fprintf_symbol_filtered (stb->stream, SYMBOL_SOURCE_NAME (sym), + SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI); + ui_out_field_stream (uiout, "name", stb); + annotate_arg_name_end (); + ui_out_text (uiout, "="); #else - /* Print the current arg. */ - if (!first) - fprintf_filtered (stream, ", "); - wrap_here (" "); - - annotate_arg_begin (); - - fprintf_symbol_filtered (stream, SYMBOL_SOURCE_NAME (sym), - SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI); - annotate_arg_name_end (); - fputs_filtered ("=", stream); + /* Print the current arg. */ + if (!first) + fprintf_filtered (stream, ", "); + wrap_here (" "); + + annotate_arg_begin (); + + fprintf_symbol_filtered (stream, SYMBOL_SOURCE_NAME (sym), + SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI); + annotate_arg_name_end (); + fputs_filtered ("=", stream); #endif - /* Avoid value_print because it will deref ref parameters. We just - want to print their addresses. Print ??? for args whose address - we do not know. We pass 2 as "recurse" to val_print because our - standard indentation here is 4 spaces, and val_print indents - 2 for each recurse. */ - val = read_var_value (sym, fi); + /* Avoid value_print because it will deref ref parameters. We just + want to print their addresses. Print ??? for args whose address + we do not know. We pass 2 as "recurse" to val_print because our + standard indentation here is 4 spaces, and val_print indents + 2 for each recurse. */ + val = read_var_value (sym, fi); - annotate_arg_value (val == NULL ? NULL : VALUE_TYPE (val)); + annotate_arg_value (val == NULL ? NULL : VALUE_TYPE (val)); - if (val) - { + if (val) + { #ifdef UI_OUT - val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0, - VALUE_ADDRESS (val), - stb->stream, 0, 0, 2, Val_no_prettyprint); - ui_out_field_stream (uiout, "value", stb); - } - else - ui_out_text (uiout, "???"); + val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0, + VALUE_ADDRESS (val), + stb->stream, 0, 0, 2, Val_no_prettyprint); + ui_out_field_stream (uiout, "value", stb); + } + else + ui_out_text (uiout, "???"); - /* Invoke ui_out_tuple_end. */ - do_cleanups (list_chain); + /* Invoke ui_out_tuple_end. */ + do_cleanups (list_chain); #else - val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0, - VALUE_ADDRESS (val), - stream, 0, 0, 2, Val_no_prettyprint); - } - else - fputs_filtered ("???", stream); + val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0, + VALUE_ADDRESS (val), + stream, 0, 0, 2, Val_no_prettyprint); + } + else + fputs_filtered ("???", stream); #endif - annotate_arg_end (); + annotate_arg_end (); - first = 0; + first = 0; + } } /* Don't print nameless args in situations where we don't know