From: Daniel Berlin <dan@cgsoftware.com>
To: Elena Zannoni <ezannoni@cygnus.com>
Cc: <drow@mvista.com>, <pes@regent.e-technik.tu-muenchen.de>,
<gdb-patches@sources.redhat.com>, <insight@sources.redhat.com>
Subject: Re: [rfa] Remove a silly looking symtab sort.
Date: Mon, 05 Nov 2001 15:40:00 -0000 [thread overview]
Message-ID: <Pine.LNX.4.33.0111051823060.31401-100000@www.cgsoftware.com> (raw)
In-Reply-To: <3BE7105A.195AF167@cygnus.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)
>
next prev parent reply other threads:[~2001-11-05 15:40 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <3BE6DCE4.6550BBBD@cygnus.com>
2001-11-05 15:04 ` Elena Zannoni
2001-11-05 15:36 ` Daniel Jacobowitz
2001-11-05 15:40 ` Daniel Berlin [this message]
2001-11-07 17:58 Elena Zannoni
2001-11-07 18:19 ` Daniel Berlin
-- strict thread matches above, loose matches on Subject: below --
2001-11-07 17:56 Elena Zannoni
2001-10-24 21:29 Daniel Jacobowitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.33.0111051823060.31401-100000@www.cgsoftware.com \
--to=dan@cgsoftware.com \
--cc=drow@mvista.com \
--cc=ezannoni@cygnus.com \
--cc=gdb-patches@sources.redhat.com \
--cc=insight@sources.redhat.com \
--cc=pes@regent.e-technik.tu-muenchen.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).