Skip to content

Commit c6539b3

Browse files
markshannonmarkshannon
authored
GH-106895: Raise a ValueError when attempting to disable events that cannot be disabled. (GH-107337)
1 parent d77d973 commit c6539b3

File tree

9 files changed

+120
-52
lines changed

9 files changed

+120
-52
lines changed

‎Include/internal/pycore_instruments.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ extern "C" {
2828
#define PY_MONITORING_EVENT_BRANCH 8
2929
#define PY_MONITORING_EVENT_STOP_ITERATION 9
3030

31-
#define PY_MONITORING_INSTRUMENTED_EVENTS 10
31+
#define PY_MONITORING_IS_INSTRUMENTED_EVENT(ev) \
32+
((ev) <= PY_MONITORING_EVENT_STOP_ITERATION)
3233

3334
/* Other events, mainly exceptions */
3435

‎Lib/test/test_monitoring.py

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,20 +136,27 @@ def test_c_return_count(self):
136136

137137
E = sys.monitoring.events
138138

139-
SIMPLE_EVENTS = [
139+
INSTRUMENTED_EVENTS = [
140140
(E.PY_START, "start"),
141141
(E.PY_RESUME, "resume"),
142142
(E.PY_RETURN, "return"),
143143
(E.PY_YIELD, "yield"),
144144
(E.JUMP, "jump"),
145145
(E.BRANCH, "branch"),
146+
]
147+
148+
EXCEPT_EVENTS = [
146149
(E.RAISE, "raise"),
147150
(E.PY_UNWIND, "unwind"),
148151
(E.EXCEPTION_HANDLED, "exception_handled"),
152+
]
153+
154+
SIMPLE_EVENTS = INSTRUMENTED_EVENTS + EXCEPT_EVENTS + [
149155
(E.C_RAISE, "c_raise"),
150156
(E.C_RETURN, "c_return"),
151157
]
152158

159+
153160
SIMPLE_EVENT_SET = functools.reduce(operator.or_, [ev for (ev, _) in SIMPLE_EVENTS], 0) | E.CALL
154161

155162

@@ -618,6 +625,49 @@ def func2():
618625

619626
self.check_lines(func2, [1,2,3,4,5,6])
620627

628+
class TestDisable(MonitoringTestBase, unittest.TestCase):
629+
630+
def gen(self, cond):
631+
for i in range(10):
632+
if cond:
633+
yield 1
634+
else:
635+
yield 2
636+
637+
def raise_handle_reraise(self):
638+
try:
639+
1/0
640+
except:
641+
raise
642+
643+
def test_disable_legal_events(self):
644+
for event, name in INSTRUMENTED_EVENTS:
645+
try:
646+
counter = CounterWithDisable()
647+
counter.disable = True
648+
sys.monitoring.register_callback(TEST_TOOL, event, counter)
649+
sys.monitoring.set_events(TEST_TOOL, event)
650+
for _ in self.gen(1):
651+
pass
652+
self.assertLess(counter.count, 4)
653+
finally:
654+
sys.monitoring.set_events(TEST_TOOL, 0)
655+
sys.monitoring.register_callback(TEST_TOOL, event, None)
656+
657+
658+
def test_disable_illegal_events(self):
659+
for event, name in EXCEPT_EVENTS:
660+
try:
661+
counter = CounterWithDisable()
662+
counter.disable = True
663+
sys.monitoring.register_callback(TEST_TOOL, event, counter)
664+
sys.monitoring.set_events(TEST_TOOL, event)
665+
with self.assertRaises(ValueError):
666+
self.raise_handle_reraise()
667+
finally:
668+
sys.monitoring.set_events(TEST_TOOL, 0)
669+
sys.monitoring.register_callback(TEST_TOOL, event, None)
670+
621671

622672
class ExceptionRecorder:
623673

‎Misc/NEWS.d/next/Core and Builtins/2023-07-26-18-53-34.gh-issue-106895.DdEwV8.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Raise a ``ValueError`` when a monitoring callback funtion returns
2+
``DISABLE`` for events that cannot be disabled locally.

‎Objects/classobject.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ method_vectorcall(PyObject *method, PyObject *const *args,
4848
PyObject *self = PyMethod_GET_SELF(method);
4949
PyObject *func = PyMethod_GET_FUNCTION(method);
5050
Py_ssize_t nargs = PyVectorcall_NARGS(nargsf);
51+
assert(nargs == 0 || args[nargs-1]);
5152

5253
PyObject *result;
5354
if (nargsf & PY_VECTORCALL_ARGUMENTS_OFFSET) {
@@ -56,6 +57,7 @@ method_vectorcall(PyObject *method, PyObject *const *args,
5657
nargs += 1;
5758
PyObject *tmp = newargs[0];
5859
newargs[0] = self;
60+
assert(newargs[nargs-1]);
5961
result = _PyObject_VectorcallTstate(tstate, func, newargs,
6062
nargs, kwnames);
6163
newargs[0] = tmp;

‎Python/bytecodes.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2672,7 +2672,12 @@ dummy_func(
26722672
assert(val && PyExceptionInstance_Check(val));
26732673
exc = PyExceptionInstance_Class(val);
26742674
tb = PyException_GetTraceback(val);
2675-
Py_XDECREF(tb);
2675+
if (tb == NULL) {
2676+
tb = Py_None;
2677+
}
2678+
else {
2679+
Py_DECREF(tb);
2680+
}
26762681
assert(PyLong_Check(lasti));
26772682
(void)lasti; // Shut up compiler warning if asserts are off
26782683
PyObject *stack[4] = {NULL, exc, val, tb};

‎Python/ceval.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ static int monitor_stop_iteration(PyThreadState *tstate,
193193
static void monitor_unwind(PyThreadState *tstate,
194194
_PyInterpreterFrame *frame,
195195
_Py_CODEUNIT *instr);
196-
static void monitor_handled(PyThreadState *tstate,
196+
static int monitor_handled(PyThreadState *tstate,
197197
_PyInterpreterFrame *frame,
198198
_Py_CODEUNIT *instr, PyObject *exc);
199199
static void monitor_throw(PyThreadState *tstate,
@@ -886,7 +886,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
886886
PyObject *exc = _PyErr_GetRaisedException(tstate);
887887
PUSH(exc);
888888
JUMPTO(handler);
889-
monitor_handled(tstate, frame, next_instr, exc);
889+
if (monitor_handled(tstate, frame, next_instr, exc) < 0) {
890+
goto exception_unwind;
891+
}
890892
/* Resume normal execution */
891893
DISPATCH();
892894
}
@@ -1924,6 +1926,7 @@ do_monitor_exc(PyThreadState *tstate, _PyInterpreterFrame *frame,
19241926
PyErr_SetRaisedException(exc);
19251927
}
19261928
else {
1929+
assert(PyErr_Occurred());
19271930
Py_DECREF(exc);
19281931
}
19291932
return err;
@@ -1988,15 +1991,15 @@ monitor_unwind(PyThreadState *tstate,
19881991
}
19891992

19901993

1991-
static void
1994+
static int
19921995
monitor_handled(PyThreadState *tstate,
19931996
_PyInterpreterFrame *frame,
19941997
_Py_CODEUNIT *instr, PyObject *exc)
19951998
{
19961999
if (no_tools_for_event(tstate, frame, PY_MONITORING_EVENT_EXCEPTION_HANDLED)) {
1997-
return;
2000+
return 0;
19982001
}
1999-
_Py_call_instrumentation_arg(tstate, PY_MONITORING_EVENT_EXCEPTION_HANDLED, frame, instr, exc);
2002+
return _Py_call_instrumentation_arg(tstate, PY_MONITORING_EVENT_EXCEPTION_HANDLED, frame, instr, exc);
20002003
}
20012004

20022005
static void

‎Python/executor_cases.c.h

Lines changed: 6 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/generated_cases.c.h

Lines changed: 6 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/instrumentation.c

Lines changed: 37 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -702,29 +702,13 @@ instrument_per_instruction(PyCodeObject *code, int i)
702702
*opcode_ptr = INSTRUMENTED_INSTRUCTION;
703703
}
704704

705-
#ifndef NDEBUG
706-
static bool
707-
instruction_has_event(PyCodeObject *code, int offset)
708-
{
709-
_Py_CODEUNIT instr = _PyCode_CODE(code)[offset];
710-
int opcode = instr.op.code;
711-
if (opcode == INSTRUMENTED_LINE) {
712-
opcode = code->_co_monitoring->lines[offset].original_opcode;
713-
}
714-
if (opcode == INSTRUMENTED_INSTRUCTION) {
715-
opcode = code->_co_monitoring->per_instruction_opcodes[offset];
716-
}
717-
return opcode_has_event(opcode);
718-
}
719-
#endif
720-
721705
static void
722706
remove_tools(PyCodeObject * code, int offset, int event, int tools)
723707
{
724708
assert(event != PY_MONITORING_EVENT_LINE);
725709
assert(event != PY_MONITORING_EVENT_INSTRUCTION);
726-
assert(event < PY_MONITORING_INSTRUMENTED_EVENTS);
727-
assert(instruction_has_event(code, offset));
710+
assert(PY_MONITORING_IS_INSTRUMENTED_EVENT(event));
711+
assert(opcode_has_event(_Py_GetBaseOpcode(code, offset)));
728712
_PyCoMonitoringData *monitoring = code->_co_monitoring;
729713
if (monitoring && monitoring->tools) {
730714
monitoring->tools[offset] &= ~tools;
@@ -779,7 +763,7 @@ add_tools(PyCodeObject * code, int offset, int event, int tools)
779763
{
780764
assert(event != PY_MONITORING_EVENT_LINE);
781765
assert(event != PY_MONITORING_EVENT_INSTRUCTION);
782-
assert(event < PY_MONITORING_INSTRUMENTED_EVENTS);
766+
assert(PY_MONITORING_IS_INSTRUMENTED_EVENT(event));
783767
assert(code->_co_monitoring);
784768
if (code->_co_monitoring &&
785769
code->_co_monitoring->tools
@@ -919,7 +903,7 @@ get_tools_for_instruction(PyCodeObject *code, PyInterpreterState *interp, int i,
919903
event == PY_MONITORING_EVENT_C_RETURN);
920904
event = PY_MONITORING_EVENT_CALL;
921905
}
922-
if (event < PY_MONITORING_INSTRUMENTED_EVENTS) {
906+
if (PY_MONITORING_IS_INSTRUMENTED_EVENT(event)) {
923907
CHECK(is_version_up_to_date(code, interp));
924908
CHECK(instrumentation_cross_checks(interp, code));
925909
if (code->_co_monitoring->tools) {
@@ -940,6 +924,26 @@ get_tools_for_instruction(PyCodeObject *code, PyInterpreterState *interp, int i,
940924
return tools;
941925
}
942926

927+
static const char *const event_names [] = {
928+
[PY_MONITORING_EVENT_PY_START] = "PY_START",
929+
[PY_MONITORING_EVENT_PY_RESUME] = "PY_RESUME",
930+
[PY_MONITORING_EVENT_PY_RETURN] = "PY_RETURN",
931+
[PY_MONITORING_EVENT_PY_YIELD] = "PY_YIELD",
932+
[PY_MONITORING_EVENT_CALL] = "CALL",
933+
[PY_MONITORING_EVENT_LINE] = "LINE",
934+
[PY_MONITORING_EVENT_INSTRUCTION] = "INSTRUCTION",
935+
[PY_MONITORING_EVENT_JUMP] = "JUMP",
936+
[PY_MONITORING_EVENT_BRANCH] = "BRANCH",
937+
[PY_MONITORING_EVENT_C_RETURN] = "C_RETURN",
938+
[PY_MONITORING_EVENT_PY_THROW] = "PY_THROW",
939+
[PY_MONITORING_EVENT_RAISE] = "RAISE",
940+
[PY_MONITORING_EVENT_RERAISE] = "RERAISE",
941+
[PY_MONITORING_EVENT_EXCEPTION_HANDLED] = "EXCEPTION_HANDLED",
942+
[PY_MONITORING_EVENT_C_RAISE] = "C_RAISE",
943+
[PY_MONITORING_EVENT_PY_UNWIND] = "PY_UNWIND",
944+
[PY_MONITORING_EVENT_STOP_ITERATION] = "STOP_ITERATION",
945+
};
946+
943947
static int
944948
call_instrumentation_vector(
945949
PyThreadState *tstate, int event,
@@ -984,7 +988,18 @@ call_instrumentation_vector(
984988
}
985989
else {
986990
/* DISABLE */
987-
remove_tools(code, offset, event, 1 << tool);
991+
if (!PY_MONITORING_IS_INSTRUMENTED_EVENT(event)) {
992+
PyErr_Format(PyExc_ValueError,
993+
"Cannot disable %s events. Callback removed.",
994+
event_names[event]);
995+
/* Clear tool to prevent infinite loop */
996+
Py_CLEAR(interp->monitoring_callables[tool][event]);
997+
err = -1;
998+
break;
999+
}
1000+
else {
1001+
remove_tools(code, offset, event, 1 << tool);
1002+
}
9881003
}
9891004
}
9901005
Py_DECREF(offset_obj);
@@ -1262,7 +1277,7 @@ initialize_tools(PyCodeObject *code)
12621277
assert(event > 0);
12631278
}
12641279
assert(event >= 0);
1265-
assert(event < PY_MONITORING_INSTRUMENTED_EVENTS);
1280+
assert(PY_MONITORING_IS_INSTRUMENTED_EVENT(event));
12661281
tools[i] = code->_co_monitoring->active_monitors.tools[event];
12671282
CHECK(tools[i] != 0);
12681283
}
@@ -2017,26 +2032,6 @@ add_power2_constant(PyObject *obj, const char *name, int i)
20172032
return err;
20182033
}
20192034

2020-
static const char *const event_names [] = {
2021-
[PY_MONITORING_EVENT_PY_START] = "PY_START",
2022-
[PY_MONITORING_EVENT_PY_RESUME] = "PY_RESUME",
2023-
[PY_MONITORING_EVENT_PY_RETURN] = "PY_RETURN",
2024-
[PY_MONITORING_EVENT_PY_YIELD] = "PY_YIELD",
2025-
[PY_MONITORING_EVENT_CALL] = "CALL",
2026-
[PY_MONITORING_EVENT_LINE] = "LINE",
2027-
[PY_MONITORING_EVENT_INSTRUCTION] = "INSTRUCTION",
2028-
[PY_MONITORING_EVENT_JUMP] = "JUMP",
2029-
[PY_MONITORING_EVENT_BRANCH] = "BRANCH",
2030-
[PY_MONITORING_EVENT_C_RETURN] = "C_RETURN",
2031-
[PY_MONITORING_EVENT_PY_THROW] = "PY_THROW",
2032-
[PY_MONITORING_EVENT_RAISE] = "RAISE",
2033-
[PY_MONITORING_EVENT_RERAISE] = "RERAISE",
2034-
[PY_MONITORING_EVENT_EXCEPTION_HANDLED] = "EXCEPTION_HANDLED",
2035-
[PY_MONITORING_EVENT_C_RAISE] = "C_RAISE",
2036-
[PY_MONITORING_EVENT_PY_UNWIND] = "PY_UNWIND",
2037-
[PY_MONITORING_EVENT_STOP_ITERATION] = "STOP_ITERATION",
2038-
};
2039-
20402035
/*[clinic input]
20412036
monitoring._all_events
20422037
[clinic start generated code]*/

0 commit comments

Comments
 (0)