[PATCH v2 14/16] convert previous_inferior_ptid to strong reference to thread_info

[Pedro Alves <pedro@palves.net>]

While working on a patch, I spotted a regression in the
gdb.multi/multi-target-no-resumed.exp.exp testcase.  Debugging the
issue, I realized that the problem was related to how I was using
previous_inferior_ptid to look up the thread the user had last
selected.  The problem is that previous_inferior_ptid alone doesn't
tell you which target that ptid is from, and I was just always using
the current target, which was incorrect.  Two different targets may
have threads with the same ptid.

I decided to fix this by replacing previous_inferior_ptid with a
strong reference to the thread, called previous_thread.

A new update_previous_thread function is added can be used to updated
previous_thread from inferior_ptid.

This must be called in several places that really want to get rid of
previous_thread thread, and reset the thread id counter, otherwise we
get regressions like these:

  (gdb) info threads -gid
    Id   GId  Target Id                               Frame
 - * 1    1    Thread 2974541.2974541 "tids-gid-reset" main () at src/gdb/testsuite/gdb.multi/tids-gid-reset.c:21
 - (gdb) PASS: gdb.multi/tids-gid-reset.exp: single-inferior: after restart: info threads -gid
 + * 1    2    Thread 2958361.2958361 "tids-gid-reset" main () at src/gdb/testsuite/gdb.multi/tids-gid-reset.c:21
 + (gdb) FAIL: gdb.multi/tids-gid-reset.exp: single-inferior: after restart: info threads -gid

