From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3557 invoked by alias); 19 Dec 2011 19:12:37 -0000 Received: (qmail 3545 invoked by uid 22791); 19 Dec 2011 19:12:35 -0000 X-SWARE-Spam-Status: No, hits=-4.6 required=5.0 tests=AWL,BAYES_00,FAKE_REPLY_C,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_BJ,TW_CP X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 19 Dec 2011 19:12:22 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pBJJC7gO023907 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 19 Dec 2011 14:12:07 -0500 Received: from host2.jankratochvil.net (ovpn-116-60.ams2.redhat.com [10.36.116.60]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id pBJJC2uN009981 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Mon, 19 Dec 2011 14:12:04 -0500 Date: Mon, 19 Dec 2011 19:47:00 -0000 From: Jan Kratochvil To: Ulrich Weigand , Tom Tromey Cc: gdb-patches@sourceware.org, Joel Brobecker , Pedro Alves Subject: Re: [patch+7.4] reread.exp 7.3->7.4 regression Message-ID: <20111219191201.GA7401@host2.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201112191030.pBJAUsf4028428@d06av02.portsmouth.uk.ibm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-12/txt/msg00647.txt.bz2 On Mon, 19 Dec 2011 11:30:54 +0100, Ulrich Weigand wrote: > I can test on ARM if you want. I have tested it on fedora13beta. Just on Hardware : OMAP3 Beagle Board Revision : 0020 Processor : ARMv7 Processor rev 2 (v7l) BogoMIPS : 506.27 it takes 57m28.760s to build GDB (although with full symbols) and 51m13.889s to run the testsuite, is it normal to develop on ARM with this performance? > I really think this ought to be fixed in reread_symbols; I did not want to touch it anymore but I see it is currently the most feasible way how to fix the trunk. > freeing the old OBFD needs to be done *after* all the callbacks to cleanup > objfile data have completed. Done, it follow the free_objfile order now. > Your initial patch already moved the callbacks calls up a bit; My patch was using free_objfile, I do not understand which patch of mine do you refer to now. Re: [patch] Replace reread_symbols by load+free calls http://sourceware.org/ml/gdb-patches/2009-06/msg00696.html > In addition, we should probably call observer_notify_new_objfile so that new > tables can be built up for the re-read file ... I can post it afterwards, that is probably not for 7.4, I will have to run the ARM testsuite again. > > - if (data != NULL) > > + if (data != NULL && sec->the_bfd_section->index < data->section_count) [...] > ... and for checks in other users like this to be turned into assertions instead. On Mon, 19 Dec 2011 16:15:23 +0100, Tom Tromey wrote: > >>>>> "Jan" == Jan Kratochvil writes: > > Jan> - if (data != NULL) > Jan> + if (data != NULL && sec->the_bfd_section->index < data->section_count) > > I don't understand the need for this hunk. > When can this be called in a context where the BFD doesn't match the > data registered with the objfile? I did not know this part of code is not affected by the problem. No regressions on {x86_64,x86_64-m32,i686}-fedora16-linux-gnu and on arm-fedora13beta-linux-gnu. OK to check it in and for 7.4? Thanks, Jan gdb/ 2011-12-19 Jan Kratochvil * symfile.c (reread_symbols): Move free_objfile_separate_debug, preserve_values, sym_finish and clear_objfile_data calls before BFD close. Move free_objfile_separate_debug as the very first call. New comment on the ordering. gdb/testsuite/ 2011-12-19 Jan Kratochvil * gdb.base/reread.exp: If srcfile2 fails to build retry it with -DNO_SECTIONS. * gdb.base/reread2.c : New sections block. --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -2446,6 +2446,29 @@ reread_symbols (void) exec_file_attach (bfd_get_filename (objfile->obfd), 0); } + /* Keep the calls order approx. the same as in free_objfile. */ + + /* Free the separate debug objfiles. It will be + automatically recreated by sym_read. */ + free_objfile_separate_debug (objfile); + + /* Remove any references to this objfile in the global + value lists. */ + preserve_values (objfile); + + /* Nuke all the state that we will re-read. Much of the following + code which sets things to NULL really is necessary to tell + other parts of GDB that there is nothing currently there. + + Try to keep the freeing order compatible with free_objfile. */ + + if (objfile->sf != NULL) + { + (*objfile->sf->sym_finish) (objfile); + } + + clear_objfile_data (objfile); + /* Clean up any state BFD has sitting around. We don't need to close the descriptor but BFD lacks a way of closing the BFD without closing the descriptor. */ @@ -2471,27 +2494,6 @@ reread_symbols (void) memcpy (offsets, objfile->section_offsets, SIZEOF_N_SECTION_OFFSETS (num_offsets)); - /* Remove any references to this objfile in the global - value lists. */ - preserve_values (objfile); - - /* Nuke all the state that we will re-read. Much of the following - code which sets things to NULL really is necessary to tell - other parts of GDB that there is nothing currently there. - - Try to keep the freeing order compatible with free_objfile. */ - - if (objfile->sf != NULL) - { - (*objfile->sf->sym_finish) (objfile); - } - - clear_objfile_data (objfile); - - /* Free the separate debug objfiles. It will be - automatically recreated by sym_read. */ - free_objfile_separate_debug (objfile); - /* FIXME: Do we have to free a whole linked list, or is this enough? */ if (objfile->global_psymbols.list) --- a/gdb/testsuite/gdb.base/reread.exp +++ b/gdb/testsuite/gdb.base/reread.exp @@ -38,7 +38,8 @@ set testfile2 "reread2" set srcfile2 ${testfile2}.c set binfile2 ${objdir}/${subdir}/${testfile2}$EXEEXT -if { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable {debug nowarnings}] != "" } { +if { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable {debug nowarnings}] != "" + && [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable {debug nowarnings additional_flags=-DNO_SECTIONS}] != ""} { untested reread.exp return -1 } --- a/gdb/testsuite/gdb.base/reread2.c +++ b/gdb/testsuite/gdb.base/reread2.c @@ -15,3 +15,14 @@ int main() foo(); return 0; } + +/* Ensure the new file will have more sections. It may exploit code not + updating its SECTION_COUNT on reread_symbols. */ + +#ifndef NO_SECTIONS +# define VAR0(n) __attribute__ ((section ("sect" #n))) int var##n; +# define VAR1(n) VAR0 (n ## 0) VAR0(n ## 1) VAR0(n ## 2) VAR0(n ## 3) +# define VAR2(n) VAR1 (n ## 0) VAR1(n ## 1) VAR1(n ## 2) VAR1(n ## 3) +# define VAR3(n) VAR2 (n ## 0) VAR2(n ## 1) VAR2(n ## 2) VAR2(n ## 3) +VAR3 (0) +#endif