This is the mail archive of the insight@sources.redhat.com mailing list for the Insight project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [rfa] Remove a silly looking symtab sort.




> 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)
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]