From: Pedro Alves Date: Mon, 18 May 2026 13:20:00 +0000 (+0100) Subject: Sync thread state after infcalls with "set unwind-on-* on" (PR gdb/34148) X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=4e51e5abc5c45332dfddd5a0434f3a4792e9c370;p=thirdparty%2Fbinutils-gdb.git Sync thread state after infcalls with "set unwind-on-* on" (PR gdb/34148) Commit 519774805a1 ("Don't pretend infcalls don't set the inferior running (PR gdb/34082)") removed the special case in proceed that skipped set_state(THREAD_RUNNING) for infcalls. That fixed gdb.threads/hand-call-new-thread.exp, but introduced a regression in gdb.compile/compile.exp: ... set unwind-on-signal on (gdb) PASS: gdb.compile/compile.exp: set unwind-on-signal on compile code *(volatile int *) 0 = 0; The program being debugged received signal SIGSEGV, Segmentation fault while in a function called from GDB. GDB has restored the context to what it was before the call. To change this behavior use "set unwind-on-signal off". Evaluation of the expression containing the function (_gdb_expr) will be abandoned. (gdb) PASS: gdb.compile/compile.exp: compile code segfault second break 132 Breakpoint 2 at 0x555555555262: file .../compile.c, line 132. (gdb) continue Cannot execute this command while the selected thread is running. (gdb) FAIL: gdb.compile/compile.exp: continue to breakpoint: break-here The "compile code" command before the FAIL is an infcall under the hood. That hits SIGSEGV with "set unwind-on-signal on" in effect, so GDB unwinds and abandons the call. After that, "continue" is rejected because the thread is still marked THREAD_RUNNING from the proceed that started the infcall. When an infcall is unwound due to a signal, timeout, or terminating exception, call_thread_fsm::should_notify_stop returns false, and so normal_stop is not called from fetch_inferior_event. normal_stop is what would normally call finish_thread_state to sync the public thread state back to THREAD_STOPPED. run_inferior_call has a fallback finish_thread_state call for that purpose, but it is gated on stop_stack_dummy == STOP_STACK_DUMMY, which is only true for successful calls. Before the commit mentioned above, proceed never marked an infcall's thread as THREAD_RUNNING, so the missing RUNNING => STOPPED transition was harmless. The old comment in infcall.c about the finish_thread_state call claimed "If the infcall does NOT succeed, normal_stop will have already finished the thread states", but that was already incorrect for the unwind paths. It just happened to not matter. Fix this by dropping the STOP_STACK_DUMMY guard and updating the comment to describe the actual rule: sync regardless of how the call ended. The !was_running check is kept since it is there to exclude the in-cond-eval case, where the thread is meant to stay marked running. finish_thread_state is idempotent, so the call is harmless on paths where normal_stop also ran. Extend gdb.base/unwindonsignal.exp to exercise the "set unwind-on-signal on" path without having to rely on the "compile code" feature. Without the fix, the test fails like so: info threads Id Target Id Frame * 1 Thread 0x7ffff7f8f740 (LWP 239019) "unwindonsignal" (running) (gdb) FAIL: gdb.base/unwindonsignal.exp: thread is stopped continue Cannot execute this command while the selected thread is running. (gdb) FAIL: gdb.base/unwindonsignal.exp: continue until exit at after unwound infcall Similarly, extend gdb.cp/gdb2495.exp for "set unwind-on-terminating-exception on", and gdb.base/infcall-timeout.exp for "set unwind-on-timeout on". Both would fail without the code fix, too. With the fix, gdb.compile/compile.exp now passes cleanly. Tested on x86_64-unknown-linux-gnu. Approved-By: Andrew Burgess Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34148 Change-Id: Idef0dcd4dd751b501869c58b752f77d4dadb6c72 --- diff --git a/gdb/infcall.c b/gdb/infcall.c index 8b26f541de6..e6b24ff5310 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -918,12 +918,20 @@ run_inferior_call (std::unique_ptr sm, current_ui->register_file_handler (); } - /* If the infcall does NOT succeed, normal_stop will have already - finished the thread states. However, on success, normal_stop - defers here, so that we can set back the thread states to what - they were before the call. Note that we must also finish the - state of new threads that might have spawned while the call was - running. The main cases to handle are: + /* Sync the user/frontend thread states from the internal thread + states. proceed marked threads in resume_ptid as THREAD_RUNNING + for this infcall; we must now sync them back, regardless of how + the call ended. For the success path and for the unwind paths + (unwind-on-{signal,timeout,terminating-exception}), + call_thread_fsm::should_notify_stop returns false and normal_stop + is skipped -- this call is the canonical place to do the sync. + For other failure paths normal_stop does run and has already + finished the thread state; finish_thread_state is idempotent, so + calling it again here is harmless. Note that we must also finish + the state of new threads that might have spawned while the call + was running. + + The main cases to handle are: - "(gdb) print foo ()", or any other command that evaluates an expression at the prompt. (The thread was marked stopped before.) @@ -934,8 +942,7 @@ run_inferior_call (std::unique_ptr sm, evaluates true and thus we'll present a user-visible stop is decided elsewhere. */ if (!was_running - && call_thread_ptid == inferior_ptid - && stop_stack_dummy == STOP_STACK_DUMMY) + && call_thread_ptid == inferior_ptid) finish_thread_state (call_thread->inf->process_target (), user_visible_resume_ptid (0)); diff --git a/gdb/testsuite/gdb.base/infcall-timeout.exp b/gdb/testsuite/gdb.base/infcall-timeout.exp index 37aa6c0ef54..99d29624337 100644 --- a/gdb/testsuite/gdb.base/infcall-timeout.exp +++ b/gdb/testsuite/gdb.base/infcall-timeout.exp @@ -86,6 +86,19 @@ proc run_test { target_async target_non_stop non_stop unwind } { gdb_test "bt" \ ".* function_that_never_returns .*.*" } + + # After the infcall, the thread should be stopped. Regression + # test for PR gdb/34148. + if {$non_stop} { + # Check the main thread only, in case we have system-spawned + # threads. + set thread_filter "1" + } else { + set thread_filter "" + } + gdb_test "info threads -running $thread_filter" \ + "No threads matched\\." \ + "thread is stopped" } foreach_with_prefix target_async { "on" "off" } { diff --git a/gdb/testsuite/gdb.base/unwindonsignal.exp b/gdb/testsuite/gdb.base/unwindonsignal.exp index aed8ef6f4c7..2ae8cc4a6dd 100644 --- a/gdb/testsuite/gdb.base/unwindonsignal.exp +++ b/gdb/testsuite/gdb.base/unwindonsignal.exp @@ -86,3 +86,13 @@ gdb_test_multiple "maint print dummy-frames" \ pass $gdb_test_name } } + +# After the unwound infcall, the thread should be stopped, and a +# subsequent resumption command should be accepted. Regression test +# for PR gdb/34148. + +gdb_test "info threads -running" \ + "No threads matched\\." \ + "thread is stopped" + +gdb_continue_to_end "after unwound infcall" diff --git a/gdb/testsuite/gdb.cp/gdb2495.exp b/gdb/testsuite/gdb.cp/gdb2495.exp index aa5a2a16e43..60396494161 100644 --- a/gdb/testsuite/gdb.cp/gdb2495.exp +++ b/gdb/testsuite/gdb.cp/gdb2495.exp @@ -64,6 +64,12 @@ gdb_test "p exceptions.throw_function()" \ "The program being debugged entered a std::terminate call, .*" \ "call a function that raises an exception without a handler." +# Make sure that the thread is stopped. Regression test for PR +# gdb/34148. +gdb_test "info threads -running" \ + "No threads matched\\." \ + "thread is stopped" + # Make sure that after rewinding we are back at the call parent. gdb_test "bt" \ "#0 main.*" \