and:

  Core was generated by `build/gdb/testsuite/outputs/gdb.reverse/sigall-precsave/si'.
  Program terminated with signal SIGTRAP, Trace/breakpoint trap.
  #0  gen_ABRT () at src/gdb/testsuite/gdb.reverse/sigall-reverse.c:398
  398      kill (getpid (), SIGABRT);
 +[Current thread is 1 (LWP 2662066)]
  Restored records from core file build/gdb/testsuite/outputs/gdb.reverse/sigall-precsave/sigall.precsave.
  #0  gen_ABRT () at src/gdb/testsuite/gdb.reverse/sigall-reverse.c:398
  398      kill (getpid (), SIGABRT);

  continue
  Continuing.

 -Program received signal SIGABRT, Aborted.
 +Thread 1 received signal SIGABRT, Aborted.
  0x00007ffff7dfd55b in kill () at ../sysdeps/unix/syscall-template.S:78
  78     ../sysdeps/unix/syscall-template.S: No such file or directory.
 -(gdb) PASS: gdb.reverse/sigall-precsave.exp: sig-test-1: get signal ABRT
 +(gdb) FAIL: gdb.reverse/sigall-precsave.exp: sig-test-1: get signal ABRT

I.e., GDB was failing to restart the thread counter back to 1, because
the previous_thread thread was being help due to the strong reference.

Tested on GNU/Linux native, gdbserver and gdbserver + "maint set
target-non-stop on".

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* infcmd.c (kill_command, detach_command, disconnect_command):
	Call update_previous_thread.
	* infrun.c (previous_inferior_ptid): Delete.
	(previous_thread): New.
	(update_previous_thread): New.
	(proceed, init_wait_for_inferior): Call update_previous_thread.
	(normal_stop): Adjust to compare previous_thread and
	inferior_thread.  Call update_previous_thread.
	* infrun.h (update_previous_thread): Declare.
	* target.c (target_pre_inferior, target_preopen): Call
	update_previous_thread.

Change-Id: I4f06dd81f00848bb7d6d26a94ff091e2a4e646d9
---
 gdb/infcmd.c |  5 +++++
 gdb/infrun.c | 25 ++++++++++++++++++-------
 gdb/infrun.h |  4 ++++
 gdb/target.c |  5 +++++
 4 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 8190ba36565..99ce0ec78fa 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2401,6 +2401,8 @@ kill_command (const char *arg, int from_tty)
 
   target_kill ();
 
+  update_previous_thread ();
+
   if (print_inferior_events)
     printf_unfiltered (_("[Inferior %d (%s) killed]\n"),
 		       infnum, pid_str.c_str ());
@@ -2756,6 +2758,8 @@ detach_command (const char *args, int from_tty)
 
   target_detach (current_inferior (), from_tty);
 
+  update_previous_thread ();
+
   /* The current inferior process was just detached successfully.  Get
      rid of breakpoints that no longer make sense.  Note we don't do
      this within target_detach because that is also used when
@@ -2794,6 +2798,7 @@ disconnect_command (const char *args, int from_tty)
   target_disconnect (args, from_tty);
   no_shared_libraries (NULL, from_tty);
   init_thread_list ();
+  update_previous_thread ();
   if (deprecated_detach_hook)
     deprecated_detach_hook ();
 }
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 1eb7526d246..3f40fa39b5a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -143,8 +143,18 @@ show_step_stop_if_no_debug (struct ui_file *file, int from_tty,
 /* proceed and normal_stop use this to notify the user when the
    inferior stopped in a different thread than it had been running
    in.  */
+static thread_info_ref previous_thread;
 
-static ptid_t previous_inferior_ptid;
+/* See infrun.h.  */
+
+void
+update_previous_thread ()
+{
+  if (inferior_ptid == null_ptid)
+    previous_thread = nullptr;
+  else
+    previous_thread = thread_info_ref::new_reference (inferior_thread ());
+}
 
 /* If set (default for legacy reasons), when following a fork, GDB
    will detach from one of the fork branches, child or parent.
@@ -3072,7 +3082,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
     }
 
   /* We'll update this if & when we switch to a new thread.  */
-  previous_inferior_ptid = inferior_ptid;
+  update_previous_thread ();
 
   regcache = get_current_regcache ();
   gdbarch = regcache->arch ();
@@ -3336,7 +3346,7 @@ init_wait_for_inferior (void)
 
   nullify_last_target_wait_ptid ();
 
-  previous_inferior_ptid = inferior_ptid;
+  update_previous_thread ();
 }
 
 \f
@@ -8559,11 +8569,11 @@ normal_stop (void)
      after this event is handled, so we're not really switching, only
      informing of a stop.  */
   if (!non_stop
-      && previous_inferior_ptid != inferior_ptid
-      && target_has_execution ()
       && last.kind != TARGET_WAITKIND_SIGNALLED
       && last.kind != TARGET_WAITKIND_EXITED
-      && last.kind != TARGET_WAITKIND_NO_RESUMED)
+      && last.kind != TARGET_WAITKIND_NO_RESUMED
+      && target_has_execution ()
+      && previous_thread != inferior_thread ())
     {
       SWITCH_THRU_ALL_UIS ()
 	{
@@ -8572,7 +8582,8 @@ normal_stop (void)
 			   target_pid_to_str (inferior_ptid).c_str ());
 	  annotate_thread_changed ();
 	}
-      previous_inferior_ptid = inferior_ptid;
+
+      update_previous_thread ();
     }
 
   if (last.kind == TARGET_WAITKIND_NO_RESUMED)
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 7ebb9fc9f4e..5d791bdc5b4 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -87,6 +87,10 @@ enum exec_direction_kind
 /* The current execution direction.  */
 extern enum exec_direction_kind execution_direction;
 
+/* Call this to point 'previous_thread' at the thread returned by
+   inferior_thread, or at nullptr, if there's no selected thread.  */
+extern void update_previous_thread ();
+
 extern void start_remote (int from_tty);
 
 /* Clear out all variables saying what to do when inferior is
diff --git a/gdb/target.c b/gdb/target.c
index 63aa3e95218..68d7e7c12d7 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2505,6 +2505,8 @@ target_pre_inferior (int from_tty)
 
   current_inferior ()->highest_thread_num = 0;
 
+  update_previous_thread ();
+
   agent_capability_invalidate ();
 }
 
@@ -2533,6 +2535,9 @@ target_preopen (int from_tty)
 	error (_("Program not killed."));
     }
 
+  /* Release reference to old previous thread.  */
+  update_previous_thread ();
+
   /* Calling target_kill may remove the target from the stack.  But if
      it doesn't (which seems like a win for UDI), remove it now.  */
   /* Leave the exec target, though.  The user may be switching from a
-- 
2.26.2