[PATCHv2] gdb/python: avoid switching threads for send_packet where possible

[PATCHv2] gdb/python: avoid switching threads for send_packet where possible

[Andrew Burgess]

Bug PR gdb/32120 identifies a case where a user tried to write a
custom frame unwinder that made use of
RemoteTargetConnection.send_packet, however, this was triggering an
assert like this:

  value.c:3954: internal-error: fetch_lazy_register: Assertion `next_frame != NULL' failed.

The implementation of RemoteTargetConnection.send_packet can be found
in the function `connpy_send_packet` in python/py-connection.c.  This
function includes this code:

  scoped_restore_current_thread restore_thread;
  switch_to_target_no_thread (conn->target);

The purpose of this is to ensure that the currently selected inferior
has the required remote connection pushed on its target stack.  This
is necessary because `send_remote_packet` (see remote.c) still relies
on global state read from the currently selected inferior.

Switching inferior and thread like this, and then switching back
thanks to the scoped restore, will flush the frame cache.

Flushing the frame cache while attempting to build the backtrace is
not something that GDB currently supports, and it's not clear to me if
this can even be supported in the general case.

However, in this specific case there's really no need to flush the
frame cache.  The user's example unwinder does this:

  class TestCaseUnwinder(Unwinder):
    def __init__(self):
      super().__init__("TestCaseUnwinder")

    def __call__(self, pending_frame):
      gdb.selected_inferior().connection.send_packet("qtest.case")
      return None

Notice that we're calling send_packet on the connection of the
currently selected inferior.  As such, the switch_to_target_no_thread
will (usually) just be re-switching to the current inferior, and
clearing the currently selected thread.  The clearing the thread bit
isn't really important, the only important bit is that the current
inferior is set correctly.

I propose adding a new function maybe_switch_to_target.  Similar in
spirit to maybe_switch_inferior, this new function checks if the
required target is already present on the current inferior's target
stack.  If it is, then no switch is performed.  If the target is not
present then switch_to_target_no_thread is used to switch to an
inferior that has the required target on its target stack.  We can
then use this new function in connpy_send_packet to avoid switching
inferior in the case where this is not necessary.

There are two tests.  First gdb.python/py-send-packet-unwinder.exp is
a new test the checks the specific case the user was reporting.

Then there is an update to gdb.python/py-send-packet.exp.  Now the
test runs first with a single inferior and performs some send_packet
checks, then the test creates a second inferior and makes that the
current inferior, and checks that we can still perform send_packet
calls from a Python variable containing the cached remote connection.
This is checking that the switch_to_target_no_thread call is still
triggering when needed.

While working on gdb.python/py-send-packet.py I spotted a couple of
places where we had:

  raise "some string"

which isn't correct, I replaced this with:

  raise gdb.GdbError("some string")

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32120
---
 gdb/process-stratum-target.c                  |  15 ++
 gdb/process-stratum-target.h                  |  15 ++
 gdb/python/py-connection.c                    |   6 +-
 .../gdb.python/py-send-packet-unwinder.c      |  47 ++++++
 .../gdb.python/py-send-packet-unwinder.exp    |  74 +++++++++
 .../gdb.python/py-send-packet-unwinder.py     |  46 ++++++
 gdb/testsuite/gdb.python/py-send-packet.exp   | 146 ++++++++++++------
 gdb/testsuite/gdb.python/py-send-packet.py    |  39 +++--
 8 files changed, 317 insertions(+), 71 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-send-packet-unwinder.c
 create mode 100644 gdb/testsuite/gdb.python/py-send-packet-unwinder.exp
 create mode 100644 gdb/testsuite/gdb.python/py-send-packet-unwinder.py

diff --git a/gdb/process-stratum-target.c b/gdb/process-stratum-target.c
index 6a205b31eee..36c1fdf3dc8 100644
--- a/gdb/process-stratum-target.c
+++ b/gdb/process-stratum-target.c
@@ -219,3 +219,18 @@ switch_to_target_no_thread (process_stratum_target *target)
       break;
     }
 }
+
+/* See process-stratum-target.h.  */
+
+std::optional<scoped_restore_current_thread>
+maybe_switch_to_target (process_stratum_target *target)
+{
+  std::optional<scoped_restore_current_thread> maybe_restore_thread;
+  if (!current_inferior ()->target_is_pushed (target))
+    {
+      maybe_restore_thread.emplace ();
+      switch_to_target_no_thread (target);
+    }
+
+  return maybe_restore_thread;
+}
diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h
index 9e987710daa..2afbc002b18 100644
--- a/gdb/process-stratum-target.h
+++ b/gdb/process-stratum-target.h
@@ -178,4 +178,19 @@ extern gdb::unordered_set<process_stratum_target *>
 
 extern void switch_to_target_no_thread (process_stratum_target *target);
 
+/* If TARGET is present on the target stack of the current inferior then
+   nothing changes and an empty optional is returned.  If TARGET is not on
+   the current inferior's target stack then switch to an inferior which
+   does have TARGET on its target stack, no thread is selected.
+
+   Be careful, in the case where TARGET is already on the target stack
+   nothing changes, the current inferior and current thread are left
+   unchanged.  But if we need to switch inferior in order to ensure TARGET
+   is on the target stack, we switch inferior, but no thread is selected.
+   As a result, after calling this function, there may, or may not, be a
+   currently selected thread.  */
+
+extern std::optional<scoped_restore_current_thread>
+     maybe_switch_to_target (process_stratum_target *target);
+
 #endif /* GDB_PROCESS_STRATUM_TARGET_H */
diff --git a/gdb/python/py-connection.c b/gdb/python/py-connection.c
index 02330b28068..519d48a6225 100644
--- a/gdb/python/py-connection.c
+++ b/gdb/python/py-connection.c
@@ -404,8 +404,10 @@ connpy_send_packet (PyObject *self, PyObject *args, PyObject *kw)
 
   try
     {
-      scoped_restore_current_thread restore_thread;
-      switch_to_target_no_thread (conn->target);
+      /* If CONN->TARGET is not already present on the current inferior's
+	 target stack, the switch to an inferior were it is.  If we switch
+	 then no new thread will be selected.  */
+      auto restore_thread = maybe_switch_to_target (conn->target);
 
       gdb::array_view<const char> view (packet_str, packet_len);
       py_send_packet_callbacks callbacks;
diff --git a/gdb/testsuite/gdb.python/py-send-packet-unwinder.c b/gdb/testsuite/gdb.python/py-send-packet-unwinder.c
new file mode 100644
index 00000000000..e9a09d50c66
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-send-packet-unwinder.c
@@ -0,0 +1,47 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2026 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 <http://www.gnu.org/licenses/>.  */
+
+void
+breakpt ()
+{
+  /* Nothing.  */
+}
+
+void
+foo ()
+{
+  breakpt ();
+}
+
+void
+bar ()
+{
+  foo ();
+}
+
+void
+baz ()
+{
+  bar ();
+}
+
+int
+main (void)
+{
+  baz ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-send-packet-unwinder.exp b/gdb/testsuite/gdb.python/py-send-packet-unwinder.exp
new file mode 100644
index 00000000000..920eac1c6be
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-send-packet-unwinder.exp
@@ -0,0 +1,74 @@
+# Copyright (C) 2026 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 <http://www.gnu.org/licenses/>.
+
+# Test for a specific use case of RemoteTargetConnection.send_packet
+# that we'd like to support.  In general the send_packet method will
+# cause GDB to switch inferior in order to ensure that the remote
+# target connection is on the target stack of the current inferior.
+#
+# The problem is that this causes the currently selected frame to
+# change, which in turn causes the frame cache to be flushed.
+#
+# If a user wants to make send_packet calls from a custom frame
+# unwinder then this is not possible as we end up flushing the frame
+# cache while trying to build the backtrace.
+#
+# The solution is that we have a special case, if when the send_packet
+# call is made, the currently selected inferior already has the
+# required remote target connection on its target stack, then it is
+# not necessary to change the currently selected inferior or thread.
+#
+# By not switching we avoid the frame cache flush, and so calling
+# send_packet from a frame unwinder is supported, so long as we only
+# send the packet to the currently selected inferior's remote target.
+
+load_lib gdb-python.exp
+load_lib gdbserver-support.exp
+
+standard_testfile
+
+require allow_gdbserver_tests allow_python_tests
+
+if { [build_executable "failed to build" ${testfile} ${srcfile}] } {
+    return
+}
+
+set remote_python_file [gdb_remote_download host \
+			    ${srcdir}/${subdir}/${testfile}.py]
+
+clean_restart $testfile
+
+# Make sure we're disconnected, in case we're testing with an
+# extended-remote board, therefore already connected.
+gdb_test "disconnect" ".*"
+
+gdbserver_run ""
+
+gdb_breakpoint "breakpt"
+gdb_continue_to_breakpoint "breakpt"
+
+gdb_test "source $remote_python_file" "Sourcing complete\\." \
+    "source ${testfile}.py script"
+
+gdb_test "bt" \
+    [multi_line \
+	 "#0  (?:$hex in )?breakpt \\(\\) at \[^\r\n\]+" \
+	 "#1  (?:$hex in )?foo \\(\\) at \[^\r\n\]+" \
+	 "#2  (?:$hex in )?bar \\(\\) at \[^\r\n\]+" \
+	 "#3  (?:$hex in )?baz \\(\\) at \[^\r\n\]+" \
+	 "#4  (?:$hex in )?main \\(\\) at \[^\r\n\]+"]
+
+# Check that the custom unwinder was actually called.
+gdb_test "python print(TestUnwinder.call_count > 0)" "True" 
diff --git a/gdb/testsuite/gdb.python/py-send-packet-unwinder.py b/gdb/testsuite/gdb.python/py-send-packet-unwinder.py
new file mode 100644
index 00000000000..05063e3c2f0
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-send-packet-unwinder.py
@@ -0,0 +1,46 @@
+# Copyright (C) 2026 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 <http://www.gnu.org/licenses/>.
+
+import gdb
+from gdb.unwinder import Unwinder
+
+
+# Count the number of times TestUnwinder.__call__ is called.
+TestUnwinder_Call_Count = 0
+
+
+# A frame unwinder that never claims any frame, but does send a packet
+# when sniffing the frame.  As the packet is only sent to the
+# currently selected inferior then we never need to switch thread to
+# send the packet, and so we should never need to flush the frame
+# cache.
+class TestUnwinder(Unwinder):
+    def __init__(self):
+        Unwinder.__init__(self, "send packet unwinder")
+
+    call_count = 0
+
+    def __call__(self, pending_frame):
+        TestUnwinder.call_count += 1
+
+        gdb.selected_inferior().connection.send_packet("vMustReplyEmpty")
+
+        # This unwinder never claims any frames.
+        return None
+
+
+gdb.unwinder.register_unwinder(None, TestUnwinder(), True)
+
+print("Sourcing complete.")
diff --git a/gdb/testsuite/gdb.python/py-send-packet.exp b/gdb/testsuite/gdb.python/py-send-packet.exp
index 4464c1c5123..8296b867ae2 100644
--- a/gdb/testsuite/gdb.python/py-send-packet.exp
+++ b/gdb/testsuite/gdb.python/py-send-packet.exp
@@ -26,69 +26,117 @@ standard_testfile
 
 require allow_gdbserver_tests allow_python_tests
 
-if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+if { [build_executable "failed to build" ${testfile} ${srcfile}] } {
     return
 }
 
-# Make sure we're disconnected, in case we're testing with an
-# extended-remote board, therefore already connected.
-gdb_test "disconnect" ".*"
-
-gdbserver_run ""
-
-gdb_breakpoint "breakpt"
-gdb_continue_to_breakpoint "breakpt"
-
-# Source the python script.
 set remote_python_file [gdb_remote_download host \
 			    ${srcdir}/${subdir}/${testfile}.py]
-gdb_test "source $remote_python_file" "Sourcing complete\\." \
-    "source ${testfile}.py script"
 
-# The test is actually written in the Python script.  Run it now.
-gdb_test "python run_send_packet_test()" "Send packet test passed"
+# Start GDB debugging the global TESTFILE.  Exercise the
+# RemoteTargetConnection.send_packet API to read the auxv data for the
+# inferior, the thread list for the inferior, and to set a global
+# within the inferior.
+#
+# When MULTI_INFERIOR is true a second inferior is started and
+# selected; an inferior with no remote connection.  In this case we
+# are checking that calling RemoteTargetConnection.send_packet on a
+# stored RemoteTargetConnection object works as expected.
+proc run_test { multi_inferior } {
+    clean_restart $::testfile
 
-# Check the string representation of a remote target connection.
-gdb_test "python print(gdb.selected_inferior().connection)" \
-    "<gdb.RemoteTargetConnection num=$decimal, what=\".*\">"
+    # Make sure we're disconnected, in case we're testing with an
+    # extended-remote board, therefore already connected.
+    gdb_test "disconnect" ".*"
 
-# Check to see if there's any auxv data for this target.
-gdb_test_multiple "info auxv" "" {
-    -re -wrap "The program has no auxiliary information now\\. " {
-	set skip_auxv_test true
-    }
-    -re -wrap "0\\s+AT_NULL\\s+End of vector\\s+0x0" {
-	set skip_auxv_test false
-    }
-}
+    gdbserver_run ""
 
-if { ! $skip_auxv_test } {
-    # Use 'maint packet' to fetch the auxv data.
-    set reply_data ""
-    gdb_test_multiple "maint packet qXfer:auxv:read::0,1000" "" {
-	-re "sending: \"qXfer:auxv:read::0,1000\"\r\n" {
-	    exp_continue
+    gdb_breakpoint "breakpt"
+    gdb_continue_to_breakpoint "breakpt"
+
+    # Source the python script.
+    gdb_test "source $::remote_python_file" "Sourcing complete\\." \
+	"source ${::testfile}.py script"
+
+    # Check to see if there's any auxv data for this target.
+    set skip_auxv_test false
+    gdb_test_multiple "info auxv" "" {
+	-re -wrap "The program has no auxiliary information now\\." {
+	    set skip_auxv_test true
 	}
-	-re -wrap "received: \"(.*)\"" {
-	    set reply_data $expect_out(1,string)
+	-re -wrap "0\\s+AT_NULL\\s+End of vector\\s+0x0" {
+	    set skip_auxv_test false
 	}
     }
 
-    # Escape any backslash characters in the output, so we can safely
-    # pass a string through to Python.
-    set reply_data [string map {\\ \\\\} $reply_data]
-    gdb_assert { ![string equal "$reply_data" ""] }
+    set auxv_reply_data ""
+    if { ! $skip_auxv_test } {
+	# Use 'maint packet' to collect the auxv data.  This only
+	# works on the current inferior, so needs to be done before we
+	# might add a new inferior.
+	gdb_test_multiple "maint packet qXfer:auxv:read::0,1000" "" {
+	    -re "sending: \"qXfer:auxv:read::0,1000\"\r\n" {
+		exp_continue
+	    }
+	    -re -wrap "received: \"(.*)\"" {
+		set auxv_reply_data $expect_out(1,string)
+	    }
+	}
 
-    # Run the test, fetches the auxv data in Python and confirm it
-    # matches the expected results.
-    gdb_test "python run_auxv_send_packet_test(\"$reply_data\")" \
-	"auxv send packet test passed" \
-	"call python run_auxv_send_packet_test function"
+	# Escape any backslash characters in the output, so we can safely
+	# pass a string through to Python.
+	set auxv_reply_data [string map {\\ \\\\} $auxv_reply_data]
+	gdb_assert { ![string equal "$auxv_reply_data" ""] } \
+	    "have got some auxv data"
+    }
+
+    # If the test requires it, add a second inferior, and make it the
+    # current inferior.
+    if { $multi_inferior } {
+	gdb_test_no_output "python inf=gdb.selected_inferior()" \
+	    "capture inferior 1"
+	set inferior "inf"
+
+	gdb_test "add-inferior -no-connection" ".*"
+	gdb_test "inferior 2" ".*"
+    } else {
+	set inferior "gdb.selected_inferior()"
+    }
+
+    # The test is actually written in the Python script.  Run it now.
+    gdb_test "python run_send_packet_test($inferior)" "Send packet test passed"
+
+    # Check the string representation of a remote target connection.
+    gdb_test "python print($inferior.connection)" \
+	"<gdb.RemoteTargetConnection num=$::decimal, what=\".*\">"
+
+    if { ! $skip_auxv_test } {
+	# Run the test, fetches the auxv data in Python and confirm it
+	# matches the expected results.
+	gdb_test "python run_auxv_send_packet_test($inferior, \"$auxv_reply_data\")" \
+	    "auxv send packet test passed" \
+	    "call python run_auxv_send_packet_test function"
+    }
+
+    # The final test relies on gdb.parse_and_eval, so needs the
+    # correct inferior selected.
+    if { $multi_inferior } {
+	gdb_test "inferior 1" ".*" "return to remote inferior"
+    }
+
+    set sizeof_global_var [get_valueof "/d" "sizeof(global_var)" "UNKNOWN"]
+    if { $sizeof_global_var == 4 } {
+	gdb_test_no_output "set debug remote 1"
+	gdb_test "python run_set_global_var_test()" \
+	    "set global_var test passed"
+    }
 }
 
-set sizeof_global_var [get_valueof "/d" "sizeof(global_var)" "UNKNOWN"]
-if { $sizeof_global_var == 4 } {
-    gdb_test_no_output "set debug remote 1"
-    gdb_test "python run_set_global_var_test()" \
-	"set global_var test passed"
+set modes { false }
+if {[allow_multi_inferior_tests]} {
+    lappend modes true
+}
+
+foreach_with_prefix multi_inferior $modes {
+    run_test $multi_inferior
 }
diff --git a/gdb/testsuite/gdb.python/py-send-packet.py b/gdb/testsuite/gdb.python/py-send-packet.py
index 46be26cac1b..ac66ed2acbe 100644
--- a/gdb/testsuite/gdb.python/py-send-packet.py
+++ b/gdb/testsuite/gdb.python/py-send-packet.py
@@ -19,7 +19,7 @@ import gdb
 
 
 # Make use of gdb.RemoteTargetConnection.send_packet to fetch the
-# thread list from the remote target.
+# thread list from the remote target for inferior INF.
 #
 # Sending existing serial protocol packets like this is not a good
 # idea, there should be better ways to get this information using an
@@ -28,10 +28,10 @@ import gdb
 # Really, the send_packet API would be used to send target
 # specific packets to the target, but these are, by definition, target
 # specific, so hard to test in a general testsuite.
-def get_thread_list_str():
+def get_thread_list_str(inf):
     start_pos = 0
     thread_desc = ""
-    conn = gdb.selected_inferior().connection
+    conn = inf.connection
     if not isinstance(conn, gdb.RemoteTargetConnection):
         raise gdb.GdbError("connection is the wrong type")
     while True:
@@ -46,19 +46,19 @@ def get_thread_list_str():
 
 
 # Use gdb.RemoteTargetConnection.send_packet to manually fetch the
-# thread list, then extract the thread list using the gdb.Inferior and
-# gdb.InferiorThread API.  Compare the two results to ensure we
-# managed to successfully read the thread list from the remote.
-def run_send_packet_test():
+# thread list for inferior INF.  Then extract the thread list using
+# the gdb.Inferior and gdb.InferiorThread API.  Compare the two
+# results to ensure we managed to successfully read the thread list
+# from the remote.
+def run_send_packet_test(inf):
     # Find the IDs of all current threads.
     all_threads = {}
-    for inf in gdb.inferiors():
-        for thr in inf.threads():
-            id = "p%x.%x" % (thr.ptid[0], thr.ptid[1])
-            all_threads[id] = False
+    for thr in inf.threads():
+        id = "p%x.%x" % (thr.ptid[0], thr.ptid[1])
+        all_threads[id] = False
 
     # Now fetch the thread list from the remote, and parse the XML.
-    str = get_thread_list_str()
+    str = get_thread_list_str(inf)
     threads_xml = ET.fromstring(str)
 
     # Look over all threads in the XML list and check we expected to
@@ -66,14 +66,14 @@ def run_send_packet_test():
     for thr in threads_xml:
         id = thr.get("id")
         if id not in all_threads:
-            raise "found unexpected thread in remote thread list"
+            raise gdb.GdbError("found unexpected thread in remote thread list")
         else:
             all_threads[id] = True
 
     # Check that all the threads were found in the XML list.
     for id in all_threads:
         if not all_threads[id]:
-            raise "thread missingt from remote thread list"
+            raise gdb.GdbError("thread missing from remote thread list")
 
     # Test complete.
     print("Send packet test passed")
@@ -93,12 +93,11 @@ def bytes_to_string(byte_array):
     return res
 
 
-# A very simple test for sending the packet that reads the auxv data.
-# We convert the result to a string and expect to find some
-# hex-encoded bytes in the output.  This test will only work on
-# targets that actually supply auxv data.
-def run_auxv_send_packet_test(expected_result):
-    inf = gdb.selected_inferior()
+# A very simple test for sending the packet that reads the auxv data
+# for inferior INF.  We convert the result to a string and expect to
+# find some hex-encoded bytes in the output.  This test will only work
+# on targets that actually supply auxv data.
+def run_auxv_send_packet_test(inf, expected_result):
     conn = inf.connection
     assert isinstance(conn, gdb.RemoteTargetConnection)
     res = conn.send_packet("qXfer:auxv:read::0,1000")

base-commit: adbfdef7131011d1ec85060128a2467c4f18fe0a
-- 
2.25.4