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

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

[Doug Evans]

Hi.

My prune_inferiors patch

2014-05-17  Doug Evans  <xdje42@gmail.com>

	* inferior.c (prune_inferiors): Fix comment.
	(remove_inferior_command): Call prune_program_spaces.

is causing gdb.multi/base.exp to fail:

UNRESOLVED: gdb.multi/base.exp: remove-inferiors 2-3
UNRESOLVED: gdb.multi/base.exp: check remove-inferiors

gdb is crashing because it's accessing/freeing already freed memory.
I was running the testsuite all weekend, sorry I only saw this now.

E.g.
==16368== Invalid read of size 4
==16368==    at 0x660A9D: find_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:1349)
==16368==    by 0x663ECB: lookup_minimal_symbol_by_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/minsyms.c:734)
==16368==    by 0x5D987A: find_pc_sect_symtab (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/symtab.c:2153)
==16368==    by 0x5D4D77: blockvector_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:168)
==16368==    by 0x5D4F59: block_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:246)
==16368==    by 0x5D4F9B: block_for_pc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:258)
==16368==    by 0x734C5D: inline_frame_sniffer (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inline-frame.c:218)
==16368==    by 0x732104: frame_unwind_try_unwinder (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:108)
==16368==    by 0x73223F: frame_unwind_find_by_frame (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:159)
==16368==    by 0x72D5AA: compute_frame_id (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:453)
==16368==    by 0x7300EC: get_prev_frame_if_no_cycle (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1758)
==16368==    by 0x73079A: get_prev_frame_always (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1931)
==16368==  Address 0x5b13500 is 16 bytes inside a block of size 24 free'd
==16368==    at 0x403072E: free (valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c:445)
==16368==    by 0x762134: xfree (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/common/common-utils.c:108)
==16368==    by 0x65DACF: objfiles_pspace_data_cleanup (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:91)
==16368==    by 0x75E546: program_spaceregistry_callback_adaptor (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45)
==16368==    by 0x7644F6: registry_clear_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:82)
==16368==    by 0x7645AB: registry_container_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:95)
==16368==    by 0x75E5B4: program_space_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45)
==16368==    by 0x75E9BA: release_program_space (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:167)
==16368==    by 0x75EB9B: prune_program_spaces (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:269)
==16368==    by 0x75303D: remove_inferior_command (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inferior.c:792)
==16368==    by 0x50B5FD: do_cfunc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:107)
==16368==    by 0x50E6F2: cmd_func (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:1886)

The problem originates from the get_current_arch call in
py-progspace.c:py_free_pspace.
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.

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.  */
+  struct gdbarch *arch = target_gdbarch ();
 
   cleanup = ensure_python_env (arch, current_language);
   object->pspace = NULL;

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

[Doug Evans]

