gh-158539: Handle deep exception chains without changing debug offsets

Author: pablogsal

Include/internal/pycore_debug_offsets.h

@@ -113,13 +113,11 @@ typedef struct _Py_DebugOffsets {
         uint64_t gil_requested;
         uint64_t current_exception;
         uint64_t exc_state;
-        uint64_t exc_info;
     } thread_state;
 
     // Exception stack item offset
     struct {
         uint64_t exc_value;
-        uint64_t previous_item;
     } err_stackitem;
 
     // InterpreterFrame offset;
@@ -306,11 +304,9 @@ typedef struct _Py_DebugOffsets {
         .gil_requested = offsetof(PyThreadState, gil_requested), \
         .current_exception = offsetof(PyThreadState, current_exception), \
         .exc_state = offsetof(PyThreadState, exc_state), \
-        .exc_info = offsetof(PyThreadState, exc_info), \
     }, \
     .err_stackitem = { \
         .exc_value = offsetof(_PyErr_StackItem, exc_value), \
-        .previous_item = offsetof(_PyErr_StackItem, previous_item), \
     }, \
     .interpreter_frame = { \
         .size = sizeof(_PyInterpreterFrame), \

Lib/test/test_external_inspection.py

@@ -3046,23 +3046,6 @@ class TestExceptionDetectionScenarios(RemoteInspectionTestBase):
 
     4. finally_no_exception: Finally block with no exception raised
        -> Should NOT have HAS_EXCEPTION (no exception state)
-
-    5. except_block_in_generator: Thread inside an except block running in a
-       generator
-       -> SHOULD have HAS_EXCEPTION (exc_info points at the generator's
-          _PyErr_StackItem, not the thread's embedded exc_state)
-
-    6. except_block_in_genexpr_callee: Except block in a function called from a
-       generator expression
-       -> SHOULD have HAS_EXCEPTION (same reason as 5)
-
-    7. except_block_in_coroutine: Thread inside an except block running in a
-       coroutine
-       -> SHOULD have HAS_EXCEPTION (same reason as 5)
-
-    8. except_block_in_callee_from_coroutine: Except block in a function called
-       from a coroutine
-       -> SHOULD have HAS_EXCEPTION (same reason as 5)
     """
 
     def _make_single_scenario_script(self, port, scenario):
@@ -3146,105 +3129,6 @@ def target_thread():
         while True:
             time.sleep(0.01)
 
-t = threading.Thread(target=target_thread)
-t.start()
-t.join()
-""",
-            "except_block_in_generator": f"""\
-import socket
-import threading
-import time
-
-def target_thread():
-    '''Inside except block that runs in a generator'''
-    conn = socket.create_connection(("localhost", {port}))
-    conn.sendall(b"ready:" + str(threading.get_native_id()).encode())
-
-    def gen():
-        try:
-            raise ValueError("test")
-        except ValueError:
-            while True:
-                time.sleep(0.01)
-        yield
-
-    for _ in gen():
-        pass
-
-t = threading.Thread(target=target_thread)
-t.start()
-t.join()
-""",
-            "except_block_in_genexpr_callee": f"""\
-import socket
-import threading
-import time
-
-def target_thread():
-    '''Inside except block in a function called from a generator expression'''
-    conn = socket.create_connection(("localhost", {port}))
-    conn.sendall(b"ready:" + str(threading.get_native_id()).encode())
-
-    def callee():
-        try:
-            raise ValueError("test")
-        except ValueError:
-            while True:
-                time.sleep(0.01)
-
-    list(callee() for _ in range(1))
-
-t = threading.Thread(target=target_thread)
-t.start()
-t.join()
-""",
-            "except_block_in_coroutine": f"""\
-import asyncio
-import socket
-import threading
-import time
-
-def target_thread():
-    '''Inside except block that runs in a coroutine'''
-    conn = socket.create_connection(("localhost", {port}))
-    conn.sendall(b"ready:" + str(threading.get_native_id()).encode())
-
-    async def coro():
-        try:
-            raise ValueError("test")
-        except ValueError:
-            while True:
-                time.sleep(0.01)
-
-    asyncio.run(coro())
-
-t = threading.Thread(target=target_thread)
-t.start()
-t.join()
-""",
-            "except_block_in_callee_from_coroutine": f"""\
-import asyncio
-import socket
-import threading
-import time
-
-def target_thread():
-    '''Inside except block in a function called from a coroutine'''
-    conn = socket.create_connection(("localhost", {port}))
-    conn.sendall(b"ready:" + str(threading.get_native_id()).encode())
-
-    def callee():
-        try:
-            raise ValueError("test")
-        except ValueError:
-            while True:
-                time.sleep(0.01)
-
-    async def coro():
-        callee()
-
-    asyncio.run(coro())
-
 t = threading.Thread(target=target_thread)
 t.start()
 t.join()
@@ -3408,72 +3292,6 @@ def test_finally_no_exception_no_flag(self):
             self.assertIsNotNone(thread_tid, "Thread ID not received")
             self._check_exception_status(p, thread_tid, expect_exception=False)
 
-    @unittest.skipIf(
-        sys.platform not in ("linux", "darwin", "win32"),
-        "Test only runs on supported platforms (Linux, macOS, or Windows)",
-    )
-    @unittest.skipIf(
-        sys.platform == "android", "Android raises Linux-specific exception"
-    )
-    def test_except_block_in_generator_has_exception(self):
-        """gh-158539: a handler running in a generator has HAS_EXCEPTION.
-
-        Generators repoint ``tstate->exc_info`` at their own
-        ``_PyErr_StackItem``, so the embedded ``exc_state`` stays empty and the
-        profiler must follow ``exc_info`` to see the handled exception.
-        """
-        with self._run_scenario_process("except_block_in_generator") as (p, thread_tid):
-            self.assertIsNotNone(thread_tid, "Thread ID not received")
-            self._check_exception_status(p, thread_tid, expect_exception=True)
-
-    @unittest.skipIf(
-        sys.platform not in ("linux", "darwin", "win32"),
-        "Test only runs on supported platforms (Linux, macOS, or Windows)",
-    )
-    @unittest.skipIf(
-        sys.platform == "android", "Android raises Linux-specific exception"
-    )
-    def test_except_block_in_genexpr_callee_has_exception(self):
-        """gh-158539: a handler in a function called from a generator expression.
-
-        The handler itself lives in an ordinary function, but the generator
-        expression on the stack means ``exc_info`` does not point at the
-        thread's embedded ``exc_state``.
-        """
-        with self._run_scenario_process(
-            "except_block_in_genexpr_callee"
-        ) as (p, thread_tid):
-            self.assertIsNotNone(thread_tid, "Thread ID not received")
-            self._check_exception_status(p, thread_tid, expect_exception=True)
-
-    @unittest.skipIf(
-        sys.platform not in ("linux", "darwin", "win32"),
-        "Test only runs on supported platforms (Linux, macOS, or Windows)",
-    )
-    @unittest.skipIf(
-        sys.platform == "android", "Android raises Linux-specific exception"
-    )
-    def test_except_block_in_coroutine_has_exception(self):
-        """gh-158539: a handler running in a coroutine has HAS_EXCEPTION."""
-        with self._run_scenario_process("except_block_in_coroutine") as (p, thread_tid):
-            self.assertIsNotNone(thread_tid, "Thread ID not received")
-            self._check_exception_status(p, thread_tid, expect_exception=True)
-
-    @unittest.skipIf(
-        sys.platform not in ("linux", "darwin", "win32"),
-        "Test only runs on supported platforms (Linux, macOS, or Windows)",
-    )
-    @unittest.skipIf(
-        sys.platform == "android", "Android raises Linux-specific exception"
-    )
-    def test_except_block_in_callee_from_coroutine_has_exception(self):
-        """gh-158539: a handler in a function called from a coroutine."""
-        with self._run_scenario_process(
-            "except_block_in_callee_from_coroutine"
-        ) as (p, thread_tid):
-            self.assertIsNotNone(thread_tid, "Thread ID not received")
-            self._check_exception_status(p, thread_tid, expect_exception=True)
-
 
 class TestExceptionDetectionInProcess(RemoteInspectionTestBase):
     """gh-158539: HAS_EXCEPTION for handlers running in generators/coroutines.
@@ -3488,7 +3306,7 @@ class TestExceptionDetectionInProcess(RemoteInspectionTestBase):
     def setUpClass(cls):
         try:
             RemoteUnwinder(os.getpid(), all_threads=True).get_stack_trace()
-        except Exception as exc:
+        except PermissionError as exc:
             raise unittest.SkipTest(f"self-inspection is unavailable: {exc}")
 
     def _check_running_handler(
@@ -3646,6 +3464,28 @@ def gen():
 
         self._check_running_handler(target, expect_exception=False)
 
+    def test_outer_handler_while_nested_generators_run(self):
+        def target(ready, stop):
+            def gen(depth):
+                if depth:
+                    yield from gen(depth - 1)
+                else:
+                    self._busy_until_stopped(ready, stop)
+                yield
+
+            try:
+                raise ValueError("outer")
+            except ValueError:
+                for _ in gen(32):
+                    pass
+
+        self._check_running_handler(
+            target,
+            expect_exception=True,
+            mode=PROFILING_MODE_EXCEPTION,
+            skip_non_matching_threads=True,
+        )
+
     def test_generator_finally_after_except(self):
         """The handled exception is cleared before the generator's finally."""
         def target(ready, stop):

Modules/_remote_debugging/debug_offsets_validation.h

@@ -31,7 +31,7 @@
 #define FIELD_SIZE(type, member) sizeof(((type *)0)->member)
 
 enum {
-    PY_REMOTE_DEBUG_OFFSETS_TOTAL_SIZE = 904,
+    PY_REMOTE_DEBUG_OFFSETS_TOTAL_SIZE = 888,
     PY_REMOTE_ASYNC_DEBUG_OFFSETS_TOTAL_SIZE = 104,
 };
 
@@ -249,15 +249,15 @@ validate_fixed_field(
 #define PY_REMOTE_DEBUG_RUNTIME_STATE_FIELDS(APPLY, buffer_size) \
     APPLY(runtime_state, interpreters_head, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size)
 
+/* current_exception also covers the adjacent exc_info pointer. */
 #define PY_REMOTE_DEBUG_THREAD_STATE_FIELDS(APPLY, buffer_size) \
     APPLY(thread_state, native_thread_id, sizeof(unsigned long), _Alignof(long), buffer_size); \
     APPLY(thread_state, interp, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
     APPLY(thread_state, datastack_chunk, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
     APPLY(thread_state, status, FIELD_SIZE(PyThreadState, _status), _Alignof(unsigned int), buffer_size); \
     APPLY(thread_state, holds_gil, sizeof(int), _Alignof(int), buffer_size); \
     APPLY(thread_state, gil_requested, sizeof(int), _Alignof(int), buffer_size); \
-    APPLY(thread_state, current_exception, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
-    APPLY(thread_state, exc_info, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
+    APPLY(thread_state, current_exception, 2 * sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
     APPLY(thread_state, thread_id, sizeof(unsigned long), _Alignof(long), buffer_size); \
     APPLY(thread_state, next, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
     APPLY(thread_state, current_frame, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
@@ -352,16 +352,11 @@ _PyRemoteDebug_ValidateDebugOffsetsLayout(struct _Py_DebugOffsets *debug_offsets
     PY_REMOTE_DEBUG_THREAD_STATE_FIELDS(
         PY_REMOTE_DEBUG_VALIDATE_FIELD,
         SIZEOF_THREAD_STATE);
+    /* exc_value also covers the adjacent previous_item pointer. */
     PY_REMOTE_DEBUG_VALIDATE_FIXED_FIELD(
         err_stackitem,
         exc_value,
-        sizeof(uintptr_t),
-        _Alignof(uintptr_t),
-        sizeof(_PyErr_StackItem));
-    PY_REMOTE_DEBUG_VALIDATE_FIXED_FIELD(
-        err_stackitem,
-        previous_item,
-        sizeof(uintptr_t),
+        2 * sizeof(uintptr_t),
         _Alignof(uintptr_t),
         sizeof(_PyErr_StackItem));
     PY_REMOTE_DEBUG_VALIDATE_NESTED_FIELD(

Modules/_remote_debugging/threads.c

@@ -17,10 +17,15 @@
 #include <sys/wait.h>
 #endif
 
-/* Upper bound on how far the handled-exception chain (exc_info->previous_item)
- * is followed in remote memory. The chain is normally at most a couple of
- * entries deep; the bound only guards against corrupted memory. */
-#define MAX_EXCEPTION_CHAIN_DEPTH 16
+/* Bound traversal of corrupted remote exception chains. */
+#define MAX_EXCEPTION_CHAIN_DEPTH (2 << 15)
+
+/* Derive unexported offsets from adjacent fields to keep the debug-offset
+ * table compatible across patch releases. */
+static_assert(offsetof(PyThreadState, exc_info) ==
+              offsetof(PyThreadState, current_exception) + sizeof(uintptr_t));
+static_assert(offsetof(_PyErr_StackItem, previous_item) ==
+              offsetof(_PyErr_StackItem, exc_value) + sizeof(uintptr_t));
 
 /* ============================================================================
  * THREAD ITERATION FUNCTIONS
@@ -441,25 +446,18 @@ unwind_stack_for_thread(
         has_exception = 1;
     }
 
-    // Check the exception currently being handled by an except block.
-    //
-    // The active _PyErr_StackItem is normally exc_state, embedded in the
-    // thread state, but generators, coroutines and async generators repoint
-    // tstate->exc_info at their own _PyErr_StackItem while they run. Reading
-    // only the embedded exc_state therefore misses every handler that runs in
-    // a generator or coroutine, or in a function one of them calls. Follow
-    // exc_info and walk previous_item like _PyErr_GetTopmostException() so
-    // that an outer handler is still found while a generator without a handler
-    // of its own is running.
+    // Generators and coroutines use their own exception stack items.
+    // Follow exc_info to find the innermost handler, as sys.exception() does.
     if (!has_exception) {
         uintptr_t exc_info = GET_MEMBER(uintptr_t, ts,
-            unwinder->debug_offsets.thread_state.exc_info);
+            unwinder->debug_offsets.thread_state.current_exception +
+            sizeof(uintptr_t));
         uintptr_t exc_state_addr =
             *current_tstate + unwinder->debug_offsets.thread_state.exc_state;
         uintptr_t exc_value_offset =
             unwinder->debug_offsets.err_stackitem.exc_value;
         uintptr_t previous_item_offset =
-            unwinder->debug_offsets.err_stackitem.previous_item;
+            exc_value_offset + sizeof(uintptr_t);
 
         for (int depth = 0; exc_info != 0 && depth < MAX_EXCEPTION_CHAIN_DEPTH;
              depth++)