Re: [PATCH 8/8] Fix removing inferiors from within "thread apply" commands

[Pedro Alves <palves@redhat.com>]

On 04/13/2017 12:33 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> @@ -313,9 +319,31 @@ public:
>>    explicit inferior (int pid);
>>    ~inferior ();
>>  
>> +  /* Returns true if we can delete this inferior.  We can't delete it
>> +     if it is the current inferior, or if there's code out there that
>> +     relies on it existing (m_refcount > 0).  */
>> +  bool deletable () const
>> +  {
>> +    return m_refcount == 0 && this != current_inferior ();
>> +  }
>> +
>>    /* Pointer to next inferior in singly-linked list of inferiors.  */
>>    struct inferior *next = NULL;
>>  
>> +  /* Increase the refcount.  */
>> +  void incref ()
>> +  {
>> +    gdb_assert (m_refcount >= 0);
>> +    m_refcount++;
>> +  }
>> +
>> +  /* Decrease the refcount.  */
>> +  void decref ()
>> +  {
>> +    m_refcount--;
>> +    gdb_assert (m_refcount >= 0);
>> +  }
>> +
>>    /* Convenient handle (GDB inferior id).  Unique across all
>>       inferiors.  */
>>    int num = 0;
>> @@ -431,6 +459,12 @@ public:
>>  
>>    /* Per inferior data-pointers required by other GDB modules.  */
>>    REGISTRY_FIELDS;
>> +
>> +private:
>> +  /* If this is > 0, then it means there's code out there that relies
>> +     on this inferior existing.  Don't allow deleting it in that
>> +     case.  */
>> +  int m_refcount = 0;
>>  };
> 
> Can we move them to a super class, so that both thread and inferior can
> extend it?

Just to be clear, would the class include the incref()/decref() 
methods + m_refcount, plus some refcount() accessor method,
or more than that?  Let me give it a try.

>> +static void
>> +switch_to_thread (thread_info *thr)
>> +{
>> +  gdb_assert (thr != NULL);
>> +
>> +  if (inferior_ptid == thr->ptid)
>>      return;
>>  
>> -  inferior_ptid = ptid;
>> +  /* Switch the current program space and current inferior as
>> +     well.  */
>> +  set_current_program_space (thr->inf->pspace);
>> +  set_current_inferior (thr->inf);
>> +
>> +  inferior_ptid = thr->ptid;
> 
> Can these code be replaced by switch_to_thread_no_regs?

Looks like it.  I'm testing the patch below on top.

From c74e7b7b6eb29664e1afded656194266e95ee869 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 13 Apr 2017 13:11:39 +0100
Subject: [PATCH] switch_to_thread_no_regs

---
 gdb/thread.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/gdb/thread.c b/gdb/thread.c
index f48d871..2c38a9a 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1457,10 +1457,8 @@ info_threads_command (char *arg, int from_tty)
 void
 switch_to_thread_no_regs (struct thread_info *thread)
 {
-  struct inferior *inf;
+  struct inferior *inf = thread->inf;
 
-  inf = find_inferior_ptid (thread->ptid);
-  gdb_assert (inf != NULL);
   set_current_program_space (inf->pspace);
   set_current_inferior (inf);
 
@@ -1491,12 +1489,8 @@ switch_to_thread (thread_info *thr)
   if (inferior_ptid == thr->ptid)
     return;
 
-  /* Switch the current program space and current inferior as
-     well.  */
-  set_current_program_space (thr->inf->pspace);
-  set_current_inferior (thr->inf);
+  switch_to_thread_no_regs (thr);
 
-  inferior_ptid = thr->ptid;
   reinit_frame_cache ();
 
   /* We don't check for is_stopped, because we're called at times
@@ -1505,8 +1499,6 @@ switch_to_thread (thread_info *thr)
   if (thr->state != THREAD_EXITED
       && !thr->executing)
     stop_pc = regcache_read_pc (get_thread_regcache (thr->ptid));
-  else
-    stop_pc = ~(CORE_ADDR) 0;
 }
 
 /* See gdbthread.h.  */
-- 
2.5.5