Fix removing inferiors from within "thread apply" commands

This patch fixes an internal error exposed by a test that does
something like:

  define kill-and-remove
    kill inferiors 2
    remove-inferiors 2
  end

  # Start one inferior.
  start

  # Start another inferior.
  add-inferior 2
  inferior 2
  start

  # Kill and remove inferior 1 while inferior 2 is selected.
  thread apply 1.1 kill-and-remove

The internal error looks like this:

 Thread 1.1 (Thread 0x7ffff7fc2700 (LWP 20677)):
 [Switching to inferior 1 [process 20677] (gdb/testsuite/outputs/gdb.threads/threadapply/threadapply)]
 [Switching to thread 1.1 (Thread 0x7ffff7fc2700 (LWP 20677))]
 #0  main () at src/gdb/testsuite/gdb.threads/threadapply.c:38
 38          for (i = 0; i < NUM; i++)
 src/gdb/inferior.c:66: internal-error: void set_current_inferior(inferior*): Assertion `inf != NULL' failed.
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 Quit this debugging session? (y or n) FAIL: gdb.threads/threadapply.exp: kill_and_remove_inferior: try kill-and-remove: thread apply 1.1 kill-and-remove (GDB internal error)

There are several problems around this area of the code.  One is that
in do_restore_current_thread_cleanup, we do a look up of inferior by
ptid, which can find the wrong inferior if the previously selected
inferior exited and some other inferior was started with a reused pid
(rare, but still...).

The other problem is that the "remove-inferiors" command rejects
attempts to remove the current inferior, but when we get to
"remove-inferiors" in a "thread apply THR remove-inferiors 2" command,
the current inferior is the inferior of thread THR, not the previously
selected inferior, so if the previously selected inferior was inferior
2, that command still manages to wipe it, and then gdb restores the
old selected inferior, which is now a dangling pointer...

So the fix here is:

- Make make_cleanup_restore_current_thread store a pointer to the
  previously selected inferior directly, and use it directly instead
  of doing ptid look ups.

- Add a refcount to inferiors, very similar to thread_info's refcount,
  that is incremented/decremented by
  make_cleanup_restore_current_thread, and checked before deleting an
  inferior.  To avoid duplication, a new refcounted_object type is
  added, that both thread_info and inferior inherit from.

gdb/ChangeLog:
2017-04-19  Pedro Alves  <palves@redhat.com>

	* common/refcounted-object.h: New file.
	* gdbthread.h: Include "common/refcounted-object.h".
	(thread_info): Inherit from refcounted_object and add comments.
	(thread_info::incref, thread_info::decref)
	(thread_info::m_refcount): Delete.
	(thread_info::deletable): Use the refcounted_object::refcount()
	method.
	* inferior.c (current_inferior_): Add comment.
	(set_current_inferior): Increment/decrement refcounts.
	(prune_inferiors, remove_inferior_command): Skip inferiors marked
	not-deletable instead of comparing with the current inferior.
	(initialize_inferiors): Increment the initial inferior's refcount.
	* inferior.h (struct inferior): Forward declare.
	Include "common/refcounted-object.h".
	(current_inferior, set_current_inferior): Move declaration to
	before struct inferior's definition, and fix comment.
	(inferior): Inherit from refcounted_object.  Add comments.
	* thread.c (switch_to_thread_no_regs): Reference the thread's
	inferior pointer directly instead of doing a ptid lookup.
	(switch_to_no_thread): New function.
	(switch_to_thread(thread_info *)): New function, factored out
	from ...
	(switch_to_thread(ptid_t)): ... this.
	(restore_current_thread): Delete.
	(current_thread_cleanup): Remove 'inf_id' and 'was_removable'
	fields, and add 'inf' field.
	(do_restore_current_thread_cleanup): Check whether old->inf is
	alive instead of looking up an inferior by ptid.  Use
	switch_to_thread and switch_to_no_thread.
	(restore_current_thread_cleanup_dtor): Use old->inf directly
	instead of lookup up an inferior by id.  Decref the inferior.
	Don't restore 'removable'.
	(make_cleanup_restore_current_thread): Same the inferior pointer
	in old, instead of the inferior number.  Incref the inferior.
	Don't save/clear 'removable'.

gdb/testsuite/ChangeLog:
2017-04-19  Pedro Alves  <palves@redhat.com>

	* gdb.threads/threadapply.exp (kill_and_remove_inferior): New
	procedure.
	(top level): Call it.
	* lib/gdb.exp (gdb_define_cmd): New procedure.

1 files changed, 21 insertions, 24 deletions

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 4cd7390..33977ee 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -31,6 +31,7 @@ struct symtab;
 #include "common/vec.h"
 #include "target/waitstatus.h"
 #include "cli/cli-utils.h"
+#include "common/refcounted-object.h"
 
 /* Frontend view of the thread state.  Possible extensions: stepping,
    finishing, until(ling),...  */
@@ -177,7 +178,24 @@ typedef struct value *value_ptr;
 DEF_VEC_P (value_ptr);
 typedef VEC (value_ptr) value_vec;
 
-struct thread_info
+/* Threads are intrusively refcounted objects.  Being the
+   user-selected thread is normally considered an implicit strong
+   reference and is thus not accounted in the refcount, unlike
+   inferior objects.  This is necessary, because there's no "current
+   thread" pointer.  Instead the current thread is inferred from the
+   inferior_ptid global.  However, when GDB needs to remember the
+   selected thread to later restore it, GDB bumps the thread object's
+   refcount, to prevent something deleting the thread object before
+   reverting back (e.g., due to a "kill" command.  If the thread
+   meanwhile exits before being re-selected, then the thread object is
+   left listed in the thread list, but marked with state
+   THREAD_EXITED.  (See make_cleanup_restore_current_thread and
+   delete_thread).  All other thread references are considered weak
+   references.  Placing a thread in the thread list is an implicit
+   strong reference, and is thus not accounted for in the thread's
+   refcount.  */
+
+class thread_info : public refcounted_object
 {
 public:
   explicit thread_info (inferior *inf, ptid_t ptid);
@@ -186,22 +204,8 @@ public:
   bool deletable () const
   {
     /* If this is the current thread, or there's code out there that
-       relies on it existing (m_refcount > 0) we can't delete yet.  */
-    return (m_refcount == 0 && !ptid_equal (ptid, inferior_ptid));
-  }
-
-  /* Increase the refcount.  */
-  void incref ()
-  {
-    gdb_assert (m_refcount >= 0);
-    m_refcount++;
-  }
-
-  /* Decrease the refcount.  */
-  void decref ()
-  {
-    m_refcount--;
-    gdb_assert (m_refcount >= 0);
+       relies on it existing (refcount > 0) we can't delete yet.  */
+    return (refcount () == 0 && !ptid_equal (ptid, inferior_ptid));
   }
 
   struct thread_info *next = NULL;
@@ -362,13 +366,6 @@ public:
      fields point to self.  */
   struct thread_info *step_over_prev = NULL;
   struct thread_info *step_over_next = NULL;
-
-private:
-
-  /* If this is > 0, then it means there's code out there that relies
-     on this thread being listed.  Don't delete it from the lists even
-     if we detect it exiting.  */
-  int m_refcount = 0;
 };
 
 /* Create an empty thread list, or empty the existing one.  */