* RFA: Fix a big memory leak reading minimal symbols
@ 2003-01-26 19:13 Daniel Jacobowitz
2003-02-05 19:31 ` Elena Zannoni
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Jacobowitz @ 2003-01-26 19:13 UTC (permalink / raw)
To: gdb-patches
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.
- dbx_symfile_read contains a complete init_minimal_symbol_collection /
install_minimal_symbols pair, which generally finds no minimal symbols
in ELF.
- install_minimal_symbols and matching cleanup in elf_symfile_read.
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.
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.
I'd like a symtab maintainer to look over this patch; the more eyes the
better. I'm probably also short some copyright dates; I'll get them when/if
I check it in.
--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer
2002-01-26 Daniel Jacobowitz <drow@mvista.com>
* dbxread.c (elfstab_build_psymtabs): Don't call
install_minimal_symbols.
* elfread.c (elf_symfile_read): Call install_minimal_symbols
earlier.
* mdebugread.c (elfmdebug_build_psymtabs): Call
install_minimal_symbols and make appropriate cleanups.
Index: dbxread.c
===================================================================
RCS file: /big/fsf/rsync/src-cvs/src/gdb/dbxread.c,v
retrieving revision 1.40
diff -u -p -r1.40 dbxread.c
--- dbxread.c 18 Jan 2003 15:55:51 -0000 1.40
+++ dbxread.c 22 Jan 2003 23:19:29 -0000
@@ -3511,13 +3511,16 @@ elfstab_build_psymtabs (struct objfile *
buildsym_new_init ();
free_header_files ();
init_header_files ();
- install_minimal_symbols (objfile);
processing_acc_compilation = 1;
/* 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);
}
\f
Index: elfread.c
===================================================================
RCS file: /big/fsf/rsync/src-cvs/src/gdb/elfread.c,v
retrieving revision 1.29
diff -u -p -r1.29 elfread.c
--- elfread.c 14 Jan 2003 00:49:03 -0000 1.29
+++ elfread.c 22 Jan 2003 23:16:02 -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. */
@@ -612,13 +621,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: /big/fsf/rsync/src-cvs/src/gdb/mdebugread.c,v
retrieving revision 1.39
diff -u -p -r1.39 mdebugread.c
--- mdebugread.c 18 Jan 2003 15:55:52 -0000 1.39
+++ mdebugread.c 26 Jan 2003 17:01:39 -0000
@@ -4810,6 +4810,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,
@@ -4820,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);
}
\f
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: Fix a big memory leak reading minimal symbols
2003-01-26 19:13 RFA: Fix a big memory leak reading minimal symbols Daniel Jacobowitz
@ 2003-02-05 19:31 ` Elena Zannoni
2003-02-05 22:26 ` Daniel Jacobowitz
0 siblings, 1 reply; 5+ messages in thread
From: Elena Zannoni @ 2003-02-05 19:31 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
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.
>
> 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.
let's see all the symtabs builders:
coffstab_build_psymtabs -> calls dbx_symfile_read ok
elfstab_build_psymtabs -> you fixed ok
stabsect_build_psymtabs -> dubious
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?
elena
>
> I'd like a symtab maintainer to look over this patch; the more eyes the
> better. I'm probably also short some copyright dates; I'll get them when/if
> I check it in.
>
> --
> Daniel Jacobowitz
> MontaVista Software Debian GNU/Linux Developer
>
> 2002-01-26 Daniel Jacobowitz <drow@mvista.com>
>
> * dbxread.c (elfstab_build_psymtabs): Don't call
> install_minimal_symbols.
> * elfread.c (elf_symfile_read): Call install_minimal_symbols
> earlier.
> * mdebugread.c (elfmdebug_build_psymtabs): Call
> install_minimal_symbols and make appropriate cleanups.
>
> Index: dbxread.c
> ===================================================================
> RCS file: /big/fsf/rsync/src-cvs/src/gdb/dbxread.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 dbxread.c
> --- dbxread.c 18 Jan 2003 15:55:51 -0000 1.40
> +++ dbxread.c 22 Jan 2003 23:19:29 -0000
> @@ -3511,13 +3511,16 @@ elfstab_build_psymtabs (struct objfile *
> buildsym_new_init ();
> free_header_files ();
> init_header_files ();
> - install_minimal_symbols (objfile);
>
> processing_acc_compilation = 1;
>
> /* 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);
> }
> \f
> Index: elfread.c
> ===================================================================
> RCS file: /big/fsf/rsync/src-cvs/src/gdb/elfread.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 elfread.c
> --- elfread.c 14 Jan 2003 00:49:03 -0000 1.29
> +++ elfread.c 22 Jan 2003 23:16:02 -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. */
>
> @@ -612,13 +621,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: /big/fsf/rsync/src-cvs/src/gdb/mdebugread.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 mdebugread.c
> --- mdebugread.c 18 Jan 2003 15:55:52 -0000 1.39
> +++ mdebugread.c 26 Jan 2003 17:01:39 -0000
> @@ -4810,6 +4810,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,
> @@ -4820,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);
> }
> \f
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: Fix a big memory leak reading minimal symbols
2003-02-05 19:31 ` Elena Zannoni
@ 2003-02-05 22:26 ` Daniel Jacobowitz
2003-02-19 21:20 ` Elena Zannoni
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Jacobowitz @ 2003-02-05 22:26 UTC (permalink / raw)
To: Elena Zannoni; +Cc: gdb-patches
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);
}
\f
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: Fix a big memory leak reading minimal symbols
2003-02-05 22:26 ` Daniel Jacobowitz
@ 2003-02-19 21:20 ` Elena Zannoni
2003-02-20 19:32 ` Daniel Jacobowitz
0 siblings, 1 reply; 5+ messages in thread
From: Elena Zannoni @ 2003-02-19 21:20 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: Elena Zannoni, gdb-patches
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@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);
> }
> \f
>
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: Fix a big memory leak reading minimal symbols
2003-02-19 21:20 ` Elena Zannoni
@ 2003-02-20 19:32 ` Daniel Jacobowitz
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Jacobowitz @ 2003-02-20 19:32 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2003-02-20 19:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-01-26 19:13 RFA: Fix a big memory leak reading minimal symbols Daniel Jacobowitz
2003-02-05 19:31 ` Elena Zannoni
2003-02-05 22:26 ` Daniel Jacobowitz
2003-02-19 21:20 ` Elena Zannoni
2003-02-20 19:32 ` Daniel Jacobowitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox