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

[Andrew Burgess <andrew.burgess@embecosm.com>]

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

In order to make this issue show up even if GDB is not compiled with
'-D_GLIBCXX_DEBUG=1' I added an assert into elfread.c, then I made
some changes in symfile.c:reread_symbols to reinitialilze the vector,
after which the problem is resolved.

While looking at the code in reread_symbols I noticed a difference
between reread_symbols and syms_from_objfile_1 with how we handle the
case where objfile->sf is NULL.  In the later function we handle this
case, while in the former we just assume that objfile->sf is not
NULL.  I can't see why the NULL case couldn't occur in reread_symbols,
however, this would only happen if the user is using srec, ihex, or
texhex format BFDs.  As such I didn't fancy trying to update
reread_symbols to handle this case, so instead I just added an error
if this case should arise.  This should be better than the current
undefined behaviour (crash), and should let us know what has gone
wrong if a user ever reports this as an issue.

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

gdb/ChangeLog:

	* elfread.c (record_minimal_symbol): Add an assertion.
	* symfile.c (reread_symbols): Clear the section_offsets vector,
	and reinitialize it during reload.  Add an error if objfile->sf is
	NULL.

Change-Id: I9d62292641416483a8a7415c7504095bf439fc29
---
 gdb/ChangeLog |  7 +++++++
 gdb/elfread.c |  4 ++++
 gdb/symfile.c | 19 +++++++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 453bca527e9..5d0acede078 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -218,6 +218,10 @@ record_minimal_symbol (minimal_symbol_reader &reader,
   if ((bfd_section_flags (bfd_section) & SEC_ALLOC) == SEC_ALLOC)
     section_index = gdb_bfd_section_index (objfile->obfd, bfd_section);
 
+  /* The section_offsets vector should have been initialised by now, and
+     there should be one entry for each section in objfile.  */
+  gdb_assert (section_index < objfile->section_offsets.size ());
+
   struct minimal_symbol *result
     = reader.record_full (name, copy_name, address, ms_type, section_index);
   if ((objfile->flags & OBJF_MAINLINE) == 0
diff --git a/gdb/symfile.c b/gdb/symfile.c
index f7bada75f35..0da879fd751 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2553,6 +2553,7 @@ reread_symbols (void)
 	  objfile->compunit_symtabs = NULL;
 	  objfile->template_symbols = NULL;
 	  objfile->static_links.reset (nullptr);
+	  objfile->section_offsets.clear ();
 
 	  /* obstack_init also initializes the obstack so it is
 	     empty.  We could use obstack_specify_allocation but
@@ -2573,6 +2574,16 @@ reread_symbols (void)
 	     start over.  PR symtab/15885  */
 	  objfile_set_sym_fns (objfile, find_sym_fns (objfile->obfd));
 
+	  /* In syms_from_objfile_1 after calling objfile_set_sym_fns we
+	     handle the possibility that objfile->sf might be NULL, which
+	     can happen for some obscure objfile formats.  We've never
+	     handled the NULL case here before, but */
+	  if (objfile->sf == nullptr)
+	    error (_("unable to reload object file with format `%s'"),
+		   bfd_get_target (objfile->obfd));
+
+	  gdb_assert (objfile->sf != nullptr);
+
 	  build_objfile_section_table (objfile);
 
 	  /* What the hell is sym_new_init for, anyway?  The concept of
@@ -2604,6 +2615,14 @@ reread_symbols (void)
 
 	  objfiles_changed ();
 
+	  /* Setup the section offsets structure for this objfile.  We use
+	     zero section address information here, though it's not clear
+	     this will always be correct.  If the user originally loaded
+	     this objfile with non-zero address information then we're
+	     going to loose that here.  */
+	  section_addr_info local_addr;
+	  (*objfile->sf->sym_offsets) (objfile, local_addr);
+
 	  read_symbols (objfile, 0);
 
 	  if (!objfile_has_symbols (objfile))
-- 
2.14.5