Re: RFA: Fix a big memory leak reading minimal symbols

[Daniel Jacobowitz <drow@mvista.com>]

On Wed, Feb 19, 2003 at 04:24:39PM -0500, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
>  > On Wed, Feb 05, 2003 at 02:35:27PM -0500, Elena Zannoni wrote:
>  > > Daniel Jacobowitz writes:
>  > >  > This patch is good for a whopping 13% (7MB) of memory usage when reading in
>  > >  > all of mozilla's minimal symbols (stabs).  The control flow looks like this
>  > >  > right now:
>  > >  >   - init_minimal_symbol_collection in elf_symfile_read
>  > >  >   - install_minimal_symbols in elfstab_build_psymtabs.  Presumably added so
>  > >  >     that we don't lose the minimal symbols above.
>  > > 
>  > > right, elfstab_build_psymtab is called by elf_symfile_read, after
>  > > init_minimal_symbol_collection.
>  > > 
>  > >  >   - dbx_symfile_read contains a complete init_minimal_symbol_collection /
>  > >  >     install_minimal_symbols pair, which generally finds no minimal symbols
>  > >  >     in ELF.
>  > > 
>  > > elfstab_build_psymtab calls dbx_symfile_read.
>  > > 
>  > >  >   - install_minimal_symbols and matching cleanup in elf_symfile_read.
>  > >  > 
>  > > 
>  > > at the end.
>  > > 
>  > >  > Only problem is, that second init_minimal_symbol_collection call zeroed out
>  > >  > the bunch pointer.  We've installed them but we never free them, so they
>  > >  > just sit around wasting memory.
>  > > 
>  > > OK, I see the problem in this sequence.  There is another 'stray' call
>  > > to install_minimal_symbols, in stabsect_build_psymtab, and this
>  > > function also calls dbx_symfile_read. The sequence is the same. So the
>  > > problem should occur there as well.
>  > > stabsect_build_psymtab is called by som_symfile_read.
>  > 
>  > And by nlm_symfile_read too.
>  > 
>  > >  > This patch arranges for all calls to init_minimal_symbol_collection in ELF
>  > >  > targets to be paired with matching cleanups and install_minimal_symbols
>  > >  > calls.  This means that elfmdebug_build_psymtabs gets its own pair instead
>  > >  > of relying on elf_symfile_read, and that elf_symfile_read installs its own
>  > >  > minimal symbols much earlier.
>  > > 
>  > > How does elfstab_build_psymtabs get invoked from mdebug? Oh, it
>  > > doesn't.  It gets invoked after the mdebug stuff is over.
>  > > 
>  > > I see, you are adding pairs of install+cleanup to the mdebug psymtab
>  > > builder, i.e. each piece is self-sufficient. For the stabs psymtab
>  > > builder the appropriate steps are done in dbx_symfile_read already.
>  > 
>  > Right.  Each reader that wants to do this, should do it itself, since
>  > they don't nest.
>  > 
>  > > let's see all the symtabs builders:
>  > > 
>  > > coffstab_build_psymtabs -> calls dbx_symfile_read ok
>  > 
>  > Ugh.  This is OK but its caller isn't; coff_symtab_read has some
>  > minimal symbols outstanding when it calls coffstab_build_psymtabs. 
>  > Fixed, but I don't have any tests for COFF set up.
>  > 
>  > > elfstab_build_psymtabs -> you fixed ok
>  > > stabsect_build_psymtabs -> dubious
>  > 
>  > Fixed, in both callers.
>  > 
>  > > dwarf2_build_psymtabs -> no need
>  > > dwarf_build_psymtabs -> don't care
>  > > hpread_build_psymtabs -> no clue, messy. I think it does it directly.
>  > > mdebug_build_psymtabs -> you fixed ok
>  > > 
>  > > if the table above makes sense, then can you verify/fix the dubious case?
>  > 
>  > How's this look?  New changes in coff_symfile_read, nlm_symfile_read,
>  > som_symfile_read, and stabsect_build_psymtabs.
>  > 
> 
> Okey

Thanks, I've checked it in!  One more off the list.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer