This is the mail archive of the
insight@sources.redhat.com
mailing list for the Insight project.
Re: [rfa] Remove a silly looking symtab sort.
- To: Elena Zannoni <ezannoni at cygnus dot com>
- Subject: Re: [rfa] Remove a silly looking symtab sort.
- From: Daniel Berlin <dan at cgsoftware dot com>
- Date: Mon, 5 Nov 2001 18:24:39 -0500 (EST)
- cc: <drow at mvista dot com>, <pes at regent dot e-technik dot tu-muenchen dot de>, <gdb-patches at sources dot redhat dot com>, <insight at sources dot redhat dot com>
> Daniel, I would prefer for the output of these functions to remain
> sorted.
> If this is something necessary, wouldn't the hashing break it?
No, we specifically test whether it is a function (and thus, the block is
it's arguments) when building the blocks, and don't hash the argument
blocks.
They remain in the normal order.
> I am not sure what would be screwed up based on the order of the
> arguments, though.
>
> Looking at our internal repository, I can see that this 'do not sort'
> was introduced by Peter Schauer in 1993:
>
> Index: symtab.h
> ===================================================================
> RCS file: /cvs/cvsfiles/devo/gdb/symtab.h,v
> retrieving revision 1.69
> retrieving revision 1.70
> diff -u -r1.69 -r1.70
> --- symtab.h 1993/06/29 16:55:29 1.69
> +++ symtab.h 1993/06/29 20:18:41 1.70
> @@ -385,9 +385,12 @@
> #define BLOCK_SUPERBLOCK(bl) (bl)->superblock
> #define BLOCK_GCC_COMPILED(bl) (bl)->gcc_compile_flag
>
> -/* Nonzero if symbols of block BL should be sorted alphabetically. */
> +/* Nonzero if symbols of block BL should be sorted alphabetically.
> + Don't sort a block which corresponds to a function. If we did the
> + sorting would have to preserve the order of the symbols for the
> + arguments. */
>
> -#define BLOCK_SHOULD_SORT(bl) ((bl)->nsyms >= 40)
> +#define BLOCK_SHOULD_SORT(bl) ((bl)->nsyms >= 40 && BLOCK_FUNCTION (bl)
> == NULL
> )
>
> Index: symfile.c
> ===================================================================
> RCS file: /cvs/cvsfiles/devo/gdb/symfile.c,v
> retrieving revision 1.82
> retrieving revision 1.83
> diff -u -p -r1.82 -r1.83
> --- symfile.c 1993/06/14 20:49:40 1.82
> +++ symfile.c 1993/06/29 20:18:35 1.83
> @@ -111,12 +111,7 @@ int symbol_reloading = 0;
> #endif
>
> ?
> -/* In the following sort, we always make sure that
> - register debug symbol declarations always come before regular
> - debug symbol declarations (as might happen when parameters are
> - then put into registers by the compiler).
> -
> - Since this function is called from within qsort, in an ANSI
> environment
> +/* Since this function is called from within qsort, in an ANSI
> environment
> it must conform to the prototype for qsort, which specifies that the
> comparison function takes two "void *" pointers. */
>
> @@ -126,17 +121,11 @@ compare_symbols (s1p, s2p)
> const PTR s2p;
> {
> register struct symbol **s1, **s2;
> - register int namediff;
>
> s1 = (struct symbol **) s1p;
> s2 = (struct symbol **) s2p;
> -
> - namediff = STRCMP (SYMBOL_NAME (*s1), SYMBOL_NAME (*s2));
> - if (namediff != 0) return namediff;
>
> - /* For symbols of the same name, registers should come first. */
> - return ((SYMBOL_CLASS (*s2) == LOC_REGISTER)
> - - (SYMBOL_CLASS (*s1) == LOC_REGISTER));
> + return (STRCMP (SYMBOL_NAME (*s1), SYMBOL_NAME (*s2)));
> }
>
> Index: symtab.c
> ===================================================================
> RCS file: /cvs/cvsfiles/devo/gdb/symtab.c,v
> retrieving revision 1.88
> retrieving revision 1.89
> diff -u -r1.88 -r1.89
> --- symtab.c 1993/06/25 03:47:17 1.88
> +++ symtab.c 1993/06/29 20:18:38 1.89
> @@ -839,8 +839,8 @@
> /* Now scan forward until we run out of symbols, find one whose
> name is
> greater than NAME, or find one we want. If there is more than
> one
> symbol with the right name and namespace, we return the first
> one.
> - dbxread.c is careful to make sure that if one is a register
> then it
> - comes first. */
> + Blocks containing argument symbols are no longer sorted so this
> + doesn't matter. */
>
> top = BLOCK_NSYMS (block);
> while (bot < top)
>
>
> > In that latter case, we certainly should not sort the symtab. I have
> > never
> > run across this in practice, but the symbol hashing patch wants this to
> > be
> > cleaned up first. Sorting a hash table's buckets is bad, mmkay?
> >
>
>
> I agree that sorting something that was hashed makes no sense, but we
> are
> one step before that still. I would like to figure out why that change
> by
> Peter was necessary. I think it has to do with the fact that stabs emits
> two symbols for each parameter (I don't have the stabs manual handy
> at the moment).
>
> > Is this patch OK to commit? If someone disagrees with my assessment on
> > the
> > need for the results of these functions to be sorted, I can add code to
> > sort
> > them after searching.
> >
>
> In theory it would be ok because the symbols shouldn't be sorted at
> this stage. So we should sort the results instead.
> If you can write something to sort the results we can get rid of these
> sorts. Well, the one in symtab.c, the gdbtk one is not for me to
> approve.
>
> But before we go ahead with the hashing I want to understand the need
> for
> that condition on sorting. Have you tried the hashing on stabs?
>
>
> Elena
>
>
> > --
> > Daniel Jacobowitz Carnegie Mellon University
> > MontaVista Software Debian GNU/Linux Developer
> >
> > 2001-10-25 Daniel Jacobowitz <drow@mvista.com>
> >
> > * symtab.c (search_symbols): Do not attempt to sort a block
> > if !BLOCK_SHOULD_SORT (b).
> >
> > 2001-10-25 Daniel Jacobowitz <drow@mvista.com>
> >
> > * generic/gdbtk-cmds (gdb_listfuncs): Do not attempt to sort a
> > block
> > if !BLOCK_SHOULD_SORT (b).
> >
> > Index: gdb/symtab.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/symtab.c,v
> > retrieving revision 1.44
> > diff -u -p -r1.44 symtab.c
> > --- gdb/symtab.c 2001/10/12 23:51:29 1.44
> > +++ gdb/symtab.c 2001/10/25 04:23:05
> > @@ -2475,9 +2475,6 @@ search_symbols (char *regexp, namespace_
> > for (i = GLOBAL_BLOCK; i <= STATIC_BLOCK; i++)
> > {
> > b = BLOCKVECTOR_BLOCK (bv, i);
> > - /* Skip the sort if this block is always sorted. */
> > - if (!BLOCK_SHOULD_SORT (b))
> > - sort_block_syms (b);
> > for (j = 0; j < BLOCK_NSYMS (b); j++)
> > {
> > QUIT;
> > Index: gdb/gdbtk/generic/gdbtk-cmds.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/gdbtk/generic/gdbtk-cmds.c,v
> > retrieving revision 1.40
> > diff -u -p -r1.40 gdbtk-cmds.c
> > --- gdb/gdbtk/generic/gdbtk-cmds.c 2001/10/12 23:51:29 1.40
> > +++ gdb/gdbtk/generic/gdbtk-cmds.c 2001/10/25 04:23:06
> > @@ -1495,9 +1495,6 @@ gdb_listfuncs (clientData, interp, objc,
> > for (i = GLOBAL_BLOCK; i <= STATIC_BLOCK; i++)
> > {
> > b = BLOCKVECTOR_BLOCK (bv, i);
> > - /* Skip the sort if this block is always sorted. */
> > - if (!BLOCK_SHOULD_SORT (b))
> > - sort_block_syms (b);
> > ALL_BLOCK_SYMBOLS (b, j, sym)
> > {
> > if (SYMBOL_CLASS (sym) == LOC_BLOCK)
>