GH-113486: Do not emit spurious PY_UNWIND events for optimized calls to classes. (GH-113680)

Author: markshannon

Include/cpython/code.h

@@ -208,6 +208,8 @@ struct PyCodeObject _PyCode_DEF(1);
 #define CO_FUTURE_GENERATOR_STOP  0x800000
 #define CO_FUTURE_ANNOTATIONS    0x1000000
 
+#define CO_NO_MONITORING_EVENTS 0x2000000
+
 /* This should be defined if a future statement modifies the syntax.
    For example, when a keyword is added.
 */

Lib/test/test_monitoring.py

@@ -750,7 +750,7 @@ class UnwindRecorder(ExceptionRecorder):
     event_type = E.PY_UNWIND
 
     def __call__(self, code, offset, exc):
-        self.events.append(("unwind", type(exc)))
+        self.events.append(("unwind", type(exc), code.co_name))
 
 class ExceptionHandledRecorder(ExceptionRecorder):
 
@@ -766,8 +766,27 @@ class ThrowRecorder(ExceptionRecorder):
     def __call__(self, code, offset, exc):
         self.events.append(("throw", type(exc)))
 
-class ExceptionMonitoringTest(CheckEvents):
+class CallRecorder:
+
+    event_type = E.CALL
+
+    def __init__(self, events):
+        self.events = events
+
+    def __call__(self, code, offset, func, arg):
+        self.events.append(("call", func.__name__, arg))
+
+class ReturnRecorder:
+
+    event_type = E.PY_RETURN
+
+    def __init__(self, events):
+        self.events = events
 
+    def __call__(self, code, offset, val):
+        self.events.append(("return", code.co_name, val))
+
+class ExceptionMonitoringTest(CheckEvents):
 
     exception_recorders = (
         ExceptionRecorder,
@@ -936,26 +955,48 @@ def func():
         )
         self.assertEqual(events[0], ("throw", IndexError))
 
-class LineRecorder:
+    def test_no_unwind_for_shim_frame(self):
 
-    event_type = E.LINE
+        class B:
+            def __init__(self):
+                raise ValueError()
+
+        def f():
+            try:
+                return B()
+            except ValueError:
+                pass
 
+        for _ in range(100):
+            f()
+        recorders = (
+            ReturnRecorder,
+            UnwindRecorder
+        )
+        events = self.get_events(f, TEST_TOOL, recorders)
+        adaptive_insts = dis.get_instructions(f, adaptive=True)
+        self.assertIn(
+            "CALL_ALLOC_AND_ENTER_INIT",
+            [i.opname for i in adaptive_insts]
+        )
+        #There should be only one unwind event
+        expected = [
+            ('unwind', ValueError, '__init__'),
+            ('return', 'f', None),
+        ]
 
-    def __init__(self, events):
-        self.events = events
+        self.assertEqual(events, expected)
 
-    def __call__(self, code, line):
-        self.events.append(("line", code.co_name, line - code.co_firstlineno))
+class LineRecorder:
 
-class CallRecorder:
+    event_type = E.LINE
 
-    event_type = E.CALL
 
     def __init__(self, events):
         self.events = events
 
-    def __call__(self, code, offset, func, arg):
-        self.events.append(("call", func.__name__, arg))
+    def __call__(self, code, line):
+        self.events.append(("line", code.co_name, line - code.co_firstlineno))
 
 class CEventRecorder:
 
@@ -1351,15 +1392,6 @@ class BranchRecorder(JumpRecorder):
     event_type = E.BRANCH
     name = "branch"
 
-class ReturnRecorder:
-
-    event_type = E.PY_RETURN
-
-    def __init__(self, events):
-        self.events = events
-
-    def __call__(self, code, offset, val):
-        self.events.append(("return", val))
 
 
 JUMP_AND_BRANCH_RECORDERS = JumpRecorder, BranchRecorder
@@ -1449,11 +1481,11 @@ def func():
             ('branch', 'func', 4, 4),
             ('line', 'func', 5),
             ('line', 'meth', 1),
-            ('return', None),
+            ('return', 'meth', None),
             ('jump', 'func', 5, 5),
             ('jump', 'func', 5, '[offset=114]'),
             ('branch', 'func', '[offset=120]', '[offset=124]'),
-            ('return', None),
+            ('return', 'func', None),
             ('line', 'get_events', 11)])
 
 class TestLoadSuperAttr(CheckEvents):

Misc/NEWS.d/next/Core and Builtins/2023-12-31-07-46-01.gh-issue-113486.uki19C.rst

@@ -0,0 +1 @@
+No longer issue spurious ``PY_UNWIND`` events for optimized calls to classes.

Python/ceval.c

@@ -2111,6 +2111,9 @@ do_monitor_exc(PyThreadState *tstate, _PyInterpreterFrame *frame,
                _Py_CODEUNIT *instr, int event)
 {
     assert(event < _PY_MONITORING_UNGROUPED_EVENTS);
+    if (_PyFrame_GetCode(frame)->co_flags & CO_NO_MONITORING_EVENTS) {
+        return 0;
+    }
     PyObject *exc = PyErr_GetRaisedException();
     assert(exc != NULL);
     int err = _Py_call_instrumentation_arg(tstate, event, frame, instr, exc);

Python/instrumentation.c

@@ -1576,13 +1576,11 @@ _Py_Instrument(PyCodeObject *code, PyInterpreterState *interp)
     }
     _Py_Executors_InvalidateDependency(interp, code);
     int code_len = (int)Py_SIZE(code);
-    /* code->_co_firsttraceable >= code_len indicates
-     * that no instrumentation can be inserted.
-     * Exit early to avoid creating instrumentation
+    /* Exit early to avoid creating instrumentation
      * data for potential statically allocated code
      * objects.
      * See https://github.com/python/cpython/issues/108390 */
-    if (code->_co_firsttraceable >= code_len) {
+    if (code->co_flags & CO_NO_MONITORING_EVENTS) {
         return 0;
     }
     if (update_instrumentation_data(code, interp)) {

Python/specialize.c

@@ -2534,7 +2534,7 @@ const struct _PyCode_DEF(8) _Py_InitCleanup = {
     .co_consts = (PyObject *)&_Py_SINGLETON(tuple_empty),
     .co_names = (PyObject *)&_Py_SINGLETON(tuple_empty),
     .co_exceptiontable = (PyObject *)&_Py_SINGLETON(bytes_empty),
-    .co_flags = CO_OPTIMIZED,
+    .co_flags = CO_OPTIMIZED | CO_NO_MONITORING_EVENTS,
     .co_localsplusnames = (PyObject *)&_Py_SINGLETON(tuple_empty),
     .co_localspluskinds = (PyObject *)&_Py_SINGLETON(bytes_empty),
     .co_filename = &_Py_ID(__init__),