gh-103615: Use local events for opcode tracing (GH-109472)

* Use local monitoring for opcode trace

* Remove f_opcode_trace_set

* Add test for setting f_trace_opcodes after settrace

diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h
index 339ced3c87a43e8727cd6938eb1c1f913ae1fad1..c372b7224fb047b88d899924c2753c2cc7cf837c 100644 (file)
--- a/Include/internal/pycore_ceval.h
+++ b/Include/internal/pycore_ceval.h
@@ -22,6 +22,8 @@ PyAPI_FUNC(int) _PyEval_SetProfile(PyThreadState *tstate, Py_tracefunc func, PyO
 
 extern int _PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg);
 
+extern int _PyEval_SetOpcodeTrace(PyFrameObject *f, bool enable);
+
 // Helper to look up a builtin object
 // Export for 'array' shared extension
 PyAPI_FUNC(PyObject*) _PyEval_GetBuiltin(PyObject *);

diff --git a/Include/internal/pycore_instruments.h b/Include/internal/pycore_instruments.h
index 97dcfb9f8672f7f99ca97dd92cdd2279ecf7362b..eae8371ef7f9b8f19e5216b130fb7c88991bd47a 100644 (file)
--- a/Include/internal/pycore_instruments.h
+++ b/Include/internal/pycore_instruments.h
@@ -63,6 +63,8 @@ typedef uint32_t _PyMonitoringEventSet;
 PyObject *_PyMonitoring_RegisterCallback(int tool_id, int event_id, PyObject *obj);
 
 int _PyMonitoring_SetEvents(int tool_id, _PyMonitoringEventSet events);
