[binutils-gdb] Sync thread state after infcalls with "set unwind-on-* on" (PR gdb/34148)

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4e51e5abc5c45332dfddd5a0434f3a4792e9c370

commit 4e51e5abc5c45332dfddd5a0434f3a4792e9c370
Author: Pedro Alves <[email protected]>
Date:   Mon May 18 14:20:00 2026 +0100

    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 <[email protected]>
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34148
    Change-Id: Idef0dcd4dd751b501869c58b752f77d4dadb6c72

Diff:
---
 gdb/infcall.c                              | 23 +++++++++++++++--------
 gdb/testsuite/gdb.base/infcall-timeout.exp | 13 +++++++++++++
 gdb/testsuite/gdb.base/unwindonsignal.exp  | 10 ++++++++++
 gdb/testsuite/gdb.cp/gdb2495.exp           |  6 ++++++
 4 files changed, 44 insertions(+), 8 deletions(-)

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<call_thread_fsm> 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<call_thread_fsm> 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 .*<function called from gdb>.*"
     }
+
+    # 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.*" \