From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Jacobowitz To: Elena Zannoni Cc: 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:36:00 -0000 Message-id: <20011105183016.A18471@nevyn.them.org> References: <3BE6DCE4.6550BBBD@cygnus.com> <3BE7105A.195AF167@cygnus.com> X-SW-Source: 2001-q4/msg00274.html On Mon, Nov 05, 2001 at 05:19:06PM -0500, Elena Zannoni wrote: > > > Daniel Jacobowitz wrote: > > > > The function search_symbols () does not seem to be used anywhere where > > the > > order of the returned functions matters, and the algorithm it searches > > with > > does not rely on any sorting. The same thing seems to be true for > > gdb_listfuncs, but I haven't investigated as thoroughly. > > > > Both of these functions have a blurb like: > > if (!BLOCK_SHOULD_SORT (b)) > > sort_block_syms (b); > > > > Daniel, I would prefer for the output of these functions to remain > sorted. > >From the gdb side, this produces sorted 'info function', 'info var', > 'info types' output, and from the gdbtk side, it builds the contents > of the function combobox at the bottom of the source window. This makes sense. It's not terribly well sorted to begin with, though, which is why I didn't sort them in the first version of this patch: ALL_SYMTABS (objfile, s) ... for (i = GLOBAL_BLOCK; i <= STATIC_BLOCK; i++) ... if (!BLOCK_SHOULD_SORT (b)) sort_block_syms (b); I.E. they're only locally sorted. I could be misinterpreting what is done with them though. > /* The symbols. If some of them are arguments, then they must be > in the order in which we would like to print them. */ > > and the comment for BLOCK_SHOULD_SORT: > > /* 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. */ > > > If this is something necessary, wouldn't the hashing break it? > I am not sure what would be screwed up based on the order of the > arguments, though. > 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). > This is what confuses me. I couldn't find where the symtab for a function block would be used in any order-sensitive way. Reading the code in the attached patch reminds me, though. Here's an example of what I think this is supposed to address: int foo (int a, int b) { return a / b; } becomes: .align 4 .stabs "foo:F(0,1)",36,0,0,foo .stabs "a:p(0,1)",160,0,0,8 .stabs "b:p(0,1)",160,0,0,12 .globl foo .type foo,@function foo: pushl %ebp movl %esp,%ebp .stabn 68,0,3,.LM1-foo .LM1: movl 8(%ebp),%eax .stabn 68,0,4,.LM2-foo .LM2: cltd idivl 12(%ebp) leave ret .Lfe1: .size foo,.Lfe1-foo .stabs "a:r(0,1)",64,0,0,0 We read in the symtab and see this (s->blockvector->block[3] is the block for function foo): (top-gdb) p *s->blockvector->block[3].sym[0] $16 = {ginfo = {name = 0x82328c8 "a", value = {ivalue = 8, block = 0x8, bytes = 0x8
, address = 8, chain = 0x8}, language_specific = { cplus_specific = {demangled_name = 0x0}, chill_specific = {demangled_name = 0x0}}, language = language_c, section = 0, bfd_section = 0x0}, type = 0x8232ae8, namespace = VAR_NAMESPACE, aclass = LOC_ARG, line = 0, aux_value = {basereg = 0}, aliases = 0x0, ranges = 0x0} (top-gdb) p *s->blockvector->block[3].sym[1] $17 = {ginfo = {name = 0x82328fc "b", value = {ivalue = 12, block = 0xc, bytes = 0xc
, address = 12, chain = 0xc}, language_specific = { cplus_specific = {demangled_name = 0x0}, chill_specific = {demangled_name = 0x0}}, language = language_c, section = 0, bfd_section = 0x0}, type = 0x8232ae8, namespace = VAR_NAMESPACE, aclass = LOC_ARG, line = 0, aux_value = {basereg = 0}, aliases = 0x0, ranges = 0x0} (top-gdb) p *s->blockvector->block[3].sym[2] $18 = {ginfo = {name = 0x8232930 "a", value = {ivalue = 0, block = 0x0, bytes = 0x0, address = 0, chain = 0x0}, language_specific = {cplus_specific = {demangled_name = 0x0}, chill_specific = { demangled_name = 0x0}}, language = language_c, section = 0, bfd_section = 0x0}, type = 0x8232ae8, namespace = VAR_NAMESPACE, aclass = LOC_REGISTER, line = 0, aux_value = { basereg = 0}, aliases = 0x0, ranges = 0x0} Note LOC_ARG and LOC_REGISTER in those. We want to look it up in the context of the function and find that it is an argument, not a local. ... aha. I see why the sort is OK now. This makes a lot more sense. The for loop is on the range [GLOBAL_BLOCK, STATIC_BLOCK] - i.e. it will never hit a function block. So it's only interested in the <= 40 check, which seems to be some sort of optimization. So I feel safe changing this code to not sort the blocks, and then sort the results afterwards, using compare_symbol. > > 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. I'd be willing to commit the one to gdbtk as obvious, esp. since no one commented on it; the code is essentially a copy of the one in symtab.c. > 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? Yup. So, OK if I sort the results afterwards? -- Daniel Jacobowitz Carnegie Mellon University MontaVista Software Debian GNU/Linux Developer