From 3dd0febed07f4351bfd4e149005a5b4bf52df9db Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Wed, 9 Oct 2024 13:54:32 +0300 Subject: [PATCH] gh-125038: Crash after genexpr.gi_frame.f_locals manipulations is fixed Some iterator checks are added for _FOR_ITER, _FOR_ITER_TIER_TWO and INSTRUMENTED_FOR_ITER bytecode implementations. TypeError is raised in case of tp_iternext == NULL. Tests on generator modifying through gi_frame.f_locals are added, both to genexpr generators and function generators. --- Lib/test/test_frame.py | 55 +++++++++++++++++++ ...-10-09-13-53-50.gh-issue-125038.ffSLCz.rst | 3 + Python/bytecodes.c | 33 ++++++++++- Python/executor_cases.c.h | 13 ++++- Python/generated_cases.c.h | 26 ++++++++- 5 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-10-09-13-53-50.gh-issue-125038.ffSLCz.rst diff --git a/Lib/test/test_frame.py b/Lib/test/test_frame.py index 32de8ed9a13f80..80c5379b69ca4b 100644 --- a/Lib/test/test_frame.py +++ b/Lib/test/test_frame.py @@ -515,6 +515,61 @@ def make_frame(): with self.assertRaises(TypeError): FrameLocalsProxy(frame=sys._getframe()) # no keyword arguments + def test_generator_f_locals(self): + def get_generator_genexpr(new_value): + g = (x for x in range(10)) + g.gi_frame.f_locals['.0'] = new_value + return g + + def get_generator_fn_call(new_value): + def gen(it): + for x in it: + yield x + + g = gen(range(10)) + g.gi_frame.f_locals['it'] = new_value + return g + + err_msg_pattern_genexpr = "'for' requires an object with __iter__ method, got %s" + err_msg_pattern_fn_call = "'%s' object is not iterable" + + sequences = [ + range(0), + range(20), + [1, 2, 3], + (2,), + set((13, 48, 211)), + frozenset((15, 8, 6)), + dict([(1, 2), (3, 4)]), + ] + + for seq in sequences: + err_msg_genexpr = err_msg_pattern_genexpr % type(seq).__name__ + with self.assertRaisesRegex(TypeError, err_msg_genexpr): + list(get_generator_genexpr(seq)) + self.assertListEqual(list(get_generator_genexpr(iter(seq))), + list(seq)) + + self.assertListEqual(list(get_generator_fn_call(seq)), + list(seq)) + self.assertListEqual(list(get_generator_fn_call(iter(seq))), + list(seq)) + + non_sequences = [ + None, + 42, + 3.0, + 2j, + ] + + for obj in non_sequences: + err_msg_genexpr = err_msg_pattern_genexpr % type(obj).__name__ + with self.assertRaisesRegex(TypeError, err_msg_genexpr): + list(get_generator_genexpr(obj)) + err_msg_fn_call = err_msg_pattern_fn_call % type(obj).__name__ + with self.assertRaisesRegex(TypeError, err_msg_fn_call): + list(get_generator_fn_call(obj)) + class FrameLocalsProxyMappingTests(mapping_tests.TestHashMappingProtocol): """Test that FrameLocalsProxy behaves like a Mapping (with exceptions)""" diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-10-09-13-53-50.gh-issue-125038.ffSLCz.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-09-13-53-50.gh-issue-125038.ffSLCz.rst new file mode 100644 index 00000000000000..4c85f5a127ed83 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-09-13-53-50.gh-issue-125038.ffSLCz.rst @@ -0,0 +1,3 @@ +Fix crash when iterating over a generator expression after direct changes on ``gi_frame.f_locals``. +Tests on generator modifying through ``gi_frame.f_locals`` are added, +both to genexpr generators and function generators. Patch by Mikhail Efimov. diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 228d82173e6126..92cb031f83e7e5 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2806,7 +2806,16 @@ dummy_func( replaced op(_FOR_ITER, (iter -- iter, next)) { /* before: [iter]; after: [iter, iter()] *or* [] (and jump over END_FOR.) */ PyObject *iter_o = PyStackRef_AsPyObjectBorrow(iter); - PyObject *next_o = (*Py_TYPE(iter_o)->tp_iternext)(iter_o); + PyTypeObject *type = Py_TYPE(iter_o); + iternextfunc iternext = type->tp_iternext; + if (iternext == NULL) { + _PyErr_Format(tstate, PyExc_TypeError, + "'for' requires an object with " + "__iter__ method, got %.100s", + type->tp_name); + ERROR_NO_POP(); + } + PyObject *next_o = (*iternext)(iter_o); if (next_o == NULL) { if (_PyErr_Occurred(tstate)) { int matches = _PyErr_ExceptionMatches(tstate, PyExc_StopIteration); @@ -2832,7 +2841,16 @@ dummy_func( op(_FOR_ITER_TIER_TWO, (iter -- iter, next)) { /* before: [iter]; after: [iter, iter()] *or* [] (and jump over END_FOR.) */ PyObject *iter_o = PyStackRef_AsPyObjectBorrow(iter); - PyObject *next_o = (*Py_TYPE(iter_o)->tp_iternext)(iter_o); + PyTypeObject *type = Py_TYPE(iter_o); + iternextfunc iternext = type->tp_iternext; + if (iternext == NULL) { + _PyErr_Format(tstate, PyExc_TypeError, + "'for' requires an object with " + "__iter__ method, got %.100s", + type->tp_name); + ERROR_NO_POP(); + } + PyObject *next_o = (*iternext)(iter_o); if (next_o == NULL) { if (_PyErr_Occurred(tstate)) { int matches = _PyErr_ExceptionMatches(tstate, PyExc_StopIteration); @@ -2856,7 +2874,16 @@ dummy_func( _Py_CODEUNIT *target; _PyStackRef iter_stackref = TOP(); PyObject *iter = PyStackRef_AsPyObjectBorrow(iter_stackref); - PyObject *next = (*Py_TYPE(iter)->tp_iternext)(iter); + PyTypeObject *type = Py_TYPE(iter); + iternextfunc iternext = type->tp_iternext; + if (iternext == NULL) { + _PyErr_Format(tstate, PyExc_TypeError, + "'for' requires an object with " + "__iter__ method, got %.100s", + type->tp_name); + ERROR_NO_POP(); + } + PyObject *next = (*iternext)(iter); if (next != NULL) { PUSH(PyStackRef_FromPyObjectSteal(next)); target = next_instr; diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 4574e183921006..f260e59e7f6a63 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -3337,8 +3337,19 @@ iter = stack_pointer[-1]; /* before: [iter]; after: [iter, iter()] *or* [] (and jump over END_FOR.) */ PyObject *iter_o = PyStackRef_AsPyObjectBorrow(iter); + PyTypeObject *type = Py_TYPE(iter_o); + iternextfunc iternext = type->tp_iternext; + if (iternext == NULL) { + _PyFrame_SetStackPointer(frame, stack_pointer); + _PyErr_Format(tstate, PyExc_TypeError, + "'for' requires an object with " + "__iter__ method, got %.100s", + type->tp_name); + stack_pointer = _PyFrame_GetStackPointer(frame); + JUMP_TO_ERROR(); + } _PyFrame_SetStackPointer(frame, stack_pointer); - PyObject *next_o = (*Py_TYPE(iter_o)->tp_iternext)(iter_o); + PyObject *next_o = (*iternext)(iter_o); stack_pointer = _PyFrame_GetStackPointer(frame); if (next_o == NULL) { if (_PyErr_Occurred(tstate)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index e1107caa197d7a..830e4f69c71c92 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3921,8 +3921,19 @@ { /* before: [iter]; after: [iter, iter()] *or* [] (and jump over END_FOR.) */ PyObject *iter_o = PyStackRef_AsPyObjectBorrow(iter); + PyTypeObject *type = Py_TYPE(iter_o); + iternextfunc iternext = type->tp_iternext; + if (iternext == NULL) { + _PyFrame_SetStackPointer(frame, stack_pointer); + _PyErr_Format(tstate, PyExc_TypeError, + "'for' requires an object with " + "__iter__ method, got %.100s", + type->tp_name); + stack_pointer = _PyFrame_GetStackPointer(frame); + goto error; + } _PyFrame_SetStackPointer(frame, stack_pointer); - PyObject *next_o = (*Py_TYPE(iter_o)->tp_iternext)(iter_o); + PyObject *next_o = (*iternext)(iter_o); stack_pointer = _PyFrame_GetStackPointer(frame); if (next_o == NULL) { if (_PyErr_Occurred(tstate)) { @@ -4614,8 +4625,19 @@ _Py_CODEUNIT *target; _PyStackRef iter_stackref = TOP(); PyObject *iter = PyStackRef_AsPyObjectBorrow(iter_stackref); + PyTypeObject *type = Py_TYPE(iter); + iternextfunc iternext = type->tp_iternext; + if (iternext == NULL) { + _PyFrame_SetStackPointer(frame, stack_pointer); + _PyErr_Format(tstate, PyExc_TypeError, + "'for' requires an object with " + "__iter__ method, got %.100s", + type->tp_name); + stack_pointer = _PyFrame_GetStackPointer(frame); + goto error; + } _PyFrame_SetStackPointer(frame, stack_pointer); - PyObject *next = (*Py_TYPE(iter)->tp_iternext)(iter); + PyObject *next = (*iternext)(iter); stack_pointer = _PyFrame_GetStackPointer(frame); if (next != NULL) { PUSH(PyStackRef_FromPyObjectSteal(next));