Convert previous_inferior_ptid to strong reference to thread_info

I originally wrote this patch, because while working on some other
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.

I have since found a new motivation for this change -- I would like to
tweak "info program" to not rely on get_last_target_status returning a
ptid that still exists in the thread list.  With both the follow_fork
changes later in this series, and the step-over-thread-exit changes,
that can happen, as we'll delete threads and not clear the last
waitstatus.

A new update_previous_thread function is added that can be used to
update 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: I42779a1ee51a996fa1e8f6e1525c6605dbfd42c7

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index a851fe1f8c85c391ab387fc8f8a6122b8e1b8083..76453402c93a01331b3c41f67731803d3d6f4e43 100644 (file)
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2451,6 +2451,8 @@ kill_command (const char *arg, int from_tty)
   target_kill ();
   bfd_cache_close_all ();
 
+  update_previous_thread ();
+
   if (print_inferior_events)
     gdb_printf (_("[Inferior %d (%s) killed]\n"),
                infnum, pid_str.c_str ());
@@ -2811,6 +2813,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
@@ -2849,6 +2853,7 @@ disconnect_command (const char *args, int from_tty)
   target_disconnect (args, from_tty);
   no_shared_libraries (nullptr, 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 3629db0443a3a7b727d93f27b46ca38863b90e91..8983a1024ebe0d396be454d30d817f1108343efc 100644 (file)
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -152,8 +152,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.
@@ -3151,7 +3161,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 ();
@@ -3434,7 +3444,7 @@ init_wait_for_inferior (void)
 
   nullify_last_target_wait_ptid ();
 
-  previous_inferior_ptid = inferior_ptid;
+  update_previous_thread ();
 }
 
 \f
@@ -8704,21 +8714,24 @@ normal_stop ()
      the current thread back to the thread the user had selected right
      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)
+  if (!non_stop)
     {
-      SWITCH_THRU_ALL_UIS ()
+      if ((last.kind () != TARGET_WAITKIND_SIGNALLED
+          && last.kind () != TARGET_WAITKIND_EXITED
+          && last.kind () != TARGET_WAITKIND_NO_RESUMED)
+         && target_has_execution ()
+         && previous_thread != inferior_thread ())
        {
-         target_terminal::ours_for_output ();
-         gdb_printf (_("[Switching to %s]\n"),
-                     target_pid_to_str (inferior_ptid).c_str ());
-         annotate_thread_changed ();
+         SWITCH_THRU_ALL_UIS ()
+           {
+             target_terminal::ours_for_output ();
+             gdb_printf (_("[Switching to %s]\n"),
+                         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 43fd1b44f5aeefc919f1cedfa211a3ffb4a8e8fa..f4f2216e09246e6957446e097a91868040d79cac 100644 (file)
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -117,6 +117,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 d0aa8f5cc6c1c4bcb4b8265642d7d23cda1dd5b5..0cebecfafc379534180a6563d246cdbc3488311e 100644 (file)
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2469,6 +2469,8 @@ target_pre_inferior (int from_tty)
 
   current_inferior ()->highest_thread_num = 0;
 
+  update_previous_thread ();
+
   agent_capability_invalidate ();
 }
 
@@ -2497,6 +2499,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