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

[Simon Marchi <simon dot marchi at polymtl dot ca>]

On 2017-04-28 19:44, Doug Gilmore wrote:
> 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

Hi Doug,

The comment certainly helps, but in the commit log I'd like to see a 
more detailed list of events that leads to the crash.

Now that I look into it again, I think I understand.  The 
objfile_pspace_info::sections array/vector is a list of obj_section 
pointers (in C++ we'd probably use an std::vector<obj_section*>).  That 
list contains pointers to all the sections from all the objfiles sorted 
in order of increasing address.  They point directly to the sections 
allocated by the objfile in their obstacks (and accessible through 
objfile::sections).  So when the obstack is freed in reread_symbols, the 
sorted list contains stale pointers.  Is that it?

If that's what's happening, then I'm more convinced the fix is right.  
Is this behaviour caught by a test?  If not, could you write one?

> 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.

If you are talking about objfile_pspace_info, it's not per-objfile.  
There's one instance of it per program space, so it's actually 
"objfiles-related per-program-space data".  It contains the sorted list 
of all sections of all objfiles loaded in the pspace.

> +
> +	     Since we discarded our section information above, we have 
> dangling
> +	     pointers in the per-objfile program space data structure.  Force

again

> +	     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

Thanks,

Simon