From: Andrew Burgess Date: Wed, 16 Jul 2025 18:29:55 +0000 (+0100) Subject: gdb: ensure normal stop finishes the thread state of all threads X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=55dbaa6ea760489a0973d99338f69beec9c6b682;p=thirdparty%2Fbinutils-gdb.git gdb: ensure normal stop finishes the thread state of all threads This patch fixes a multi-target issue where the normal_stop function can fail to finish the thread state of threads from a non current target, this leaves the threads marked as running in GDB core, while the threads is actually stopped. For testing I used this test program: #include int main () { while (1) sleep (1); return 0; } Compile this to make '/tmp/spin', then the bug can be shown using this command: $ gdb -ex 'file /tmp/spin' \ -ex 'start' \ -ex 'add-inferior' \ -ex 'inferior 2' \ -ex 'set sysroot' \ -ex 'target extended-remote | gdbserver --multi --once - /tmp/spin' \ -ex 'inferior 1' \ -ex 'continue&' \ -ex 'inferior 2' \ -ex 'search sleep' \ -ex 'break $_ inferior 2' \ -ex 'continue' \ -ex 'info threads' The interesting part of the output is: Id Target Id Frame 1.1 process 1610445 "spin" (running) * 2.1 Thread 1610451.1610451 "spin" main () at spin.c:7 (gdb) Notice that thread 1.1 is marked as running when it should be stopped. We can see that the thread is actually stopped if we try this: (gdb) inferior 1 [Switching to inferior 1 [process 1610445] (/tmp/spin)] [Switching to thread 1.1 (process 1610445)](running) (gdb) continue Cannot execute this command while the selected thread is running. (gdb) interrupt (gdb) info threads Id Target Id Frame * 1.1 process 1610445 "spin" (running) 2.1 Thread 1610451.1610451 "spin" main () at spin.c:7 (gdb) We can see the expected behaviour if both inferiors run on the same target, like this: $ gdb -ex 'file /tmp/spin' \ -ex 'start' \ -ex 'add-inferior' \ -ex 'inferior 2' \ -ex 'file /tmp/spin' \ -ex 'start' \ -ex 'inferior 1' \ -ex 'continue&' \ -ex 'inferior 2' \ -ex 'search sleep' \ -ex 'break $_ inferior 2' \ -ex 'continue' \ -ex 'info threads' The 'info threads' from this series of commands looks like this: Id Target Id Frame 1.1 process 1611589 "spin" 0x00007ffff7e951e7 in nanosleep () from /lib64/libc.so.6 * 2.1 process 1611593 "spin" main () at spin.c:7 (gdb) Now both threads are stopped as we'd expect. The problem is in normal_stop. The scoped_finish_thread_state uses user_visible_resume_target to select the target(s) over which GDB will iterate to find the threads to update. The problem with this is that when the ptid_t is minus_one_ptid, meaning all threads, user_visible_resume_target only returns nullptr, meaning all targets, when sched_multi is true. This dependency on sched_multi makes sense when _resuming_ threads. If we are resuming all threads, then when sched_multi (the schedule-multiple setting) is off (the default), all threads actually means all threads in the current inferior only. When sched_multi is true (schedule-multiple is on) then this means all threads, from all inferiors, which means GDB needs to consider every target. However, when stopping an inferior in all-stop mode (non_stop is false), then GDB wants to stop all threads from all inferiors, regardless of the sched_multi setting. What this means is that, when 'non_stop' is false, then we should be passing nullptr as the target selection to scoped_finish_thread_state. My proposal is that we should stop using user_visible_resume_target in the normal_stop function for the target selection of the scoped_finish_thread_state, instead we should manually figure out the correct target value and pass this in. There is precedent for this in GDB, see run_command_1, where 'finish_target' is calculated directly within the function rather than using user_visible_resume_target. After this commit, when using two different targets (native and remote) as in my first example above, both threads will be correctly stopped. --- diff --git a/gdb/infrun.c b/gdb/infrun.c index 289c503963c..eeef01ae66d 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -9525,6 +9525,7 @@ normal_stop () here, so do this before any filtered output. */ ptid_t finish_ptid = null_ptid; + process_stratum_target *finish_target = nullptr; if (!non_stop) finish_ptid = minus_one_ptid; @@ -9537,17 +9538,30 @@ normal_stop () linux-fork.c automatically switches to another fork from within target_mourn_inferior. */ if (inferior_ptid != null_ptid) - finish_ptid = ptid_t (inferior_ptid.pid ()); + { + finish_ptid = ptid_t (inferior_ptid.pid ()); + finish_target = current_inferior ()->process_target (); + } } else if (last.kind () != TARGET_WAITKIND_NO_RESUMED && last.kind () != TARGET_WAITKIND_THREAD_EXITED) - finish_ptid = inferior_ptid; + { + finish_ptid = inferior_ptid; + finish_target = current_inferior ()->process_target (); + } std::optional maybe_finish_thread_state; if (finish_ptid != null_ptid) { - maybe_finish_thread_state.emplace - (user_visible_resume_target (finish_ptid), finish_ptid); + /* It might be tempting to use user_visible_resume_target to compute + FINISH_TARGET from FINISH_PTID, however, that is the wrong choice + in this case. + + When resuming, we only resume the current target unless + schedule-multiple is in effect. However, when handling a stop, if + FINISH_PTID is minus_one_ptid, then we really do want to look for + stop events from _any_ target. */ + maybe_finish_thread_state.emplace (finish_target, finish_ptid); } /* As we're presenting a stop, and potentially removing breakpoints, diff --git a/gdb/testsuite/gdb.multi/interrupt-bg-exec.c b/gdb/testsuite/gdb.multi/interrupt-bg-exec.c new file mode 100644 index 00000000000..b5fa5680cbf --- /dev/null +++ b/gdb/testsuite/gdb.multi/interrupt-bg-exec.c @@ -0,0 +1,47 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2025 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +volatile int wait_for_gdb = 1; + +void +breakpt (void) +{ + /* Nothing. */ +} + +void +all_started (void) +{ + /* Nothing. */ +} + +int +main (void) +{ + alarm (360); + + all_started (); + + while (wait_for_gdb) + sleep (1); + + breakpt (); + + return 0; +} diff --git a/gdb/testsuite/gdb.multi/interrupt-bg-exec.exp b/gdb/testsuite/gdb.multi/interrupt-bg-exec.exp new file mode 100644 index 00000000000..065a1128e5d --- /dev/null +++ b/gdb/testsuite/gdb.multi/interrupt-bg-exec.exp @@ -0,0 +1,143 @@ +# Copyright 2025 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# In all-stop mode, set one inferior running in the background, then +# continue a second inferior. When the second inferior hits a breakpoint, +# both inferiors should be stopped. + +# This tests the use of native and remote targets. If we try to run +# it with a board that forces native targets to become remote, then +# this doesn't really make sense (for this test). +require {string equal [target_info gdb_protocol] ""} + +source $srcdir/$subdir/multi-target.exp.tcl + +load_lib gdbserver-support.exp + +require allow_gdbserver_tests + +# This overrides the call in multi-target.exp.tcl. +standard_testfile + +if { [build_executable "failed to build" $testfile $srcfile] } { + return +} + +# Start two inferiors, TARGET_TYPE_1 and TARGET_TYPE_2 are strings, either +# 'extended-remote' or 'native', and control the connection type of each +# inferior. +# +# Set the first inferior running in the background, then continue theA +# second inferior allowing it to hit a breakpoint. +# +# Once the breakpoint is hit, both inferiors should be stopped. +proc run_test { target_type_1 target_type_2 } { + cleanup_gdbservers + + clean_restart + + gdb_test "disconnect" ".*" + + gdb_test_no_output "set sysroot" + + # multi-target depends on target running in non-stop mode. Force it + # on for remote targets, until this is the default. + gdb_test_no_output "maint set target-non-stop on" + + # Run in all-stop mode. + gdb_test_no_output "set non-stop off" + + if {![add_inferior 2 $target_type_1 $::binfile]} { + return 0 + } + + if {![add_inferior 3 $target_type_2 $::binfile]} { + return 0 + } + + # Check we see all the expected threads. + gdb_test "info threads" \ + [multi_line \ + "\\s+Id\\s+Target Id\\s+Frame\\s*" \ + "\\s+2\\.1\\s+\[^\r\n\]+" \ + "\\*\\s+3\\.1\\s+\[^\r\n\]+"] \ + "check expected threads exist" + + # The breakpoint will be set in both inferiors, but only inferior 3 + # will hit it as 'wait_for_gdb' is cleared only in that inferior. + gdb_breakpoint breakpt + gdb_test "thread apply 3.1 set wait_for_gdb = 0" + + # Let inferior 2 run in the background. + gdb_test "thread 2.1" + gdb_test -no-prompt-anchor "continue&" + + # Run inferior 3 until it hits a breakpoint. + gdb_test "thread 3.1" + gdb_test "continue" \ + [multi_line \ + "Thread 3\\.1 \[^\r\n\]+ hit Breakpoint \[^\r\n\]+, breakpt \\(\\) \[^\r\n\]+" \ + "$::decimal\\s+\[^\r\n\]+"] \ + "continue to breakpt function" + + # Check the state of all threads. None should be running. + set saw_inferior_2 false + set saw_inferior_3 false + gdb_test_multiple "info threads" "check threads after stop" { + -re "^info threads\r\n" { + exp_continue + } + + -re "^\\s+Id\\s+Target Id\\s+Frame\\s*\r\n" { + exp_continue + } + + -re "^\\s+2\\.1\\s+\[^\r\n\]+\\s+\\(running\\)\\s*\r\n" { + # Don't count this as seeing inferior 2 as the thread is + # incorrectly still marked as running. By not setting the + # SAW_INFERIOR_2 flag this test will now fail. + exp_continue + } + + -re "^\\s+2\\.1\\s+\[^\r\n\]+\r\n" { + set saw_inferior_2 true + exp_continue + } + + -re "^\\*\\s+3\\.1\\s+\[^\r\n\]+\r\n" { + set saw_inferior_3 true + exp_continue + } + + -re "^$::gdb_prompt $" { + gdb_assert { $saw_inferior_2 && $saw_inferior_3 } \ + $gdb_test_name + } + + -re "^\[^\r\n\]*\r\n" { + exp_continue + } + } +} + +set all_target_types { extended-remote native } + +foreach_with_prefix target_type_1 $all_target_types { + foreach_with_prefix target_type_2 $all_target_types { + run_test $target_type_1 $target_type_2 + } +} + +multi_target_cleanup