On Mon, May 19, 2014 at 1:39 PM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> My prune_inferiors patch
>
> 2014-05-17  Doug Evans  <xdje42@gmail.com>
>
>         * inferior.c (prune_inferiors): Fix comment.
>         (remove_inferior_command): Call prune_program_spaces.
>
> is causing gdb.multi/base.exp to fail:
>
> UNRESOLVED: gdb.multi/base.exp: remove-inferiors 2-3
> UNRESOLVED: gdb.multi/base.exp: check remove-inferiors
>
> gdb is crashing because it's accessing/freeing already freed memory.
> I was running the testsuite all weekend, sorry I only saw this now.
>
> E.g.
> ==16368== Invalid read of size 4
> ==16368==    at 0x660A9D: find_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:1349)
> ==16368==    by 0x663ECB: lookup_minimal_symbol_by_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/minsyms.c:734)
> ==16368==    by 0x5D987A: find_pc_sect_symtab (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/symtab.c:2153)
> ==16368==    by 0x5D4D77: blockvector_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:168)
> ==16368==    by 0x5D4F59: block_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:246)
> ==16368==    by 0x5D4F9B: block_for_pc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:258)
> ==16368==    by 0x734C5D: inline_frame_sniffer (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inline-frame.c:218)
> ==16368==    by 0x732104: frame_unwind_try_unwinder (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:108)
> ==16368==    by 0x73223F: frame_unwind_find_by_frame (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:159)
> ==16368==    by 0x72D5AA: compute_frame_id (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:453)
> ==16368==    by 0x7300EC: get_prev_frame_if_no_cycle (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1758)
> ==16368==    by 0x73079A: get_prev_frame_always (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1931)
> ==16368==  Address 0x5b13500 is 16 bytes inside a block of size 24 free'd
> ==16368==    at 0x403072E: free (valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c:445)
> ==16368==    by 0x762134: xfree (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/common/common-utils.c:108)
> ==16368==    by 0x65DACF: objfiles_pspace_data_cleanup (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:91)
> ==16368==    by 0x75E546: program_spaceregistry_callback_adaptor (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45)
> ==16368==    by 0x7644F6: registry_clear_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:82)
> ==16368==    by 0x7645AB: registry_container_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:95)
> ==16368==    by 0x75E5B4: program_space_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45)
> ==16368==    by 0x75E9BA: release_program_space (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:167)
> ==16368==    by 0x75EB9B: prune_program_spaces (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:269)
> ==16368==    by 0x75303D: remove_inferior_command (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inferior.c:792)
> ==16368==    by 0x50B5FD: do_cfunc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:107)
> ==16368==    by 0x50E6F2: cmd_func (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:1886)
>
> The problem originates from the get_current_arch call in
> py-progspace.c:py_free_pspace.
> 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.
>
> 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.  */
> +  struct gdbarch *arch = target_gdbarch ();
>
>    cleanup = ensure_python_env (arch, current_language);
>    object->pspace = NULL;

Hmmm...

I looked at py_free_inferior to see what it does:

