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


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.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2002-02-05  Daniel Jacobowitz  <drow@mvista.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]