From 0ae60b66dea5140382190463a676bafe706608f5 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 5 Jan 2024 09:45:22 +0000 Subject: [PATCH] GH-113486: Do not emit spurious PY_UNWIND events for optimized calls to classes. (GH-113680) --- Include/cpython/code.h | 2 + Lib/test/test_monitoring.py | 78 +++++++++++++------ ...-12-31-07-46-01.gh-issue-113486.uki19C.rst | 1 + Python/ceval.c | 3 + Python/instrumentation.c | 6 +- Python/specialize.c | 2 +- 6 files changed, 64 insertions(+), 28 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-12-31-07-46-01.gh-issue-113486.uki19C.rst diff --git a/Include/cpython/code.h b/Include/cpython/code.h index cf715c55a2b..1f47d99fb60 100644 --- a/Include/cpython/code.h +++ b/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. */ diff --git a/Lib/test/test_monitoring.py b/Lib/test/test_monitoring.py index a2efbc95f55..a64d1ed79de 100644 --- a/Lib/test/test_monitoring.py +++ b/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,6 +955,38 @@ class ExceptionMonitoringTest(CheckEvents): ) self.assertEqual(events[0], ("throw", IndexError)) + def test_no_unwind_for_shim_frame(self): + + 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), + ] + + self.assertEqual(events, expected) + class LineRecorder: event_type = E.LINE @@ -947,16 +998,6 @@ class LineRecorder: def __call__(self, code, line): self.events.append(("line", code.co_name, line - code.co_firstlineno)) -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 CEventRecorder: def __init__(self, events): @@ -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 @@ class TestBranchAndJumpEvents(CheckEvents): ('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): diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-12-31-07-46-01.gh-issue-113486.uki19C.rst b/Misc/NEWS.d/next/Core and Builtins/2023-12-31-07-46-01.gh-issue-113486.uki19C.rst new file mode 100644 index 00000000000..42ff4a2feb1 --- /dev/null +++ b/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. diff --git a/Python/ceval.c b/Python/ceval.c index 1fea9747488..b3b542f8dde 100644 --- a/Python/ceval.c +++ b/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); diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 35b0e7a8f35..533aece2102 100644 --- a/Python/instrumentation.c +++ b/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)) { diff --git a/Python/specialize.c b/Python/specialize.c index 369b962a545..7b63393803b 100644 --- a/Python/specialize.c +++ b/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__),