Re: [patch+7.4] reread.exp 7.3->7.4 regression

Jan Kratochvil
2011-12-19T19:12:37Z

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 <jan.kratochvil@redhat.com> 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 <jan.kratochvil@redhat.com>

* 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 <jan.kratochvil@redhat.com>

* gdb.base/reread.exp: If srcfile2 fails to build retry it with
-DNO_SECTIONS.
* gdb.base/reread2.c <!NO_SECTIONS>: 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