This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS
Daniel Jacobowitz writes:
> 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.
>
Yes, thanks!!
Elena
> --
> Daniel Jacobowitz Carnegie Mellon University
> MontaVista Software Debian GNU/Linux Developer
>
> 2001-10-11 Daniel Jacobowitz <drow@mvista.com>
>
> * 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