From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Berlin To: Elena Zannoni Cc: , , , Subject: Re: [rfa] Remove a silly looking symtab sort. Date: Mon, 05 Nov 2001 15:40:00 -0000 Message-id: References: <3BE7105A.195AF167@cygnus.com> X-SW-Source: 2001-q4/msg00275.html > 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 > > > > * symtab.c (search_symbols): Do not attempt to sort a block > > if !BLOCK_SHOULD_SORT (b). > > > > 2001-10-25 Daniel Jacobowitz > > > > * 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) >