From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Buettner To: Elena Zannoni Cc: gdb-patches@sourceware.cygnus.com Subject: Re: [PATCH RFA] DWARF v2.1 64-bit support Date: Fri, 04 Aug 2000 09:56:00 -0000 Message-id: <1000804165608.ZM4227@ocotillo.lan> References: <1000803222955.ZM2958@ocotillo.lan> <14730.54591.722920.958087@kwikemart.cygnus.com> <1000804163052.ZM4179@ocotillo.lan> X-SW-Source: 2000-08/msg00097.html On Aug 4, 9:30am, Kevin Buettner wrote: > - Kevin, Sept 4, 2000 ] ^^^^ Okay, so I really seem to have a problem with the month of August. (I also sent a recent message claiming that August was also July.) Anyway, thanks to Elena for pointing this out to me. It's fixed now. >From ezannoni@cygnus.com Fri Aug 04 10:28:00 2000 From: Elena Zannoni To: Kevin Buettner Cc: gdb-patches@sourceware.cygnus.com Subject: Re: [PATCH RFA]: Fix hash table bugs in minsyms.c Date: Fri, 04 Aug 2000 10:28:00 -0000 Message-id: <14730.64814.976208.581246@kwikemart.cygnus.com> References: <1000804003817.ZM3200@ocotillo.lan> X-SW-Source: 2000-08/msg00098.html Content-length: 5640 Kevin Buettner writes: > I request approval for committing the patch below. > Go ahead, it makes sense. Thanks Elena > I'm working on a project in which the OS wants to place certain groups > of sections at different addresses than specified by the symbolic > information in the executable. Thus it is necessary to call > objfile_relocate() (in objfiles.c). objfile_relocate() calls > msymbols_sort() which cause the minimal symbols to get sorted. > > The problem with this is that after the sort operation, the > minimal symbol hash table entries are no longer correct. In > fact, they are wildly incorrect. The reason for this is that > the sort routine does not account for the motion of the symbol > table entries in the hash table. > > In the process of diagnosing this problem, I took a close look > at how the hash table was being constructed. The minimal symbols > reside in an obstack and the hash table links are constructed > in compact_minimal_symbols(). This code, as written, contains > a minor bug, a fence post error in which the last symbol in > the obstack was *not* added to the hash table. > > But it contains a much more serious bug as well. Because the > symbols reside in an obstack, the obstack machinery is free > to relocate the space up until the time obstack_finish() is > called. Since the call to compact_minimal_symbols() (and hence > the hash table creation) occurs before obstack_finish(), it is > not safe to create references to the obstack entries since the > space could get moved and any reference created could wind up > being either incorrect or even dangling. > > In order to correct these problems, I've added a new function called > build_minimal_symbol_hash_tables() which is called after > obstack_finish() as well as after the sorting has been done in > msymbols_sort. > > I've tested this patch on GNU/linux/x86 with the compiler set to > generate DWARF2 info instead of the default (stabs). I've seen no > regressions. (And better still, it also fixes the problems that I ran > into on my project.) > > * minsyms.c (build_minimal_symbol_hash_tables): New function. > (compact_minimal_symbols): Don't construct hash tables here. > (install_minimal_symbols): Instead, construct them here. > (msymbols_sort): And rebuild them here too. > > Index: minsyms.c > =================================================================== > RCS file: /cvs/src/src/gdb/minsyms.c,v > retrieving revision 1.10 > diff -u -p -r1.10 minsyms.c > --- minsyms.c 2000/07/30 01:48:26 1.10 > +++ minsyms.c 2000/08/03 23:27:59 > @@ -801,11 +801,7 @@ compact_minimal_symbols (struct minimal_ > copyfrom++; > } > else > - { > - *copyto++ = *copyfrom++; > - > - add_minsym_to_hash_table (copyto - 1, objfile->msymbol_hash); > - } > + *copyto++ = *copyfrom++; > } > *copyto++ = *copyfrom++; > mcount = copyto - msymbol; > @@ -813,6 +809,38 @@ compact_minimal_symbols (struct minimal_ > return (mcount); > } > > +/* Build (or rebuild) the minimal symbol hash tables. This is necessary > + after compacting or sorting the table since the entries move around > + thus causing the internal minimal_symbol pointers to become jumbled. */ > + > +static void > +build_minimal_symbol_hash_tables (struct objfile *objfile) > +{ > + int i; > + struct minimal_symbol *msym; > + > + /* Clear the hash tables. */ > + for (i = 0; i < MINIMAL_SYMBOL_HASH_SIZE; i++) > + { > + objfile->msymbol_hash[i] = 0; > + objfile->msymbol_demangled_hash[i] = 0; > + } > + > + /* Now, (re)insert the actual entries. */ > + for (i = objfile->minimal_symbol_count, msym = objfile->msymbols; > + i > 0; > + i--, msym++) > + { > + msym->hash_next = 0; > + add_minsym_to_hash_table (msym, objfile->msymbol_hash); > + > + msym->demangled_hash_next = 0; > + if (SYMBOL_DEMANGLED_NAME (msym) != NULL) > + add_minsym_to_demangled_hash_table (msym, > + objfile->msymbol_demangled_hash); > + } > +} > + > /* Add the minimal symbols in the existing bunches to the objfile's official > minimal symbol table. In most cases there is no minimal symbol table yet > for this objfile, and the existing bunches are used to create one. Once > @@ -928,12 +956,13 @@ install_minimal_symbols (struct objfile > ones and attempting to cache their C++ demangled names. */ > > for (; mcount-- > 0; msymbols++) > - { > - SYMBOL_INIT_DEMANGLED_NAME (msymbols, &objfile->symbol_obstack); > - if (SYMBOL_DEMANGLED_NAME (msymbols) != NULL) > - add_minsym_to_demangled_hash_table (msymbols, > - objfile->msymbol_demangled_hash); > - } > + SYMBOL_INIT_DEMANGLED_NAME (msymbols, &objfile->symbol_obstack); > + > + /* Now build the hash tables; we can't do this incrementally > + at an earlier point since we weren't finished with the obstack > + yet. (And if the msymbol obstack gets moved, all the internal > + pointers to other msymbols need to be adjusted.) */ > + build_minimal_symbol_hash_tables (objfile); > } > } > > @@ -944,6 +973,7 @@ msymbols_sort (struct objfile *objfile) > { > qsort (objfile->msymbols, objfile->minimal_symbol_count, > sizeof (struct minimal_symbol), compare_minimal_symbols); > + build_minimal_symbol_hash_tables (objfile); > } > > /* Check if PC is in a shared library trampoline code stub. > >From eliz@delorie.com Fri Aug 04 11:10:00 2000 From: Eli Zaretskii To: kevinb@cygnus.com Cc: gdb-patches@sourceware.cygnus.com Subject: Re: [PATCH RFC] Protoize ch-exp.c, core-regset.c Date: Fri, 04 Aug 2000 11:10:00 -0000 Message-id: <200008041810.OAA28507@indy.delorie.com> References: <1000803190941.ZM2697@ocotillo.lan> <200008040931.FAA28059@indy.delorie.com> <1000804151730.ZM4041@ocotillo.lan> X-SW-Source: 2000-08/msg00099.html Content-length: 564 > Date: Fri, 4 Aug 2000 08:17:30 -0700 > From: Kevin Buettner > > Okay... here's what I'll do. I'll make a reasonable stab at moving > comments embedded in the parameter list to before the function along > with some minimal prefatory remarks. I ask that maintainers of the > affected files (and anyone else who's interested) look over these > changes to make sure that they aren't objectionable. I don't think anyone could expect more. I agree with this plan, and will try to review all the patches you post for this as closely as I can.