Re: [PATCH] gdb: Reinitialize objfile::section_offsets during objfile reload

[Tom Tromey <tom@tromey.com>]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> When building and testing with '-D_GLIBCXX_DEBUG=1' I noticed that the
Andrew> test gdb.base/reload.exp was failing.  This turns out to be because
Andrew> the objfile::section_offsets vector is not reinitialilzed during the
Andrew> objfile reload process, and in this particular test, GDB ends up
Andrew> indexing outside the bounds of the vector.

Thanks for catching this.

Andrew> One thing I did wonder about while looking at this fix is whether it
Andrew> would be possible to combine at least parts of syms_from_objfile_1
Andrew> with the core of reread_symbols.  I did have a go at doing this but
Andrew> gave up in the end due to the subtle differences between the two.
Andrew> Still, I think that with some more effort this might be possible, and
Andrew> this could be a nice clean up.

A long time ago, Jan had a patch along these lines.
I believe what his did was just throw away the logic in reread_symbols
in favor of simply creating a new objfile.  I wonder if it's too late to
do this now, since objfiles are exposed to Python.

Anyway, IMO, if there are subtle differences, they are probably bugs of
some sort; and unifying these code paths seems like clearly the right
thing to do.

Andrew> +	  /* In syms_from_objfile_1 after calling objfile_set_sym_fns we
Andrew> +	     handle the possibility that objfile->sf might be NULL, which
Andrew> +	     can happen for some obscure objfile formats.  We've never
Andrew> +	     handled the NULL case here before, but */

This looks like it got cut off.

Andrew> +	  /* Setup the section offsets structure for this objfile.  We use
Andrew> +	     zero section address information here, though it's not clear
Andrew> +	     this will always be correct.  If the user originally loaded
Andrew> +	     this objfile with non-zero address information then we're
Andrew> +	     going to loose that here.  */

s/loose/lose/

I don't know what else would make sense in this case.
Warn the user?

The patch seems otherwise reasonable to me.

Tom