summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPedro Alves <pedro@palves.net>2022-11-17 23:08:58 +0000
committerPedro Alves <pedro@palves.net>2022-12-03 20:54:57 +0000
commitddd4202830aa9a4d1d53169ee51c72329e1685c4 (patch)
tree438ec0fad4eee20ddf3e62b1cb9e4af30f65fecd
parentd34abbc2d75705020caf4e61b10d6755d5666a62 (diff)
downloadbinutils-gdb-ddd4202830aa9a4d1d53169ee51c72329e1685c4.tar.gz
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
-rw-r--r--gdb/infcmd.c5
-rw-r--r--gdb/infrun.c43
-rw-r--r--gdb/infrun.h4
-rw-r--r--gdb/target.c5
4 files changed, 42 insertions, 15 deletions
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index a27d3577b3a..bffe8d554e7 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2456,6 +2456,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 ());
@@ -2816,6 +2818,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
@@ -2854,6 +2858,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 c67458b30b6..a936f18fbb1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -151,8 +151,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.
@@ -3150,7 +3160,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 ();
@@ -3433,7 +3443,7 @@ init_wait_for_inferior (void)
nullify_last_target_wait_ptid ();
- previous_inferior_ptid = inferior_ptid;
+ update_previous_thread ();
}
@@ -8647,21 +8657,24 @@ normal_stop (void)
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 c711b9b21cc..acaec9b5064 100644
--- 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 1dd0f42af7d..f781f7e4f96 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2502,6 +2502,8 @@ target_pre_inferior (int from_tty)
current_inferior ()->highest_thread_num = 0;
+ update_previous_thread ();
+
agent_capability_invalidate ();
}
@@ -2530,6 +2532,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