From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25448 invoked by alias); 5 Feb 2003 19:31:17 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 25441 invoked from network); 5 Feb 2003 19:31:17 -0000 Received: from unknown (HELO mx1.redhat.com) (172.16.49.200) by 172.16.49.205 with SMTP; 5 Feb 2003 19:31:17 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id h15JVHf20151 for ; Wed, 5 Feb 2003 14:31:17 -0500 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h15JVGa23642 for ; Wed, 5 Feb 2003 14:31:16 -0500 Received: from localhost.redhat.com (romulus-int.sfbay.redhat.com [172.16.27.46]) by pobox.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h15JVGY01811 for ; Wed, 5 Feb 2003 14:31:16 -0500 Received: by localhost.redhat.com (Postfix, from userid 469) id 7C486FF79; Wed, 5 Feb 2003 14:35:27 -0500 (EST) From: Elena Zannoni MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <15937.26495.326748.558348@localhost.redhat.com> Date: Wed, 05 Feb 2003 19:31:00 -0000 To: Daniel Jacobowitz Cc: gdb-patches@sources.redhat.com Subject: Re: RFA: Fix a big memory leak reading minimal symbols In-Reply-To: <20030126191411.GA1824@nevyn.them.org> References: <20030126191411.GA1824@nevyn.them.org> X-SW-Source: 2003-02/txt/msg00196.txt.bz2 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 > > * 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); > } > > 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); > } > >