[PATCH] Switch the inferior too in switch_to_program_space_and_thread (Re: [PATCH v2 08/24] Introduce switch_to_inferior_no_thread)

[Aktemur, Tankut Baris]

On Friday, January 10, 2020 3:03 AM, Pedro Alves wrote:
> 
> The changes around clear_symtab_users calls are necessary because
> otherwise we regress gdb.base/step-over-exit.exp, hitting the new
> assertion in switch_to_program_space_and_thread.  The problem is, a
> forked child terminates, and when GDB decides to auto-purge that
> inferior, GDB tries to switch to the pspace of that no-longer-existing
> inferior.
> 
> The root of the problem is within the program_space destructor:
> 
> program_space::~program_space ()
> {
> ...
>   set_current_program_space (this);        # (1)
> ...
>   breakpoint_program_space_exit (this);    # (2)
> ...
>   free_all_objfiles ();                    # (3)
> ...
> }
> 
> We get here from delete_inferior -> delete_program_space.
> 
> So we're deleting an inferior, and the inferior to be
> deleted is no longer in the inferior list.
> 
> At (2), we've deleted all the breakpoints and locations for the
> program space being deleted.
> 
> The crash happens while doing a breakpoint re-set, called by
> clear_symtab_users at the tail end of (3).  That is, while recreating
> breakpoints for the current program space, which is the program space
> we're tearing down.  During breakpoint re-set, we try to switch to the
> new location's pspace (the current pspace set in (1), so the pspace
> we're tearing down) with switch_to_program_space_and_thread, and that
> hits the failed assertion.  It's the fact that we recreate breakpoints
> in the program_space destructor that is the latent bug here.  Just
> don't do that, since we've already taken care of breakpoints with
> breakpoint_program_space_exit, and we don't end up in the crash situation.
> 
> My first approach to fix this added a symfile_add_flags parameter to
> program_space::free_all_objfiles, and then passed that down to
> clear_symtab_users.  The program_space dtor would then pass down
> SYMFILE_DEFER_BP_RESET to free_all_objfiles.  I couldn't help feeling
> that adding that parameter to free_all_objfiles looked a little
> awkward, so I settled on something a little different -- hoist the
> clear_symtab_users call to the callers.  There are only two callers.
> I felt that that didn't look as odd, particularly since
> remove_symbol_file_command also does:
> 
>   objf->unlink ();
>   clear_symtab_users (0);
> 
> I.e., objfile deletion is already separate from calling
> clear_symtab_users in some places.
> 
> I'm not hard set on this approach though; I can go with adding the
> symfile_add_flags parameter to program_space::free_all_objfiles if
> people prefer that.

I personally think that in particular the comment and the change below
have educational value.  So, I'd vote for the approach you sent.

> diff --git a/gdb/progspace.c b/gdb/progspace.c
> index 96f8acc64c..1361040347 100644
> --- a/gdb/progspace.c
> +++ b/gdb/progspace.c
> @@ -147,6 +147,9 @@ program_space::~program_space ()
>    no_shared_libraries (NULL, 0);
>    exec_close ();
>    free_all_objfiles ();
> +  /* Defer breakpoint re-set because we don't want to create new
> +     locations for this pspace which we're tearing down.  */
> +  clear_symtab_users (SYMFILE_DEFER_BP_RESET);
>    if (!gdbarch_has_shared_address_space (target_gdbarch ()))
>      free_address_space (this->aspace);
>    clear_section_table (&this->target_sections);

As far as I can tell, you rebased the series on the current master, correct?
Could you update the users/palves/multi-target-v2 branch?  The new patch does not
apply there cleanly.  I attempted to replicate it on users/palves/multi-target-v2,
but gdb.base/step-over-exit.exp still hits the assertion.

-Baris

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928