---snip---
static void
py_free_inferior (struct inferior *inf, void *datum)
{

  struct cleanup *cleanup;
  inferior_object *inf_obj = datum;
  struct threadlist_entry *th_entry, *th_tmp;

  if (!gdb_python_initialized)
    return;

  cleanup = ensure_python_env (python_gdbarch, python_language);
---snip---

Just passing python_gdbarch back into ensure_python_env doesn't feel right.
One might try to get the arch from INF but it is being destructed and
we it would be preferable to not have to rely on it being available.
One solution would be to have a special "dummy" or some such arch for
use during these kinds of situations.

Plus I see python_on_normal_stop calling get_current_arch whereas
python_on_resume calling target_gdbarch.
There are no comments in the code explaining why things are the way
they are, thus it's hard to reason whether these choices have been
deliberately made or are essentially random.  It could be the case
that either choice is fine because it will never be used, but IWBN to
have that documented (or better yet just be consistent and document it
some place).

It feels like there's a good cleanup project here.
Am I missing something?

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

[Doug Evans]

On Mon, May 19, 2014 at 2:52 PM, Doug Evans <dje@google.com> wrote:
> On Mon, May 19, 2014 at 1:39 PM, Doug Evans <dje@google.com> wrote:
>> Hi.
>>
>> My prune_inferiors patch
>>
>> 2014-05-17  Doug Evans  <xdje42@gmail.com>
>>
>>         * inferior.c (prune_inferiors): Fix comment.
>>         (remove_inferior_command): Call prune_program_spaces.
>>
>> is causing gdb.multi/base.exp to fail:
>>
>> UNRESOLVED: gdb.multi/base.exp: remove-inferiors 2-3
>> UNRESOLVED: gdb.multi/base.exp: check remove-inferiors
>>
>> gdb is crashing because it's accessing/freeing already freed memory.
>> I was running the testsuite all weekend, sorry I only saw this now.
>>
>> E.g.
>> ==16368== Invalid read of size 4
>> ==16368==    at 0x660A9D: find_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:1349)
>> ==16368==    by 0x663ECB: lookup_minimal_symbol_by_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/minsyms.c:734)
>> ==16368==    by 0x5D987A: find_pc_sect_symtab (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/symtab.c:2153)
>> ==16368==    by 0x5D4D77: blockvector_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:168)
>> ==16368==    by 0x5D4F59: block_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:246)
>> ==16368==    by 0x5D4F9B: block_for_pc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:258)
>> ==16368==    by 0x734C5D: inline_frame_sniffer (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inline-frame.c:218)
>> ==16368==    by 0x732104: frame_unwind_try_unwinder (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:108)
>> ==16368==    by 0x73223F: frame_unwind_find_by_frame (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:159)
>> ==16368==    by 0x72D5AA: compute_frame_id (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:453)
>> ==16368==    by 0x7300EC: get_prev_frame_if_no_cycle (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1758)
>> ==16368==    by 0x73079A: get_prev_frame_always (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1931)
>> ==16368==  Address 0x5b13500 is 16 bytes inside a block of size 24 free'd
>> ==16368==    at 0x403072E: free (valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c:445)
>> ==16368==    by 0x762134: xfree (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/common/common-utils.c:108)
>> ==16368==    by 0x65DACF: objfiles_pspace_data_cleanup (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:91)
>> ==16368==    by 0x75E546: program_spaceregistry_callback_adaptor (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45)
>> ==16368==    by 0x7644F6: registry_clear_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:82)
>> ==16368==    by 0x7645AB: registry_container_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:95)
>> ==16368==    by 0x75E5B4: program_space_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45)
>> ==16368==    by 0x75E9BA: release_program_space (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:167)
>> ==16368==    by 0x75EB9B: prune_program_spaces (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:269)
>> ==16368==    by 0x75303D: remove_inferior_command (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inferior.c:792)
>> ==16368==    by 0x50B5FD: do_cfunc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:107)
>> ==16368==    by 0x50E6F2: cmd_func (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:1886)
>>
>> The problem originates from the get_current_arch call in
>> py-progspace.c:py_free_pspace.
>> 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.
>>
>> 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.  */
>> +  struct gdbarch *arch = target_gdbarch ();
>>
>>    cleanup = ensure_python_env (arch, current_language);
>>    object->pspace = NULL;
>
> Hmmm...
>
> I looked at py_free_inferior to see what it does:
>
> ---snip---
> static void
> py_free_inferior (struct inferior *inf, void *datum)
> {
>
>   struct cleanup *cleanup;
>   inferior_object *inf_obj = datum;
>   struct threadlist_entry *th_entry, *th_tmp;
>
>   if (!gdb_python_initialized)
>     return;
>
>   cleanup = ensure_python_env (python_gdbarch, python_language);
> ---snip---
>
> Just passing python_gdbarch back into ensure_python_env doesn't feel right.
> One might try to get the arch from INF but it is being destructed and
> we it would be preferable to not have to rely on it being available.
> One solution would be to have a special "dummy" or some such arch for
> use during these kinds of situations.
>
> Plus I see python_on_normal_stop calling get_current_arch whereas
> python_on_resume calling target_gdbarch.
> There are no comments in the code explaining why things are the way
> they are, thus it's hard to reason whether these choices have been
> deliberately made or are essentially random.  It could be the case
> that either choice is fine because it will never be used, but IWBN to
> have that documented (or better yet just be consistent and document it
> some place).
>
> It feels like there's a good cleanup project here.
> Am I missing something?

Curiouser and curiouser ...

progspace.c:release_program_space has this:

  if (!gdbarch_has_shared_address_space (target_gdbarch ()))
    free_address_space (pspace->aspace);

That also can't be right. [Right?]
We haven't (and can't) set current_inferior to one that used this
program space: it's gone.
So target_gdbarch() is essentially returning a random value upon which
we will decide whether to call free_address_space.
Since it's the arch that decides "has_shared_address_space", and since
we're allocating an object stored in a pspace based on this decision,
ISTM that a program space also has an arch.

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

[Yao Qi]

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 (齐尧)

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

[Doug Evans]

Here is what I checked in.

2014-05-20  Doug Evans  <dje@google.com>

	* python/py-progspace.c (py_free_pspace): Call target_gdbarch
	instead of get_current_arch.

diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index cda5a86..db4c564 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: All inferiors that use PSPACE may no longer exist.
+     We don't need to do much here, and since "there is always an inferior"
+     using target_gdbarch 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.  */
+  struct gdbarch *arch = target_gdbarch ();
 
   cleanup = ensure_python_env (arch, current_language);
   object->pspace = NULL;