This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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

elena


 > -- 
 > Daniel Jacobowitz
 > MontaVista Software                         Debian GNU/Linux Developer
 > 
 > 2002-02-05  Daniel Jacobowitz  <drow at mvista dot com>
 > 
 > 	* coffread.c (coff_symfile_read): Clean up minimal symbols earlier.
 > 	* dbxread.c (elfstab_build_psymtabs): Don't call
 > 	install_minimal_symbols.
 > 	(stabsect_build_psymtabs): Likewise.
 > 	* elfread.c (elf_symfile_read): Call install_minimal_symbols
 > 	earlier.
 > 	* somread.c (som_symfile_read): Call install_minimal_symbols
 > 	and do_cleanups earlier.
 > 	* nlmread.c (nlm_symfile_read): Likewise.
 > 	* mdebugread.c (elfmdebug_build_psymtabs): Call
 > 	install_minimal_symbols and make appropriate cleanups.
 > 
 > Index: coffread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/coffread.c,v
 > retrieving revision 1.33
 > diff -u -p -r1.33 coffread.c
 > --- coffread.c	4 Feb 2003 18:07:00 -0000	1.33
 > +++ coffread.c	5 Feb 2003 22:22:13 -0000
 > @@ -515,7 +515,7 @@ coff_symfile_read (struct objfile *objfi
 >    unsigned int num_symbols;
 >    int symtab_offset;
 >    int stringtab_offset;
 > -  struct cleanup *back_to;
 > +  struct cleanup *back_to, *cleanup_minimal_symbols;
 >    int stabstrsize;
 >    int len;
 >    char * target;
 > @@ -595,7 +595,7 @@ coff_symfile_read (struct objfile *objfi
 >      error ("\"%s\": can't get string table", name);
 >  
 >    init_minimal_symbol_collection ();
 > -  make_cleanup_discard_minimal_symbols ();
 > +  cleanup_minimal_symbols = make_cleanup_discard_minimal_symbols ();
 >  
 >    /* Now that the executable file is positioned at symbol table,
 >       process it and define symbols accordingly.  */
 > @@ -615,6 +615,9 @@ coff_symfile_read (struct objfile *objfi
 >       minimal symbols for this objfile.  */
 >  
 >    install_minimal_symbols (objfile);
 > +
 > +  /* Free the installed minimal symbol data.  */
 > +  do_cleanups (cleanup_minimal_symbols);
 >  
 >    bfd_map_over_sections (abfd, coff_locate_sections, (void *) info);
 >  
 > Index: dbxread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/dbxread.c,v
 > retrieving revision 1.41
 > diff -u -p -r1.41 dbxread.c
 > --- dbxread.c	31 Jan 2003 19:22:17 -0000	1.41
 > +++ dbxread.c	5 Feb 2003 22:22:13 -0000
 > @@ -3555,7 +3555,6 @@ elfstab_build_psymtabs (struct objfile *
 >    buildsym_new_init ();
 >    free_header_files ();
 >    init_header_files ();
 > -  install_minimal_symbols (objfile);
 >  
 >    processing_acc_compilation = 1;
 >  
 > @@ -3567,7 +3566,10 @@ elfstab_build_psymtabs (struct objfile *
 >  
 >    /* In an elf file, we've already installed the minimal symbols that came
 >       from the elf (non-stab) symbol table, so always act like an
 > -     incremental load here. */
 > +     incremental load here.  dbx_symfile_read should not generate any new
 > +     minimal symbols, since we will have already read the ELF dynamic symbol
 > +     table and normal symbol entries won't be in the ".stab" section; but in
 > +     case it does, it will install them itself.  */
 >    dbx_symfile_read (objfile, 0);
 >  
 >    if (back_to)
 > @@ -3649,7 +3651,6 @@ stabsect_build_psymtabs (struct objfile 
 >    buildsym_new_init ();
 >    free_header_files ();
 >    init_header_files ();
 > -  install_minimal_symbols (objfile);
 >  
 >    /* Now, do an incremental load */
 >  
 > Index: elfread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/elfread.c,v
 > retrieving revision 1.30
 > diff -u -p -r1.30 elfread.c
 > --- elfread.c	31 Jan 2003 19:22:18 -0000	1.30
 > +++ elfread.c	5 Feb 2003 22:22:14 -0000
 > @@ -542,6 +542,15 @@ elf_symfile_read (struct objfile *objfil
 >  
 >    elf_symtab_read (objfile, 1);
 >  
 > +  /* Install any minimal symbols that have been collected as the current
 > +     minimal symbols for this objfile.  The debug readers below this point
 > +     should not generate new minimal symbols; if they do it's their
 > +     responsibility to install them.  "mdebug" appears to be the only one
 > +     which will do this.  */
 > +
 > +  install_minimal_symbols (objfile);
 > +  do_cleanups (back_to);
 > +
 >    /* Now process debugging information, which is contained in
 >       special ELF sections. */
 >  
 > @@ -611,13 +620,6 @@ elf_symfile_read (struct objfile *objfil
 >  
 >    if (DWARF2_BUILD_FRAME_INFO_P ())
 >      DWARF2_BUILD_FRAME_INFO(objfile);
 > -
 > -  /* Install any minimal symbols that have been collected as the current
 > -     minimal symbols for this objfile. */
 > -
 > -  install_minimal_symbols (objfile);
 > -
 > -  do_cleanups (back_to);
 >  }
 >  
 >  /* This cleans up the objfile's sym_stab_info pointer, and the chain of
 > Index: mdebugread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/mdebugread.c,v
 > retrieving revision 1.40
 > diff -u -p -r1.40 mdebugread.c
 > --- mdebugread.c	4 Feb 2003 18:07:01 -0000	1.40
 > +++ mdebugread.c	5 Feb 2003 22:22:15 -0000
 > @@ -4808,6 +4808,14 @@ elfmdebug_build_psymtabs (struct objfile
 >  {
 >    bfd *abfd = objfile->obfd;
 >    struct ecoff_debug_info *info;
 > +  struct cleanup *back_to;
 > +
 > +  /* FIXME: It's not clear whether we should be getting minimal symbol
 > +     information from .mdebug in an ELF file, or whether we will.
 > +     Re-initialize the minimal symbol reader in case we do.  */
 > +
 > +  init_minimal_symbol_collection ();
 > +  back_to = make_cleanup_discard_minimal_symbols ();
 >  
 >    info = ((struct ecoff_debug_info *)
 >  	  obstack_alloc (&objfile->psymbol_obstack,
 > @@ -4818,6 +4826,9 @@ elfmdebug_build_psymtabs (struct objfile
 >  	   bfd_errmsg (bfd_get_error ()));
 >  
 >    mdebug_build_psymtabs (objfile, swap, info);
 > +
 > +  install_minimal_symbols (objfile);
 > +  do_cleanups (back_to);
 >  }
 >  
 >  
 > Index: nlmread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/nlmread.c,v
 > retrieving revision 1.9
 > diff -u -p -r1.9 nlmread.c
 > --- nlmread.c	2 Dec 2001 22:38:23 -0000	1.9
 > +++ nlmread.c	5 Feb 2003 22:22:15 -0000
 > @@ -190,6 +190,12 @@ nlm_symfile_read (struct objfile *objfil
 >  
 >    nlm_symtab_read (abfd, offset, objfile);
 >  
 > +  /* Install any minimal symbols that have been collected as the current
 > +     minimal symbols for this objfile. */
 > +
 > +  install_minimal_symbols (objfile);
 > +  do_cleanups (back_to);
 > +
 >    stabsect_build_psymtabs (objfile, mainline, ".stab",
 >  			   ".stabstr", ".text");
 >  
 > @@ -204,13 +210,6 @@ nlm_symfile_read (struct objfile *objfil
 >  
 >    /* FIXME:  We could locate and read the optional native debugging format
 >       here and add the symbols to the minimal symbol table. */
 > -
 > -  /* Install any minimal symbols that have been collected as the current
 > -     minimal symbols for this objfile. */
 > -
 > -  install_minimal_symbols (objfile);
 > -
 > -  do_cleanups (back_to);
 >  }
 >  
 >  
 > Index: somread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/somread.c,v
 > retrieving revision 1.16
 > diff -u -p -r1.16 somread.c
 > --- somread.c	19 Jan 2003 04:06:46 -0000	1.16
 > +++ somread.c	5 Feb 2003 22:22:15 -0000
 > @@ -353,6 +353,14 @@ som_symfile_read (struct objfile *objfil
 >  
 >    som_symtab_read (abfd, objfile, objfile->section_offsets);
 >  
 > +  /* Install any minimal symbols that have been collected as the current
 > +     minimal symbols for this objfile. 
 > +     Further symbol-reading is done incrementally, file-by-file,
 > +     in a step known as "psymtab-to-symtab" expansion. hp-symtab-read.c
 > +     contains the code to do the actual DNTT scanning and symtab building. */
 > +  install_minimal_symbols (objfile);
 > +  do_cleanups (back_to);
 > +
 >    /* Now read information from the stabs debug sections.
 >       This is a no-op for SOM.
 >       Perhaps it is intended for some kind of mixed STABS/SOM
 > @@ -366,16 +374,8 @@ som_symfile_read (struct objfile *objfil
 >       together with a scan of the GNTT. See hp-psymtab-read.c. */
 >    hpread_build_psymtabs (objfile, mainline);
 >  
 > -  /* Install any minimal symbols that have been collected as the current
 > -     minimal symbols for this objfile. 
 > -     Further symbol-reading is done incrementally, file-by-file,
 > -     in a step known as "psymtab-to-symtab" expansion. hp-symtab-read.c
 > -     contains the code to do the actual DNTT scanning and symtab building. */
 > -  install_minimal_symbols (objfile);
 > -
 >    /* Force hppa-tdep.c to re-read the unwind descriptors.  */
 >    objfile->obj_private = NULL;
 > -  do_cleanups (back_to);
 >  }
 >  
 >  /* Initialize anything that needs initializing when a completely new symbol


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]