diff options
author | Pedro Alves <pedro@palves.net> | 2022-11-17 23:08:58 +0000 |
---|---|---|
committer | Pedro Alves <pedro@palves.net> | 2023-02-27 19:12:28 +0000 |
commit | a81871f7136c0c0ae6e9d921cb0717829543ee61 (patch) | |
tree | 89bff973d4fc04a03c9e6628b891890ee63c1b50 /gdb/infrun.c | |
parent | fb6d30e013edc16cc830447a7115c0a5189f753f (diff) | |
download | binutils-gdb-a81871f7136c0c0ae6e9d921cb0717829543ee61.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
Diffstat (limited to 'gdb/infrun.c')
-rw-r--r-- | gdb/infrun.c | 43 |
1 files changed, 28 insertions, 15 deletions
diff --git a/gdb/infrun.c b/gdb/infrun.c index 3629db0443a..8983a1024eb 100644 --- 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 (); } @@ -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) |