Conversation
`sys.setprofile` and `sys.settrace` callbacks are no longer called for `sys.monitoring` events that extension modules fire for their own code objects through the monitoring C API. Until now, the `sys.setprofile` or `sys.settrace` function would get run, but it would be passed the top frame on the Python stack rather than anything related to the code object passed to the monitoring C API, and this would manifest to the legacy tracing function as nonsensical duplicate events rather than anything that could be attributed to the code object passed to the monitoring API.
| ( 1, E.PY_RESUME, capi.fire_event_py_resume), | ||
| ( 1, E.PY_YIELD, capi.fire_event_py_yield, 10), | ||
| ( 1, E.PY_RETURN, capi.fire_event_py_return, 20), | ||
| ( 2, E.CALL, capi.fire_event_call, callable, 40), |
There was a problem hiding this comment.
Note: The removed 1st tuple element (never reflected in the above comment!) was the expected number of calls that would be seen. It was 1 for everything except E.CALL, where it was 2 instead. It was 2 for E.CALL because we'd see both a call to the monitoring API and a call to the Python function that triggered the call to the monitoring API. Instead of having this count of expected calls, the test now uses a Counter class that looks only for calls for a particular CodeLike and ignores all others, which makes it so that the expected count is 1 for everything instead of for everything but CALL.
| sys.monitoring.register_callback(TEST_TOOL, event, counter) | ||
| if event == E.C_RETURN or event == E.C_RAISE: | ||
| sys.monitoring.set_events(TEST_TOOL, E.CALL) | ||
| event_value = int(math.log2(E.CALL)) |
There was a problem hiding this comment.
Previously this would call
self.Scope(self.codelike, math.log2(E.C_RETURN))which results in an out-of-bounds read inside PyMonitoring_EnterScope. It accesses m->tools[event], and m->tools is an array of 16 elements, and PY_MONITORING_EVENT_C_RETURN is 16 and so one past the end of that array.
This was wrong; you get the C_RETURN event by subscribing to C_CALL events. That's what this fix is.
| if (PyVectorcall_NARGS(nargsf) != 4) { | ||
| // PyMonitoring_FireCReturnEvent and PyMonitoring_FireCRaiseEvent | ||
| // don't provide the callable. | ||
| assert(PyVectorcall_NARGS(nargsf) == 3); |
There was a problem hiding this comment.
This fixes an assertion error that would be hit on debug builds whenever you call PyMonitoring_FireCReturnEvent or PyMonitoring_FireCRaiseEvent with a sys.setprofile or sys.settrace legacy trace function installed.
| assert(!PyErr_Occurred()); | ||
| assert(kwnames == NULL); | ||
| assert(PyVectorcall_NARGS(nargsf) == 3); | ||
| assert(PyCode_Check(args[0])); |
There was a problem hiding this comment.
This would trigger an assertion error in the tests, where we pass a CodeLike instead of a code object. It only didn't fail up until now because the path wasn't being exercised.
| PyFrameObject *frame = get_top_frame_if_running_code(args[0]); | ||
| if (frame == NULL) { | ||
| Py_RETURN_NONE; | ||
| } |
There was a problem hiding this comment.
This check is moved up so that we know that the code object we received is actually a PyCodeObject before we cast it to one.
1 participant
sys.setprofileandsys.settracecallbacks are no longer called forsys.monitoringevents that extension modules fire for their own code objects through the monitoring C API.Until now, the
sys.setprofileorsys.settracefunction would get run, but it would be passed the top frame on the Python stack rather than anything related to the code object passed to the monitoring C API, and this would manifest to the legacy tracing function as nonsensical duplicate events rather than anything that could be attributed to the code object passed to the monitoring API.PyMonitoring_FirePyStartEventresults in incorrect duplicate calls tosys.setprofileprofile functions #158711