+int _PyMonitoring_SetLocalEvents(PyCodeObject *code, int tool_id, _PyMonitoringEventSet events);
+int _PyMonitoring_GetLocalEvents(PyCodeObject *code, int tool_id, _PyMonitoringEventSet *events);
 
 extern int
 _Py_call_instrumentation(PyThreadState *tstate, int event,

diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
index 78b841afae937ea990c6db90ceae8f309a269ccf..498db8becf114cf24a32d98b8829057a03983e5a 100644 (file)
--- a/Include/internal/pycore_interp.h
+++ b/Include/internal/pycore_interp.h
@@ -200,7 +200,6 @@ struct _is {
     uint32_t next_func_version;
 
     _Py_GlobalMonitors monitors;
-    bool f_opcode_trace_set;
     bool sys_profile_initialized;
     bool sys_trace_initialized;
     Py_ssize_t sys_profiling_threads; /* Count of threads with c_profilefunc set */

diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py
index df7dd36274df2eb06d862f6326e7eb66bc21cc53..292096383bca6374246db1417db32222662d1969 100644 (file)
--- a/Lib/test/test_sys_settrace.py
+++ b/Lib/test/test_sys_settrace.py
@@ -9,6 +9,10 @@ from functools import wraps
 import asyncio
 from test.support import import_helper
 import contextlib
+import os
+import tempfile
+import textwrap
+import subprocess
 import warnings
 
 support.requires_working_socket(module=True)
@@ -1802,6 +1806,39 @@ class TraceOpcodesTestCase(TraceTestCase):
     def make_tracer():
         return Tracer(trace_opcode_events=True)
 
+    def test_trace_opcodes_after_settrace(self):
+        """Make sure setting f_trace_opcodes after starting trace works even
+        if it's the first time f_trace_opcodes is being set. GH-103615"""
+
+        code = textwrap.dedent("""
+            import sys
+
+            def opcode_trace_func(frame, event, arg):
+                if event == "opcode":
+                    print("opcode trace triggered")
+                return opcode_trace_func
+
+            sys.settrace(opcode_trace_func)
+            sys._getframe().f_trace = opcode_trace_func
+            sys._getframe().f_trace_opcodes = True
+            a = 1
+        """)
+
+        # We can't use context manager because Windows can't execute a file while
+        # it's being written
+        tmp = tempfile.NamedTemporaryFile(delete=False, suffix='.py')
+        tmp.write(code.encode('utf-8'))
+        tmp.close()
+        try:
+            p = subprocess.Popen([sys.executable, tmp.name], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+            p.wait()
+            out = p.stdout.read()
+        finally:
+            os.remove(tmp.name)
+            p.stdout.close()
+            p.stderr.close()
+        self.assertIn(b"opcode trace triggered", out)
+
 
 class RaisingTraceFuncTestCase(unittest.TestCase):
     def setUp(self):

diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-15-23-39-43.gh-issue-103615.WZavly.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-15-23-39-43.gh-issue-103615.WZavly.rst
new file mode 100644 (file)
index 0000000..2a0e10b
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2023-09-15-23-39-43.gh-issue-103615.WZavly.rst
@@ -0,0 +1 @@
+Use local events for opcode tracing

diff --git a/Objects/frameobject.c b/Objects/frameobject.c
index 170c1177069dca32e7013ffc48934d30f7a4a398..3b72651a1c0f7415306eb99d949c0a43c432f368 100644 (file)
--- a/Objects/frameobject.c
+++ b/Objects/frameobject.c
@@ -127,10 +127,13 @@ frame_settrace_opcodes(PyFrameObject *f, PyObject* value, void *Py_UNUSED(ignore
     }
     if (value == Py_True) {
         f->f_trace_opcodes = 1;
-        _PyInterpreterState_GET()->f_opcode_trace_set = true;
+        if (f->f_trace) {
+            return _PyEval_SetOpcodeTrace(f, true);
+        }
     }
     else {
         f->f_trace_opcodes = 0;
+        return _PyEval_SetOpcodeTrace(f, false);
     }
     return 0;
 }
@@ -842,6 +845,9 @@ frame_settrace(PyFrameObject *f, PyObject* v, void *closure)
     }
     if (v != f->f_trace) {
         Py_XSETREF(f->f_trace, Py_XNewRef(v));
+        if (v != NULL && f->f_trace_opcodes) {
+            return _PyEval_SetOpcodeTrace(f, true);
+        }
     }
     return 0;
 }

diff --git a/Python/instrumentation.c b/Python/instrumentation.c
index 9ee11588e448aea9c48490570b855d4f7679398b..35b0e7a8f35c564fec1d57ad4d1d52aeff2c162e 100644 (file)
--- a/Python/instrumentation.c
+++ b/Python/instrumentation.c
@@ -1833,6 +1833,23 @@ _PyMonitoring_SetLocalEvents(PyCodeObject *code, int tool_id, _PyMonitoringEvent
     return 0;
 }
 
+int
+_PyMonitoring_GetLocalEvents(PyCodeObject *code, int tool_id, _PyMonitoringEventSet *events)
+{
+    assert(0 <= tool_id && tool_id < PY_MONITORING_TOOL_IDS);
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+    if (check_tool(interp, tool_id)) {
+        return -1;
+    }
+    if (code->_co_monitoring == NULL) {
+        *events = 0;
+        return 0;
+    }
+    _Py_LocalMonitors *local = &code->_co_monitoring->local_monitors;
+    *events = get_local_events(local, tool_id);
+    return 0;
+}
+
 /*[clinic input]
 module monitoring
 [clinic start generated code]*/

diff --git a/Python/legacy_tracing.c b/Python/legacy_tracing.c
index ddc727113af3341a3928306077acf6fa089fbc7c..ccbb3eb3f7c82a2502c0743c072e3e16f8b39908 100644 (file)
--- a/Python/legacy_tracing.c
+++ b/Python/legacy_tracing.c
@@ -117,6 +117,35 @@ sys_profile_call_or_return(
     Py_RETURN_NONE;
 }
 
+int
+_PyEval_SetOpcodeTrace(
+    PyFrameObject *frame,
+    bool enable
+) {
+    assert(frame != NULL);
+    assert(PyCode_Check(frame->f_frame->f_executable));
+
+    PyCodeObject *code = (PyCodeObject *)frame->f_frame->f_executable;
+    _PyMonitoringEventSet events = 0;
+
+    if (_PyMonitoring_GetLocalEvents(code, PY_MONITORING_SYS_TRACE_ID, &events) < 0) {
+        return -1;
+    }
+
+    if (enable) {
+        if (events & (1 << PY_MONITORING_EVENT_INSTRUCTION)) {
+            return 0;
+        }
+        events |= (1 << PY_MONITORING_EVENT_INSTRUCTION);
+    } else {
+        if (!(events & (1 << PY_MONITORING_EVENT_INSTRUCTION))) {
+            return 0;
+        }
+        events &= (~(1 << PY_MONITORING_EVENT_INSTRUCTION));
+    }
+    return _PyMonitoring_SetLocalEvents(code, PY_MONITORING_SYS_TRACE_ID, events);
+}
+
 static PyObject *
 call_trace_func(_PyLegacyEventHandler *self, PyObject *arg)
 {
@@ -130,6 +159,12 @@ call_trace_func(_PyLegacyEventHandler *self, PyObject *arg)
                         "Missing frame when calling trace function.");
         return NULL;
     }
