diff --git a/gdb/infrun.c b/gdb/infrun.c index 82e7284065b..2e59af2a735 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -9582,7 +9582,38 @@ normal_stop () update_thread_list (); if (last.kind () == TARGET_WAITKIND_STOPPED && stopped_by_random_signal) - notify_signal_received (inferior_thread ()->stop_signal ()); + { + gdb_assert (inferior_ptid != null_ptid); + + /* Calling update_thread_list pulls information from the target. For + native targets we can be (reasonably) sure that the information we + get back is sane, but for remote targets, we cannot reply on the + returned thread list to be correct. + + Specifically, a remote target (not gdbserver), has been seen to + prematurely remove threads from the thread list after sending a + signal stop event. The consequence of this, is that the thread + might now be exited. This is bad as, trying to calling + notify_signal_received will cause GDB to read registers for the + current thread, but requesting the regcache for an exited thread + will trigger an assertion. + + Check for the exited thread case here, and convert the stop reason + to a spurious stop event. The thread exiting will have already + been reported (when the thread list was parsed), so making this a + spurious stop will cause GDB to drop back to the prompt. */ + if (inferior_thread ()->state != THREAD_EXITED) + notify_signal_received (inferior_thread ()->stop_signal ()); + else + { + warning (_("command aborted, %s unexpectedly exited after signal stop event"), + target_pid_to_str (inferior_thread ()->ptid).c_str ()); + + /* Mark this as a spurious stop. GDB will return to the + prompt. The warning above tells the user why. */ + last.set_spurious (); + } + } /* As with the notification of thread events, we want to delay notifying the user that we've switched thread context until diff --git a/gdb/testsuite/gdb.replay/missing-thread.c b/gdb/testsuite/gdb.replay/missing-thread.c new file mode 100644 index 00000000000..8edb2402b26 --- /dev/null +++ b/gdb/testsuite/gdb.replay/missing-thread.c @@ -0,0 +1,61 @@ +/* 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 +#include +#include +#include +#include +#include + +pthread_mutex_t g_mutex = PTHREAD_MUTEX_INITIALIZER; +pthread_cond_t g_condvar = PTHREAD_COND_INITIALIZER; + +void * +worker_function (void *arg) +{ + printf ("In worker, about to notify\n"); + pthread_cond_signal (&g_condvar); + + while (true) + sleep(1); + + return NULL; +} + +int +main() +{ + pthread_t my_thread; + + int result = pthread_create (&my_thread, NULL, worker_function, NULL); + assert (result == 0); + + pthread_mutex_lock (&g_mutex); + pthread_cond_wait (&g_condvar, &g_mutex); + + printf ("In main, have been woken.\n"); + pthread_mutex_unlock (&g_mutex); + + result = pthread_kill (my_thread, SIGTRAP); + assert (result == 0); + + result = pthread_join (my_thread, NULL); + assert (result == 0); + + return 0; +} diff --git a/gdb/testsuite/gdb.replay/missing-thread.exp b/gdb/testsuite/gdb.replay/missing-thread.exp new file mode 100644 index 00000000000..15a8aaed2e4 --- /dev/null +++ b/gdb/testsuite/gdb.replay/missing-thread.exp @@ -0,0 +1,237 @@ +# 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 . */ + +# This test confirms how GDB handles a badly behaving remote target. The +# remote target reports a stop event (signal delivery), then, as GDB is +# processing the stop it syncs the thread list with the remote. +# +# The badly behaving remote target was dropping the signaled thread from the +# thread list at this point, that is, the thread appeared to exit before an +# exit event had been sent to (and seen by) GDB. +# +# At one point this was causing an assertion failed. GDB would try to +# process the signal stop event, and to do this would try to read some +# registers. Reading registers requires a regcache, and GDB will only +# create a regcache for a non-exited thread. + +load_lib gdbserver-support.exp +load_lib gdbreplay-support.exp + +require allow_gdbserver_tests +require has_gdbreplay + +standard_testfile + +if { [build_executable "failed to build exec" $testfile $srcfile {debug pthreads}] } { + return -1 +} + +# Start the inferior and record a remote log for our interaction with it. +# All we do is start the inferior and wait for thread 2 to receive a signal. +# Check that GDB correctly shows the signal as received. LOG_FILENAME is +# where we should write the remote log. +proc_with_prefix record_initial_logfile { log_filename } { + clean_restart $::testfile + + # Make sure we're disconnected, in case we're testing with an + # extended-remote board, therefore already connected. + gdb_test "disconnect" ".*" + + gdb_test_no_output "set sysroot" \ + "setting sysroot before starting gdbserver" + + # Start gdbserver like: + # gdbserver :PORT .... + set res [gdbserver_start "" $::binfile] + set gdbserver_protocol [lindex $res 0] + set gdbserver_gdbport [lindex $res 1] + + gdb_test_no_output "set remotelogfile $log_filename" \ + "setup remotelogfile" + + # Connect to gdbserver. + if {![gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport] == 0} { + unsupported "$testfile (couldn't start gdbserver)" + return + } + + gdb_breakpoint main + gdb_continue_to_breakpoint "continuing to main" + + gdb_test "continue" \ + "Thread $::decimal \[^\r\n\]+ received signal SIGTRAP, .*" + + gdb_test "disconnect" ".*" \ + "disconnect after seeing signal" +} + +# Copy the remote log from IN_FILENAME to OUT_FILENAME, but modify one +# particular line. +# +# The line to be modified is the last ... line, this is +# the reply from the remote that indicates the thread list. It is expected +# that the thread list will contain two threads. +# +# Whn DROP_BOTH is true then both threads will be removed from the modified +# line. Otherwise, only the second thread is removed. +proc update_replay_log { in_filename out_filename drop_both } { + # Read IN_FILENAME into a list. + set fd [open $in_filename] + set data [read $fd] + close $fd + set lines [split $data "\n"] + + # Find the last line in LINES that contains the list. + set idx -1 + for { set i 0 } { $i < [llength $lines] } { incr i } { + if { [regexp "^r.*.*" [lindex $lines $i]] } { + set idx $i + } + } + + # Modify the line by dropping the second thread. This does assume + # the thread order as seen in the ... list, but + # this seems stable for now. + set line [lindex $lines $idx] + set fixed_log false + if {[regexp "^(r .*\\\\n)(\\\\n)(\\\\n)(.*)$" $line \ + match part1 part2 part3 part4]} { + if { $drop_both } { + set line $part1$part4 + } else { + set line $part1$part2$part4 + } + set lines [lreplace $lines $idx $idx $line] + set fixed_log true + } + + # Write all the lines to OUT_FILENAME + set fd [open $out_filename "w"] + foreach l $lines { + puts $fd $l + } + close $fd + + # Did we manage to update the log file? + return $fixed_log +} + +# Replay the test process using REMOTE_LOG as the logfile to replay. If +# EXPECT_ERROR is true then after the final 'continue' we expect GDB to give +# an error as the required thread is missing. When EXPECT_ERROR is false +# then we expect the test to complete as normal. NON_STOP is eithe 'on' or +# 'off' and indicates GDBs non-stop mode. +proc_with_prefix replay_with_log { remote_log expect_error non_stop } { + clean_restart $::testfile + + # Make sure we're disconnected, in case we're testing with an + # extended-remote board, therefore already connected. + gdb_test "disconnect" ".*" + + gdb_test_no_output "set sysroot" + + set res [gdbreplay_start $remote_log] + set gdbserver_protocol [lindex $res 0] + set gdbserver_gdbport [lindex $res 1] + + # Connect to gdbserver. + if {![gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport] == 0} { + fail "couldn't connect to gdbreplay" + return + } + + gdb_breakpoint main + gdb_continue_to_breakpoint "continuing to main" + + if { $expect_error } { + set expected_output \ + [list \ + "\\\[Thread \[^\r\n\]+ exited\\\]" \ + "warning: command aborted, Thread \[^\r\n\]+ unexpectedly exited after signal stop event"] + + if { !$non_stop } { + lappend expected_output "\\\[Switching to Thread \[^\r\n\]+\\\]" + } + + gdb_test "continue" [multi_line {*}$expected_output] + } else { + # This is the original behaviour, we see this when running + # with the unmodified log. + gdb_test "continue" \ + "Thread ${::decimal}(?: \[^\r\n\]+)? received signal SIGTRAP, .*" + } + + gdb_test "disconnect" ".*" \ + "disconnect after seeing signal" +} + +# Run the complete test cycle; generate an initial log file, modify the log +# file, then check that GDB correctly handles replaying the modified log +# file. +# +# NON_STOP is either 'on' or 'off' and indicates GDB's non-stop mode. +proc run_test { non_stop } { + if { $non_stop } { + set suffix "-ns" + } else { + set suffix "" + } + + # The replay log is placed in 'replay.log'. + set remote_log [standard_output_file replay${suffix}.log] + set missing_1_log [standard_output_file replay-missing-1${suffix}.log] + set missing_2_log [standard_output_file replay-missing-2${suffix}.log] + + record_initial_logfile $remote_log + + if { ![update_replay_log $remote_log $missing_1_log false] } { + fail "couldn't update remote replay log (drop 1 case)" + } + + if { ![update_replay_log $remote_log $missing_2_log true] } { + fail "couldn't update remote replay log (drop 2 case)" + } + + with_test_prefix "with unmodified log" { + # Replay with the unmodified log. This confirms that we can replay this + # scenario correctly. + replay_with_log $remote_log false $non_stop + } + + with_test_prefix "missing 1 thread log" { + # Now replay with the modified log, this time the thread that receives + # the event should be missing from the thread list, GDB will give an + # error when the inferior stops. + replay_with_log $missing_1_log true $non_stop + } + + with_test_prefix "missing 2 threads log" { + # When we drop both threads from the reply, GDB doesn't + # actually remove both threads from the inferior; an inferior must + # always have at least one thread. So in this case, as the primary + # thread is first, GDB drops this, then retains the second thread, which + # is the one we're stopping in, and so, we don't expect to see the error + # in this case. + replay_with_log $missing_2_log false $non_stop + } +} + +# Run the test twice, with non-stop on and off. +foreach_with_prefix non_stop { on off } { + save_vars { ::GDBFLAGS } { + append ::GDBFLAGS " -ex \"set non-stop $non_stop\"" + run_test $non_stop + } +}