[Python-checkins] GH-106895: Raise a `ValueError` when attempting to disable events that cannot be disabled. (GH-107337)
markshannon
webhook-mailer at python.org
Thu Jul 27 10:27:16 EDT 2023
https://github.com/python/cpython/commit/c6539b36c163efff3d6ed02b938a6151325f4db7
commit: c6539b36c163efff3d6ed02b938a6151325f4db7
branch: main
author: Mark Shannon <mark at hotpy.org>
committer: markshannon <mark at hotpy.org>
date: 2023-07-27T15:27:11+01:00
summary:
GH-106895: Raise a `ValueError` when attempting to disable events that cannot be disabled. (GH-107337)
files:
A Misc/NEWS.d/next/Core and Builtins/2023-07-26-18-53-34.gh-issue-106895.DdEwV8.rst
M Include/internal/pycore_instruments.h
M Lib/test/test_monitoring.py
M Objects/classobject.c
M Python/bytecodes.c
M Python/ceval.c
M Python/executor_cases.c.h
M Python/generated_cases.c.h
M Python/instrumentation.c
diff --git a/Include/internal/pycore_instruments.h b/Include/internal/pycore_instruments.h
index cfa5d09a4704f..ccccd54a2f70a 100644
--- a/Include/internal/pycore_instruments.h
+++ b/Include/internal/pycore_instruments.h
@@ -28,7 +28,8 @@ extern "C" {
#define PY_MONITORING_EVENT_BRANCH 8
#define PY_MONITORING_EVENT_STOP_ITERATION 9
-#define PY_MONITORING_INSTRUMENTED_EVENTS 10
+#define PY_MONITORING_IS_INSTRUMENTED_EVENT(ev) \
+ ((ev) <= PY_MONITORING_EVENT_STOP_ITERATION)
/* Other events, mainly exceptions */
diff --git a/Lib/test/test_monitoring.py b/Lib/test/test_monitoring.py
index 7538786ccf218..7098f483e0ee7 100644
--- a/Lib/test/test_monitoring.py
+++ b/Lib/test/test_monitoring.py
@@ -136,20 +136,27 @@ def test_c_return_count(self):
E = sys.monitoring.events
-SIMPLE_EVENTS = [
+INSTRUMENTED_EVENTS = [
(E.PY_START, "start"),
(E.PY_RESUME, "resume"),
(E.PY_RETURN, "return"),
(E.PY_YIELD, "yield"),
(E.JUMP, "jump"),
(E.BRANCH, "branch"),
+]
+
+EXCEPT_EVENTS = [
(E.RAISE, "raise"),
(E.PY_UNWIND, "unwind"),
(E.EXCEPTION_HANDLED, "exception_handled"),
+]
+
+SIMPLE_EVENTS = INSTRUMENTED_EVENTS + EXCEPT_EVENTS + [
(E.C_RAISE, "c_raise"),
(E.C_RETURN, "c_return"),
]
+
SIMPLE_EVENT_SET = functools.reduce(operator.or_, [ev for (ev, _) in SIMPLE_EVENTS], 0) | E.CALL
@@ -618,6 +625,49 @@ def func2():
self.check_lines(func2, [1,2,3,4,5,6])
+class TestDisable(MonitoringTestBase, unittest.TestCase):
+
+ def gen(self, cond):
+ for i in range(10):
+ if cond:
+ yield 1
+ else:
+ yield 2
+
+ def raise_handle_reraise(self):
+ try:
+ 1/0
+ except:
+ raise
+
+ def test_disable_legal_events(self):
+ for event, name in INSTRUMENTED_EVENTS:
+ try:
+ counter = CounterWithDisable()
+ counter.disable = True
+ sys.monitoring.register_callback(TEST_TOOL, event, counter)
+ sys.monitoring.set_events(TEST_TOOL, event)
+ for _ in self.gen(1):
+ pass
+ self.assertLess(counter.count, 4)
+ finally:
+ sys.monitoring.set_events(TEST_TOOL, 0)
+ sys.monitoring.register_callback(TEST_TOOL, event, None)
+
+
+ def test_disable_illegal_events(self):
+ for event, name in EXCEPT_EVENTS:
+ try:
+ counter = CounterWithDisable()
+ counter.disable = True
+ sys.monitoring.register_callback(TEST_TOOL, event, counter)
+ sys.monitoring.set_events(TEST_TOOL, event)
+ with self.assertRaises(ValueError):
+ self.raise_handle_reraise()
+ finally:
+ sys.monitoring.set_events(TEST_TOOL, 0)
+ sys.monitoring.register_callback(TEST_TOOL, event, None)
+
class ExceptionRecorder:
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-07-26-18-53-34.gh-issue-106895.DdEwV8.rst b/Misc/NEWS.d/next/Core and Builtins/2023-07-26-18-53-34.gh-issue-106895.DdEwV8.rst
new file mode 100644
index 0000000000000..370a29d34c860
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2023-07-26-18-53-34.gh-issue-106895.DdEwV8.rst
@@ -0,0 +1,2 @@
+Raise a ``ValueError`` when a monitoring callback funtion returns
+``DISABLE`` for events that cannot be disabled locally.
diff --git a/Objects/classobject.c b/Objects/classobject.c
index 6f4457d42dda9..5471045d777c9 100644
--- a/Objects/classobject.c
+++ b/Objects/classobject.c
@@ -48,6 +48,7 @@ method_vectorcall(PyObject *method, PyObject *const *args,
PyObject *self = PyMethod_GET_SELF(method);
PyObject *func = PyMethod_GET_FUNCTION(method);
Py_ssize_t nargs = PyVectorcall_NARGS(nargsf);
+ assert(nargs == 0 || args[nargs-1]);
PyObject *result;
if (nargsf & PY_VECTORCALL_ARGUMENTS_OFFSET) {
@@ -56,6 +57,7 @@ method_vectorcall(PyObject *method, PyObject *const *args,
nargs += 1;
PyObject *tmp = newargs[0];
newargs[0] = self;
+ assert(newargs[nargs-1]);
result = _PyObject_VectorcallTstate(tstate, func, newargs,
nargs, kwnames);
newargs[0] = tmp;
diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index da9d8e2a56a41..2b871e8546efb 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -2672,7 +2672,12 @@ dummy_func(
assert(val && PyExceptionInstance_Check(val));
exc = PyExceptionInstance_Class(val);
tb = PyException_GetTraceback(val);
- Py_XDECREF(tb);
+ if (tb == NULL) {
+ tb = Py_None;
+ }
+ else {
+ Py_DECREF(tb);
+ }
assert(PyLong_Check(lasti));
(void)lasti; // Shut up compiler warning if asserts are off
PyObject *stack[4] = {NULL, exc, val, tb};
diff --git a/Python/ceval.c b/Python/ceval.c
index a5d37c0629df9..c0b37b328a25f 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -193,7 +193,7 @@ static int monitor_stop_iteration(PyThreadState *tstate,
static void monitor_unwind(PyThreadState *tstate,
_PyInterpreterFrame *frame,
_Py_CODEUNIT *instr);
-static void monitor_handled(PyThreadState *tstate,
+static int monitor_handled(PyThreadState *tstate,
_PyInterpreterFrame *frame,
_Py_CODEUNIT *instr, PyObject *exc);
static void monitor_throw(PyThreadState *tstate,
@@ -886,7 +886,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
PyObject *exc = _PyErr_GetRaisedException(tstate);
PUSH(exc);
JUMPTO(handler);
- monitor_handled(tstate, frame, next_instr, exc);
+ if (monitor_handled(tstate, frame, next_instr, exc) < 0) {
+ goto exception_unwind;
+ }
/* Resume normal execution */
DISPATCH();
}
@@ -1924,6 +1926,7 @@ do_monitor_exc(PyThreadState *tstate, _PyInterpreterFrame *frame,
PyErr_SetRaisedException(exc);
}
else {
+ assert(PyErr_Occurred());
Py_DECREF(exc);
}
return err;
@@ -1988,15 +1991,15 @@ monitor_unwind(PyThreadState *tstate,
}
-static void
+static int
monitor_handled(PyThreadState *tstate,
_PyInterpreterFrame *frame,
_Py_CODEUNIT *instr, PyObject *exc)
{
if (no_tools_for_event(tstate, frame, PY_MONITORING_EVENT_EXCEPTION_HANDLED)) {
- return;
+ return 0;
}
- _Py_call_instrumentation_arg(tstate, PY_MONITORING_EVENT_EXCEPTION_HANDLED, frame, instr, exc);
+ return _Py_call_instrumentation_arg(tstate, PY_MONITORING_EVENT_EXCEPTION_HANDLED, frame, instr, exc);
}
static void
diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
index 0f04b428ba805..f3e24bc0479ec 100644
--- a/Python/executor_cases.c.h
+++ b/Python/executor_cases.c.h
@@ -1949,7 +1949,12 @@
assert(val && PyExceptionInstance_Check(val));
exc = PyExceptionInstance_Class(val);
tb = PyException_GetTraceback(val);
- Py_XDECREF(tb);
+ if (tb == NULL) {
+ tb = Py_None;
+ }
+ else {
+ Py_DECREF(tb);
+ }
assert(PyLong_Check(lasti));
(void)lasti; // Shut up compiler warning if asserts are off
PyObject *stack[4] = {NULL, exc, val, tb};
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index 37ffb560f1457..6ac622b11acac 100644
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -3315,7 +3315,12 @@
assert(val && PyExceptionInstance_Check(val));
exc = PyExceptionInstance_Class(val);
tb = PyException_GetTraceback(val);
- Py_XDECREF(tb);
+ if (tb == NULL) {
+ tb = Py_None;
+ }
+ else {
+ Py_DECREF(tb);
+ }
assert(PyLong_Check(lasti));
(void)lasti; // Shut up compiler warning if asserts are off
PyObject *stack[4] = {NULL, exc, val, tb};
diff --git a/Python/instrumentation.c b/Python/instrumentation.c
index 280e13d65526b..123c20dfe1a99 100644
--- a/Python/instrumentation.c
+++ b/Python/instrumentation.c
@@ -702,29 +702,13 @@ instrument_per_instruction(PyCodeObject *code, int i)
*opcode_ptr = INSTRUMENTED_INSTRUCTION;
}
-#ifndef NDEBUG
-static bool
-instruction_has_event(PyCodeObject *code, int offset)
-{
- _Py_CODEUNIT instr = _PyCode_CODE(code)[offset];
- int opcode = instr.op.code;
- if (opcode == INSTRUMENTED_LINE) {
- opcode = code->_co_monitoring->lines[offset].original_opcode;
- }
- if (opcode == INSTRUMENTED_INSTRUCTION) {
- opcode = code->_co_monitoring->per_instruction_opcodes[offset];
- }
- return opcode_has_event(opcode);
-}
-#endif
-
static void
remove_tools(PyCodeObject * code, int offset, int event, int tools)
{
assert(event != PY_MONITORING_EVENT_LINE);
assert(event != PY_MONITORING_EVENT_INSTRUCTION);
- assert(event < PY_MONITORING_INSTRUMENTED_EVENTS);
- assert(instruction_has_event(code, offset));
+ assert(PY_MONITORING_IS_INSTRUMENTED_EVENT(event));
+ assert(opcode_has_event(_Py_GetBaseOpcode(code, offset)));
_PyCoMonitoringData *monitoring = code->_co_monitoring;
if (monitoring && monitoring->tools) {
monitoring->tools[offset] &= ~tools;
@@ -779,7 +763,7 @@ add_tools(PyCodeObject * code, int offset, int event, int tools)
{
assert(event != PY_MONITORING_EVENT_LINE);
assert(event != PY_MONITORING_EVENT_INSTRUCTION);
- assert(event < PY_MONITORING_INSTRUMENTED_EVENTS);
+ assert(PY_MONITORING_IS_INSTRUMENTED_EVENT(event));
assert(code->_co_monitoring);
if (code->_co_monitoring &&
code->_co_monitoring->tools
@@ -919,7 +903,7 @@ get_tools_for_instruction(PyCodeObject *code, PyInterpreterState *interp, int i,
event == PY_MONITORING_EVENT_C_RETURN);
event = PY_MONITORING_EVENT_CALL;
}
- if (event < PY_MONITORING_INSTRUMENTED_EVENTS) {
+ if (PY_MONITORING_IS_INSTRUMENTED_EVENT(event)) {
CHECK(is_version_up_to_date(code, interp));
CHECK(instrumentation_cross_checks(interp, code));
if (code->_co_monitoring->tools) {
@@ -940,6 +924,26 @@ get_tools_for_instruction(PyCodeObject *code, PyInterpreterState *interp, int i,
return tools;
}
+static const char *const event_names [] = {
+ [PY_MONITORING_EVENT_PY_START] = "PY_START",
+ [PY_MONITORING_EVENT_PY_RESUME] = "PY_RESUME",
+ [PY_MONITORING_EVENT_PY_RETURN] = "PY_RETURN",
+ [PY_MONITORING_EVENT_PY_YIELD] = "PY_YIELD",
+ [PY_MONITORING_EVENT_CALL] = "CALL",
+ [PY_MONITORING_EVENT_LINE] = "LINE",
+ [PY_MONITORING_EVENT_INSTRUCTION] = "INSTRUCTION",
+ [PY_MONITORING_EVENT_JUMP] = "JUMP",
+ [PY_MONITORING_EVENT_BRANCH] = "BRANCH",
+ [PY_MONITORING_EVENT_C_RETURN] = "C_RETURN",
+ [PY_MONITORING_EVENT_PY_THROW] = "PY_THROW",
+ [PY_MONITORING_EVENT_RAISE] = "RAISE",
+ [PY_MONITORING_EVENT_RERAISE] = "RERAISE",
+ [PY_MONITORING_EVENT_EXCEPTION_HANDLED] = "EXCEPTION_HANDLED",
+ [PY_MONITORING_EVENT_C_RAISE] = "C_RAISE",
+ [PY_MONITORING_EVENT_PY_UNWIND] = "PY_UNWIND",
+ [PY_MONITORING_EVENT_STOP_ITERATION] = "STOP_ITERATION",
+};
+
static int
call_instrumentation_vector(
PyThreadState *tstate, int event,
@@ -984,7 +988,18 @@ call_instrumentation_vector(
}
else {
/* DISABLE */
- remove_tools(code, offset, event, 1 << tool);
+ if (!PY_MONITORING_IS_INSTRUMENTED_EVENT(event)) {
+ PyErr_Format(PyExc_ValueError,
+ "Cannot disable %s events. Callback removed.",
+ event_names[event]);
+ /* Clear tool to prevent infinite loop */
+ Py_CLEAR(interp->monitoring_callables[tool][event]);
+ err = -1;
+ break;
+ }
+ else {
+ remove_tools(code, offset, event, 1 << tool);
+ }
}
}
Py_DECREF(offset_obj);
@@ -1262,7 +1277,7 @@ initialize_tools(PyCodeObject *code)
assert(event > 0);
}
assert(event >= 0);
- assert(event < PY_MONITORING_INSTRUMENTED_EVENTS);
+ assert(PY_MONITORING_IS_INSTRUMENTED_EVENT(event));
tools[i] = code->_co_monitoring->active_monitors.tools[event];
CHECK(tools[i] != 0);
}
@@ -2017,26 +2032,6 @@ add_power2_constant(PyObject *obj, const char *name, int i)
return err;
}
-static const char *const event_names [] = {
- [PY_MONITORING_EVENT_PY_START] = "PY_START",
- [PY_MONITORING_EVENT_PY_RESUME] = "PY_RESUME",
- [PY_MONITORING_EVENT_PY_RETURN] = "PY_RETURN",
- [PY_MONITORING_EVENT_PY_YIELD] = "PY_YIELD",
- [PY_MONITORING_EVENT_CALL] = "CALL",
- [PY_MONITORING_EVENT_LINE] = "LINE",
- [PY_MONITORING_EVENT_INSTRUCTION] = "INSTRUCTION",
- [PY_MONITORING_EVENT_JUMP] = "JUMP",
- [PY_MONITORING_EVENT_BRANCH] = "BRANCH",
- [PY_MONITORING_EVENT_C_RETURN] = "C_RETURN",
- [PY_MONITORING_EVENT_PY_THROW] = "PY_THROW",
- [PY_MONITORING_EVENT_RAISE] = "RAISE",
- [PY_MONITORING_EVENT_RERAISE] = "RERAISE",
- [PY_MONITORING_EVENT_EXCEPTION_HANDLED] = "EXCEPTION_HANDLED",
- [PY_MONITORING_EVENT_C_RAISE] = "C_RAISE",
- [PY_MONITORING_EVENT_PY_UNWIND] = "PY_UNWIND",
- [PY_MONITORING_EVENT_STOP_ITERATION] = "STOP_ITERATION",
-};
-
/*[clinic input]
monitoring._all_events
[clinic start generated code]*/
More information about the Python-checkins
mailing list