Re: [PATCH 2/3] gdb: refactor core_target ::close and ::detach functions

[Lancelot SIX <lancelot.six@amd.com>]

On Mon, Mar 30, 2026 at 04:30:52PM +0100, Andrew Burgess wrote:
>  void
>  core_target::close ()
>  {
> -  clear_core ();
> +  /* The core BFD is set when the core_target is created and attached to
> +     the inferior.  It is never explicitly cleared, instead m_core_bfd will
> +     have its reference count reduced when the core_target is deleted.  */
> +  gdb_assert (this->core_bfd () != nullptr);
> +
> +  /* If we called ::detach before calling ::close then the inferior will
> +     have already been exited.  This will happen if the user clears the
> +     core file with the 'core-file' or 'detach' commands.
> +
> +     However, if the user just causes the core_target to be unpushed, by
> +     pushing an alternative target, e.g. 'target remote ....', then we will
> +     not call ::detach before calling ::close.
> +
> +     In the former case we don't want to exit the inferior twice; this is
> +     mostly harmless except it causes two 'exited' events to be emitted in
> +     the Python API, which isn't ideal.
> +
> +     As opening a core_target always ensures that some thread is selected,
> +     then we can tell if exit_core_file_inferior has already been called by
> +     checking if no thread is now selected.  */
> +  if (inferior_ptid != null_ptid)
> +    exit_core_file_inferior ();

Hi Andrew,

We are observing a behaviour change after your change in the downstream
ROCgdb port.  Long story short is: when doing "exit" with a core file
opened, the "inferior_exit" observer is not called at all.

When calling "exit", we execute:

  quit_command
    quit_force
      inferior::pop_all_targets
        pop_all_targets_above (dummy_stratum)

From there, pop_all_targets does:

    switch_to_inferior_no_thread (this);

    while (top_target ()->stratum () > stratum)
      unpush_target_and_assert (top_target ());

The switch_to_inferior_no_thread sets inferior_ptid to null_ptid, then
we unpush the core_target, decrement its refcount and end up here in
core_target::close.  Because inferior_ptid is null_ptid, we skip calling
exit_core_file_inferior.

One would expect that we detach before reaching this point, and
quit_force tries to do so:

     for (inferior *inf : all_inferiors ())
       kill_or_detach (inf, from_tty);

However, kill_or_detach explicitly does not call target_detach for core
files:

    /* Leave core files alone.  */
    if (target_has_execution ())
      {
        if (inf->attach_flag)
	  target_detach (inf, from_tty);
	else
	  target_kill ();
      }

Because exit_core_file_inferior is not called, we never call
"exit_inferior (current_inferior ())", and therefore fail to notify the
"inferior_exit" observer.

In our case, we notice this because we use the inferior_exit observer to
detach the GPU side of the process.  Because we fail to do this detach,
we leave some unclean state in our GPU debugging library, which
eventually causes complaints (i.e. segfault)  when calling global
destructors.

Given this scenario, I expect the last part of the comment regarding the
guarantee of having a thread selected is invalid.  I have not looked too
deeply into a solution yet, but I can.  Given that the core_target is
not shareable, could each instance have a "detached" flag which could be
used in placed of checking inferior_ptid against null_ptid?

Best,
Lancelot.

>  
>    /* Core targets are heap-allocated (see core_target_open), so here
>       we delete ourselves.  */
>    delete this;
> +
> +  /* Notify that the core file has changed.  This is intentionally done
> +     after the core_target is deleted as nothing in here depends on the
> +     core_target itself, the core_target has already been removed from the
> +     inferior's target stack by this point.  */
> +  gdb::observers::core_file_changed.notify (current_inferior ());
>  }