[binutils-gdb] gdb: delete some unnecessary code from core_target::detach

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=eecd9f4cece1b7c89c6c3ac188e1a29152b2be7a

commit eecd9f4cece1b7c89c6c3ac188e1a29152b2be7a
Author: Andrew Burgess <[email protected]>
Date:   Sun Mar 29 20:19:55 2026 +0100

    gdb: delete some unnecessary code from core_target::detach
    
    This commit removes some unnecessary code from core_target::detach.
    
    When a core_target is created the core BFD (m_core_bfd) is set, and
    will never be NULL.  The m_core_bfd remains set until either
    core_target::detach or core_target::close is called.
    
    The core_target::close function is only called when the refcount of a
    core_target reaches zero, the core_target::close function deletes the
    core_target, so we know that after calling core_target::close no other
    core_target member functions will be called (as the core_target will
    have been deleted).
    
    The core_target::detach function is called as a result of calling
    target_detach, which is called as a result of either the 'detach'
    command, or the 'core-file' command (without passing a file name).
    
    As a core_target is not shareable (see
    process_stratum_target::is_shareable), once a core_target is detached,
    its reference count will eventually reduce to zero (a reference is
    temporarily held in target_detach), and then it will be closed and
    deleted.
    
    What this means is that there is absolutely no way that a core_target
    can ever be detached twice, not that such a thing would make much
    sense, but it cannot happen.
    
    Understanding this we can know that when core_target::detach is called
    m_core_bfd will never be NULL, I've added an assert for this case.
    
    Given this assert, if we look at core_target::clear_core, which
    core_target::detach calls, we can see that exit_inferior will always
    be called.  If we look at exit_inferior (in inferior.c) we see that
    the last two actions of that function are:
    
      /* Clear the register cache and the frame cache.  */
      registers_changed ();
      reinit_frame_cache ();
    
    Which are also two of the last three actions of core_target::detach.
    Clearly the calls in core_target::detach are redundant.
    
    Just for good measure, if we look in target_detach, from where
    core_target::detach will have been called, just before the function
    returns we have:
    
      registers_changed_ptid (proc_target, save_pid_ptid);
      reinit_frame_cache ();
    
    The registers_changed_ptid call is slightly more restrictive, only
    clearing the register cache for the target being detached, but that
    should be good enough -- I think exit_inferior could probably be
    changed to call registers_changed_ptid for the inferior that exited,
    but that's a problem for another day.
    
    What this all tells me is that the registers_changed call and the
    reinit_frame_cache call in core_target::detach are unnecessary, and
    can be deleted, which is what this patch does.
    
    Given I was making changes in core_target::detach, I've taken this
    opportunity to update an out of date comment.  The comment talked
    about 'this' possibly becoming dangling, however, this was never the
    case as target_detach holds a reference to the target, preventing it
    from being deleted until target_detach returns.
    
    There should be no user visible changes after this commit.

Diff:
---
 gdb/corelow.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index 8548311b906..c66ff50cbc2 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -1241,20 +1241,28 @@ core_target_open (const char *arg, int from_tty)
 void
 core_target::detach (inferior *inf, int from_tty)
 {
+  /* The core BFD is set when the core_target is created and attached to
+     the inferior.  It is only cleared during detach or close.  After
+     detaching the core target will be closed and deleted, so detach can
+     never be called twice.  What this means is that detach will never be
+     called without the core BFD being set.  */
+  gdb_assert (this->core_bfd () != nullptr);
+
   /* Get rid of the core.  Don't rely on core_target::close doing it,
      because target_detach may be called with core_target's refcount > 1,
      meaning core_target::close may not be called yet by the
      unpush_target call below.  */
   clear_core ();
 
-  /* Note that 'this' may be dangling after this call.  unpush_target
-     closes the target if the refcount reaches 0, and our close
-     implementation deletes 'this'.  */
+  /* This detach method should only be called from target_detach, which
+     holds a reference to this core_target.  As such, this core_target will
+     not be deleted when it is unpushed from INF's target stack.  Instead
+     this core_target will be deleted when target_detach returns, at which
+     point the reference count on this core_target will reach 0, and our
+     close method will delete 'this'.  */
   inf->unpush_target (this);
 
-  /* Clear the register cache and the frame cache.  */
-  registers_changed ();
-  reinit_frame_cache ();
+  /* Inform the user.  */
   maybe_say_no_core_file_now (from_tty);
 }