[binutils-gdb] gdb: ensure all targets are popped before an inferior is destructed

[binutils-gdb] gdb: ensure all targets are popped before an inferior is destructed

[Andrew Burgess]

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

commit 740a579fd5f5cd56bfb42a3f62c61ffd42d43186
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Sep 22 18:11:30 2022 +0100

    gdb: ensure all targets are popped before an inferior is destructed
    
    Now that the inferiors target_stack automatically manages target
    reference counts, we might think that we don't need to unpush targets
    when an inferior is deleted...
    
    ...unfortunately that is not the case.  The inferior::unpush function
    can do some work depending on the type of target, so it is important
    that we still pass through this function.
    
    To ensure that this is the case, in this commit I've added an assert
    to inferior::~inferior that ensures the inferior's target_stack is
    empty (except for the ever present dummy_target).
    
    I've then added a pop_all_targets call to delete_inferior, otherwise
    the new assert will fire in, e.g. the gdb.python/py-inferior.exp test.

Diff:
---
 gdb/inferior.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/gdb/inferior.c b/gdb/inferior.c
index 683e8952d00..ec63e0491e5 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -70,6 +70,15 @@ inferior::~inferior ()
 {
   inferior *inf = this;
 
+  /* Before the inferior is deleted, all target_ops should be popped from
+     the target stack, this leaves just the dummy_target behind.  If this
+     is not done, then any target left in the target stack will be left
+     with an artificially high reference count.  As the dummy_target is
+     still on the target stack then we are about to loose a reference to
+     that target, leaving its reference count artificially high.  However,
+     this is not critical as the dummy_target is a singleton.  */
+  gdb_assert (m_target_stack.top ()->stratum () == dummy_stratum);
+
   m_continuations.clear ();
   target_desc_info_free (inf->tdesc_info);
 }
@@ -233,6 +242,12 @@ delete_inferior (struct inferior *inf)
 
   gdb::observers::inferior_removed.notify (inf);
 
+  /* Pop all targets now, this ensures that inferior::unpush is called
+     correctly.  As pop_all_targets ends up making a temporary switch to
+     inferior INF then we need to make this call before we delete the
+     program space, which we do below.  */
+  inf->pop_all_targets ();
+
   /* If this program space is rendered useless, remove it. */
   if (inf->pspace->empty ())
     delete inf->pspace;