+    if (frame->f_trace_opcodes) {
+        if (_PyEval_SetOpcodeTrace(frame, true) != 0) {
+            return NULL;
+        }
+    }
+
     Py_INCREF(frame);
     int err = tstate->c_tracefunc(tstate->c_traceobj, frame, self->event, arg);
     Py_DECREF(frame);
@@ -230,11 +265,14 @@ sys_trace_instruction_func(
                         "Missing frame when calling trace function.");
         return NULL;
     }
-    if (!frame->f_trace_opcodes) {
+    PyThreadState *tstate = _PyThreadState_GET();
+    if (!tstate->c_tracefunc || !frame->f_trace_opcodes) {
+        if (_PyEval_SetOpcodeTrace(frame, false) != 0) {
+            return NULL;
+        }
         Py_RETURN_NONE;
     }
     Py_INCREF(frame);
-    PyThreadState *tstate = _PyThreadState_GET();
     int err = tstate->c_tracefunc(tstate->c_traceobj, frame, self->event, Py_None);
     frame->f_lineno = 0;
     Py_DECREF(frame);
@@ -531,9 +569,15 @@ _PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg)
             (1 << PY_MONITORING_EVENT_PY_UNWIND) | (1 << PY_MONITORING_EVENT_PY_THROW) |
             (1 << PY_MONITORING_EVENT_STOP_ITERATION) |
             (1 << PY_MONITORING_EVENT_EXCEPTION_HANDLED);
-        if (tstate->interp->f_opcode_trace_set) {
-            events |= (1 << PY_MONITORING_EVENT_INSTRUCTION);
+
+        PyFrameObject* frame = PyEval_GetFrame();
+        if (frame->f_trace_opcodes) {
+            int ret = _PyEval_SetOpcodeTrace(frame, true);
+            if (ret != 0) {
+                return ret;
+            }
         }
     }
+
     return _PyMonitoring_SetEvents(PY_MONITORING_SYS_TRACE_ID, events);
 }

diff --git a/Python/pystate.c b/Python/pystate.c
index 8970e17a3c101bfb688ae0744ec9bf1f5a1c2026..b369a56d6d5a08e0d140d59eef10ce442e66dac9 100644 (file)
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -708,7 +708,6 @@ init_interpreter(PyInterpreterState *interp,
         /* Fix the self-referential, statically initialized fields. */
         interp->dtoa = (struct _dtoa_state)_dtoa_state_INIT(interp);
     }
-    interp->f_opcode_trace_set = false;
 
     interp->_initialized = 1;
     return _PyStatus_OK();
@@ -958,7 +957,6 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
         interp->code_watchers[i] = NULL;
     }
     interp->active_code_watchers = 0;
-    interp->f_opcode_trace_set = false;
     // XXX Once we have one allocator per interpreter (i.e.
     // per-interpreter GC) we must ensure that all of the interpreter's
     // objects have been cleaned up at the point.