Re: [PATCH] Fix gdb.multi/base.exp, gdb memory corruption

[Yao Qi <yao at codesourcery dot com>]

On 05/20/2014 04:39 AM, Doug Evans wrote:
> The problem originates from the get_current_arch call in
> py-progspace.c:py_free_pspace.

The backtrace below can make the problem clear, the per-pspace data
is used (frame #0) when it is being released (frame #20).

#0  get_objfile_pspace_data (pspace=0x86e5ad0) at ../../../git/gdb/objfiles.c:102
#1  0x08239496 in find_pc_section (pc=134513888) at ../../../git/gdb/objfiles.c:1348                      
#2  0x0823c255 in lookup_minimal_symbol_by_pc_section (pc=134513888, section=0x0) at ../../../git/gdb/minsyms.c:734
#3  0x081c1dea in find_pc_sect_symtab (pc=pc@entry=134513888, section=section@entry=0x0) at ../../../git/gdb/symtab.c:2153
.....
#17 0x082ed6db in get_current_frame () at ../../../git/gdb/frame.c:1485
#18 0x082ed7fc in get_selected_frame (message=0x0) at ../../../git/gdb/frame.c:1540
#19 0x08212d92 in get_current_arch () at ../../../git/gdb/arch-utils.c:758
#20 0x0813d788 in py_free_pspace (pspace=0x86e5ad0, datum=0xb7bf9ad0) at ../../../git/gdb/python/py-progspace.c:244
#21 0x08313d4a in program_spaceregistry_callback_adaptor (func=0x813d777 <py_free_pspace>, container=0x86e5ad0, data=0xb7bf9ad0)
    at ../../../git/gdb/progspace.c:45
#22 0x08318d6b in registry_clear_data (data_registry=0x85c425c <program_space_data_registry>, adaptor=0x8313d1d <program_spaceregistry_callback_adaptor>, 
    container=0x86e5ad0, fields=0x86e5b10) at ../../../git/gdb/registry.c:82
#23 0x08318e16 in registry_container_free_data (data_registry=0x85c425c <program_space_data_registry>, 
    adaptor=0x8313d1d <program_spaceregistry_callback_adaptor>, container=0x86e5ad0, fields=0x86e5b10) at ../../../git/gdb/registry.c:95
#24 0x08313db0 in program_space_free_data (container=0x86e5ad0) at ../../../git/gdb/progspace.c:45
#25 0x0831412f in release_program_space (pspace=0x86e5ad0) at ../../../git/gdb/progspace.c:167
#26 0x083142cd in prune_program_spaces () at ../../../git/gdb/progspace.c:269

> I think the comment in this patch explains things pretty well.
> Basically, we're in the pspace destructor, and thus there's not much we can
> rely on.  The Python machinery needs an arch, so we give it one,
> albeit a fiction.  I think(!) it doesn't matter.
> I'm going with this fix because it's not clear to me what The Right fix
> is, short of removing global state in gdb which is non-trivial.
> Since "there is always an inferior" calling target_gdbarch seems
> pretty safe here.

The patch looks reasonable to me.  Although I tried to do something
different at first, remove pspace earlier, say, when delete the inferior,
delete the pspace too if it isn't shared with other inferiors.  This
crash isn't fixed unfortunately.

> 
> 2014-05-19  Doug Evans  <dje@google.com>
> 
> 	* python/py-progspace.c (py_free_pspace): Use target_gdbarch instead
> 	of get_current_arch.
> 
> diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
> index cda5a86..c787c69 100644
> --- a/gdb/python/py-progspace.c
> +++ b/gdb/python/py-progspace.c
> @@ -241,7 +241,16 @@ py_free_pspace (struct program_space *pspace, void *datum)
>  {
>    struct cleanup *cleanup;
>    pspace_object *object = datum;
> -  struct gdbarch *arch = get_current_arch ();
> +  /* This is a fiction, but we're in a nasty spot: The pspace is in the
> +     process of being deleted, we can't rely on anything in it.  Plus
> +     this is one time when the current program space and current inferior
> +     are not in sync.  The architecture comes from the inferior, which cannot
> +     be the current one because we wouldn't be deleting its pspace.
> +     We don't need to do much here so this fiction suffices.
> +     Note: We cannot call get_current_arch because it may try to access
> +     the target, which may involve accessing data in the pspace currently
> +     being deleted.  */

There are five sentences in this paragraph, but only the first and the last
are necessary, IMO.  The rest doesn't bring any useful information, if
not confusion, into this comment.

> +  struct gdbarch *arch = target_gdbarch ();
>  
>    cleanup = ensure_python_env (arch, current_language);
>    object->pspace = NULL;


-- 
Yao (éå)