Re: [PATCH] Fix PR 21337 v2: segfault when re-reading symbols with remote debugging.

[Doug Gilmore <Doug dot Gilmore at imgtec dot com>]

> ...
> Hi Doug,
> 
> I've ran this in my head many times, but I still don't understand which field exactly is stale and causes the segfault.
> 
> According to the backtrace, the line of the crash is:
> 
>   if (pc < obj_section_addr (section))
> 
> That macro expands to
> 
> #define obj_section_addr(s)                              \
>   (bfd_get_section_vma ((s)->objfile->obfd, s->the_bfd_section)        \
>    + obj_section_offset (s))
> 
> which further expands to
> 
> #define obj_section_offset(s)                        \
>   (((s)->objfile->section_offsets)->offsets[gdb_bfd_section_index ((s)->objfile->obfd, (s)->the_bfd_section)])
> 
> 
> Could you point out which dereferencing operator/memory access causes the crash?  At first I thought it would be ->section_offsets, but that field is set properly before calling read_symbols:
> 
>       /* We use the same section offsets as from last time.  I'm not
>          sure whether that is always correct for shared libraries.  */
>       objfile->section_offsets = (struct section_offsets *)
>         obstack_alloc (&objfile->objfile_obstack,
>                SIZEOF_N_SECTION_OFFSETS (num_offsets));
>       memcpy (objfile->section_offsets, offsets,
>           SIZEOF_N_SECTION_OFFSETS (num_offsets));
>       objfile->num_sections = num_offsets;
> 
> so it should not be the culprit...  The s variable itself should point to an instance of obj_section, contained in the objfile_pspace_info::sections array.  This one is allocated with XNEWVEC, so it shouldn't be affected by the fact that we clear the obstack.
> 
> The objfile object itself doesn't change address, so the pointers to it should still be valid...
> 
> I find the obj_section_addr and obj_section_offset very bad for readability and debuggability.  Could you change them for inline functions that are not one liners?  Then it will be obvious in the backtrace which operation causes the crash.
> 
> Thanks,
> 
> Simon
Hi Simon,

After thinking about it my comment and code placement wasn't
particularly good.  Something along the line's of Luis's change
is better.

Does Luis's comment address the question you have?

If so, Luis: Should is it OK we incorporate your changes in the patch?

I attached a diff for the change.

Thanks,

Doug


diff --git a/gdb/symfile.c b/gdb/symfile.c
index 846aabe..aff4341 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2628,6 +2628,20 @@ reread_symbols (void)
 	  clear_complaints (&symfile_complaints, 1, 1);
 
 	  objfile->flags &= ~OBJF_PSYMTABS_READ;
+
+	  /* We are about to read new symbols and potentially also DWARF
+	     information.  Some targets may want to pass addresses read from
+	     DWARF DIE's through an adjustment function before saving them, like
+	     MIPS, which may call into "find_pc_section".  When called, that
+	     function will make use of per-objfile program space data.
+
+	     Since we discarded our section information above, we have dangling
+	     pointers in the per-objfile program space data structure.  Force
+	     GDB to update the section mapping information by letting it know
+	     the objfile has changed, making the dangling pointers point to
+	     correct data again.  */
+	  objfiles_changed ();
+
 	  read_symbols (objfile, 0);
 
 	  if (!objfile_has_symbols (objfile))
@@ -2660,9 +2674,6 @@ reread_symbols (void)
 
   if (!new_objfiles.empty ())
     {
-      /* Notify objfiles that we've modified objfile sections.  */
-      objfiles_changed ();
-
       clear_symtab_users (0);
 
       /* clear_objfile_data for each objfile was called before freeing it and
-- 
1.9.1