From f012f05a359f9f2a743e8091762b5d370c7e6f92 Mon Sep 17 00:00:00 2001 From: Anselm Kruis Date: Sun, 20 Jun 2021 23:15:59 +0200 Subject: [PATCH 1/3] Stackless issue #270: Remove duplicated code Integrate the C-function PyEval_EvalFrameEx_slp into into the C-function slp_eval_frame_value and rename slp_eval_frame_value to PyEval_EvalFrameEx_slp. Adapt the gdb support library and document the change. --- Lib/test/test_gdb.py | 5 +- Python/ceval.c | 184 ++++++++++++---------------------------- Stackless/changelog.txt | 5 ++ Tools/gdb/libpython.py | 4 +- 4 files changed, 67 insertions(+), 131 deletions(-) diff --git a/Lib/test/test_gdb.py b/Lib/test/test_gdb.py index 4d1ce4ed96c06d..b22f5f39fd5229 100644 --- a/Lib/test/test_gdb.py +++ b/Lib/test/test_gdb.py @@ -741,8 +741,11 @@ def test_down_at_bottom(self): @unittest.skipUnless(HAS_PYUP_PYDOWN, "test requires py-up/py-down commands") def test_up_at_top(self): 'Verify handling of "py-up" at the top of the stack' + n = 5 + if support.stackless: + n += 1 bt = self.get_stack_trace(script=self.get_sample_script(), - cmds_after_breakpoint=['py-up'] * 5) + cmds_after_breakpoint=['py-up'] * n) self.assertEndsWith(bt, 'Unable to find an older python frame\n') diff --git a/Python/ceval.c b/Python/ceval.c index 1ff3db0c4172a5..9b7d3b3ff69d45 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -673,7 +673,7 @@ do { \ PyObject* _Py_HOT_FUNCTION #ifdef STACKLESS -slp_eval_frame_value(PyFrameObject *f, int throwflag, PyObject *retval) +PyEval_EvalFrameEx_slp(PyFrameObject *f, int throwflag, PyObject *retval_arg) #else _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) #endif @@ -686,9 +686,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) int opcode; /* Current opcode */ int oparg; /* Current opcode argument, if any */ PyObject **fastlocals, **freevars; -#ifndef STACKLESS PyObject *retval = NULL; /* Return value */ -#endif PyThreadState *tstate = _PyThreadState_GET(); PyCodeObject *co; @@ -970,8 +968,6 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) /* Stackless specific defines start here.. */ #ifdef STACKLESS - int executing = f->f_executing; - #define SLP_CHECK_INTERRUPT() \ if (tstate->st.interrupt && !tstate->curexc_type) { \ if (tstate->st.tick_counter > tstate->st.tick_watermark) { \ @@ -989,6 +985,34 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) } \ tstate->st.tick_counter++; + int executing = f->f_executing; + if (executing == SLP_FRAME_EXECUTING_INVALID) { + --tstate->recursion_depth; + return slp_cannot_execute((PyCFrameObject *)f, "PyEval_EvalFrameEx_slp", retval_arg); + } else if (f->f_executing != SLP_FRAME_EXECUTING_NO) { + goto slp_setup_completed; + } + + /* Check, if an extension module has changed tstate->interp->eval_frame. + * PEP 523 defines this function pointer as an API to customise the frame + * evaluation. Stackless can not support this API. In order to prevent + * undefined behavior, we terminate the interpreter. + */ + if (tstate->interp->eval_frame != _PyEval_EvalFrameDefault) + Py_FatalError("An extension module has set a custom frame evaluation function (see PEP 523).\n" + "Stackless Python does not support the frame evaluation API defined by PEP 523.\n" + "The programm now terminates to prevent undefined behavior.\n"); + + if (SLP_CSTACK_SAVE_NOW(tstate, f)) + return slp_eval_frame_newstack(f, throwflag, retval_arg); + + /* push frame */ + if (Py_EnterRecursiveCall("")) { + Py_XDECREF(retval_arg); + SLP_STORE_NEXT_FRAME(tstate, f->f_back); + return NULL; + } + #else #define SLP_CHECK_INTERRUPT() ; @@ -997,11 +1021,8 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) /* push frame */ if (Py_EnterRecursiveCall("")) return NULL; +#endif - /* STACKLESS: - * the code starting from here on until the end-marker - * is duplicated below. Keep the two copies in sync! - */ tstate->frame = f; if (tstate->use_tracing) { @@ -1037,10 +1058,10 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) } } } - /* STACKLESS: end of duplicated code - */ - -#endif /* #ifdef STACKLESS */ +#ifdef STACKLESS + executing = SLP_FRAME_EXECUTING_NOVAL; +slp_setup_completed: +#endif if (PyDTrace_FUNCTION_ENTRY_ENABLED()) dtrace_function_entry(f); @@ -1086,14 +1107,14 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) lltrace = _PyDict_GetItemId(f->f_globals, &PyId___ltrace__) != NULL; #endif - if (throwflag) { /* support for generator.throw() */ - assert(retval == NULL); /* to prevent reference leaks */ + if (throwflag) /* support for generator.throw() */ goto error; - } - #ifdef STACKLESS assert(f->f_executing == SLP_FRAME_EXECUTING_VALUE); + assert(retval == NULL); + retval = retval_arg; + retval_arg = NULL; switch(executing){ case SLP_FRAME_EXECUTING_NOVAL: /* don't push it, frame ignores value */ @@ -2026,12 +2047,14 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) STACKLESS_ASSERT(); } else { _Py_IDENTIFIER(send); - if (v == Py_None) { + if (v == Py_None) + { STACKLESS_PROPOSE_METHOD(tstate, receiver, tp_iternext); retval = Py_TYPE(receiver)->tp_iternext(receiver); STACKLESS_ASSERT(); } - else { + else + { STACKLESS_PROPOSE_ALL(tstate); retval = _PyObject_CallMethodIdObjArgs(receiver, &PyId_send, v, NULL); STACKLESS_ASSERT(); @@ -2043,7 +2066,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) HANDLE_UNWINDING(SLP_FRAME_EXECUTING_YIELD_FROM, 0, retval); } if (0) { - slp_continue_slp_eval_frame_yield_from: +slp_continue_slp_eval_frame_yield_from: /* Initialize variables */ receiver = TOP(); } @@ -3132,7 +3155,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) HANDLE_UNWINDING(SLP_FRAME_EXECUTING_ITER, 1, next); } if (0) { - slp_continue_slp_eval_frame_iter: +slp_continue_slp_eval_frame_iter: SLP_SET_OPCODE_AND_OPARG(); assert(opcode == FOR_ITER); @@ -3234,7 +3257,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) HANDLE_UNWINDING(SLP_FRAME_EXECUTING_SETUP_WITH, 1, res); } if(0) { - slp_continue_slp_eval_frame_setup_with: +slp_continue_slp_eval_frame_setup_with: SLP_SET_OPCODE_AND_OPARG(); assert(opcode == SETUP_WITH); /* Initialize variables */ @@ -3323,7 +3346,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) HANDLE_UNWINDING(SLP_FRAME_EXECUTING_WITH_CLEANUP, 0, res); } if (0) { - slp_continue_slp_eval_frame_with_cleanup: +slp_continue_slp_eval_frame_with_cleanup: /* Initialize variables */ exc = TOP(); if (exc == NULL) @@ -3549,6 +3572,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) Py_DECREF(func); Py_DECREF(callargs); Py_XDECREF(kwargs); + #ifdef STACKLESS if (STACKLESS_UNWINDING(result)) { (void) POP(); /* top of stack causes a GC related assertion error */ @@ -3798,29 +3822,24 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) } /* pop frame */ -/* exit_eval_frame: */ -#ifndef STACKLESS exit_eval_frame: if (PyDTrace_FUNCTION_RETURN_ENABLED()) dtrace_function_return(f); Py_LeaveRecursiveCall(); f->f_executing = 0; - tstate->frame = f->f_back; - - return _Py_CheckFunctionResult(NULL, retval, "PyEval_EvalFrameEx"); - -#else - if (PyDTrace_FUNCTION_RETURN_ENABLED()) - dtrace_function_return(f); - Py_LeaveRecursiveCall(); - f->f_executing = 0; +#ifdef STACKLESS SLP_STORE_NEXT_FRAME(tstate, f->f_back); + Py_CLEAR(retval_arg); +#else + tstate->frame = f->f_back; +#endif return _Py_CheckFunctionResult(NULL, retval, "PyEval_EvalFrameEx"); - +#ifdef STACKLESS stackless_interrupt_call: /* interrupted during unwinding */ + assert(retval_arg == NULL); assert(f->f_executing == SLP_FRAME_EXECUTING_VALUE); f->f_executing = SLP_FRAME_EXECUTING_NOVAL; f->f_stacktop = stack_pointer; @@ -4000,98 +4019,6 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) assert(SLP_CURRENT_FRAME_IS_VALID(tstate)); return retval; } - -PyObject * _Py_HOT_FUNCTION -PyEval_EvalFrameEx_slp(PyFrameObject *f, int throwflag, PyObject *retval) -{ - if (f->f_executing == SLP_FRAME_EXECUTING_INVALID) { - PyThreadState *tstate = _PyThreadState_GET(); - --tstate->recursion_depth; - return slp_cannot_execute((PyCFrameObject *)f, "PyEval_EvalFrameEx_slp", retval); - } else if (f->f_executing != SLP_FRAME_EXECUTING_NO) { - return slp_eval_frame_value(f, throwflag, retval); - } - - PyThreadState *tstate = _PyThreadState_GET(); - - /* Check, if an extension module has changed tstate->interp->eval_frame. - * PEP 523 defines this function pointer as an API to customise the frame - * evaluation. Stackless can not support this API. In order to prevent - * undefined behavior, we terminate the interpreter. - */ - if (tstate->interp->eval_frame != _PyEval_EvalFrameDefault) - Py_FatalError("An extension module has set a custom frame evaluation function (see PEP 523).\n" - "Stackless Python does not support the frame evaluation API defined by PEP 523.\n" - "The programm now terminates to prevent undefined behavior.\n"); - - /* Start of code, similar to non stackless - * PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) - */ - - if (SLP_CSTACK_SAVE_NOW(tstate, f)) - return slp_eval_frame_newstack(f, throwflag, retval); - - /* push frame */ - if (Py_EnterRecursiveCall("")) { - Py_XDECREF(retval); - SLP_STORE_NEXT_FRAME(tstate, f->f_back); - return NULL; - } - - /* STACKLESS: - * the code starting from here on until the end-marker - * is a copy of code above. Keep the two copies in sync! - */ - tstate->frame = f; - - if (tstate->use_tracing) { - if (tstate->c_tracefunc != NULL) { - /* tstate->c_tracefunc, if defined, is a - function that will be called on *every* entry - to a code block. Its return value, if not - None, is a function that will be called at - the start of each executed line of code. - (Actually, the function must return itself - in order to continue tracing.) The trace - functions are called with three arguments: - a pointer to the current frame, a string - indicating why the function is called, and - an argument which depends on the situation. - The global trace function is also called - whenever an exception is detected. */ - if (call_trace_protected(tstate->c_tracefunc, - tstate->c_traceobj, - tstate, f, PyTrace_CALL, Py_None)) { - /* Trace function raised an error */ - goto exit_eval_frame; - } - } - if (tstate->c_profilefunc != NULL) { - /* Similar for c_profilefunc, except it needn't - return itself and isn't called for "line" events */ - if (call_trace_protected(tstate->c_profilefunc, - tstate->c_profileobj, - tstate, f, PyTrace_CALL, Py_None)) { - /* Profile function raised an error */ - goto exit_eval_frame; - } - } - } - /* STACKLESS: end of duplicated code - */ - - - f->f_executing = SLP_FRAME_EXECUTING_NOVAL; - return slp_eval_frame_value(f, throwflag, retval); -exit_eval_frame: - Py_XDECREF(retval); - if (PyDTrace_FUNCTION_RETURN_ENABLED()) - dtrace_function_return(f); - Py_LeaveRecursiveCall(); - f->f_executing = SLP_FRAME_EXECUTING_NO; - SLP_STORE_NEXT_FRAME(tstate, f->f_back); - return NULL; -} #endif /* #ifdef STACKLESS */ @@ -4288,7 +4215,6 @@ _PyEval_EvalCodeWithName(PyObject *_co, PyObject *globals, PyObject *locals, if (f == NULL) { return NULL; } - fastlocals = f->f_localsplus; freevars = f->f_localsplus + co->co_nlocals; @@ -5076,7 +5002,7 @@ PyObject * PyEval_GetGlobals(void) { PyFrameObject *current_frame = PyEval_GetFrame(); -#if 1 && defined STACKLESS +#ifdef STACKLESS if (current_frame == NULL) { PyThreadState *ts = _PyThreadState_GET(); diff --git a/Stackless/changelog.txt b/Stackless/changelog.txt index ab010f4d5f38a9..46e95036fdc4de 100644 --- a/Stackless/changelog.txt +++ b/Stackless/changelog.txt @@ -11,6 +11,11 @@ What's New in Stackless 3.X.X? - https://github.com/stackless-dev/stackless/issues/270 Stackless now uses an unmodified PyFrameObject structure. + The internal C-function "slp_eval_frame_value" has been integrated into the + C-function "PyEval_EvalFrameEx_slp". As a consequence a gdb backtrace shows + "PyEval_EvalFrameEx_slp" instead of "slp_eval_frame_value". Because + "PyEval_EvalFrameEx_slp" invokes itself recursively to setup the C-stack, + you may observe two C stack frames for the first Python frame. - https://github.com/stackless-dev/stackless/issues/269 A failure to unpickle a frame could cause a NULL pointer access when diff --git a/Tools/gdb/libpython.py b/Tools/gdb/libpython.py index 4be4b2a527fa15..3b836296330b25 100755 --- a/Tools/gdb/libpython.py +++ b/Tools/gdb/libpython.py @@ -1536,7 +1536,9 @@ def is_evalframe(self): try: gdb.lookup_type("PyCFrameObject") # it is Stackless Python - EVALFRAMEEX_FUNCTION_NAME = ('slp_eval_frame_value', 'PyEval_EvalFrame_value') + # - 'PyEval_EvalFrameEx_slp': Stackless starting from version 3.8.0a1 + # - 'slp_eval_frame_value': versions released after Dec 2016 + EVALFRAMEEX_FUNCTION_NAME = ('PyEval_EvalFrameEx_slp', 'slp_eval_frame_value') except gdb.error: # regular CPython EVALFRAMEEX_FUNCTION_NAME = (EVALFRAME,) From 35d1435e2004c9fbf62e25ed6a064b9d616324e3 Mon Sep 17 00:00:00 2001 From: Anselm Kruis Date: Tue, 22 Jun 2021 17:27:34 +0200 Subject: [PATCH 2/3] simplify the code --- Python/ceval.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index 9b7d3b3ff69d45..567ec0b0475d86 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -989,7 +989,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) if (executing == SLP_FRAME_EXECUTING_INVALID) { --tstate->recursion_depth; return slp_cannot_execute((PyCFrameObject *)f, "PyEval_EvalFrameEx_slp", retval_arg); - } else if (f->f_executing != SLP_FRAME_EXECUTING_NO) { + } else if (executing != SLP_FRAME_EXECUTING_NO) { goto slp_setup_completed; } From b016e57703c398650c5fd9ff3f0d6290b60f9b33 Mon Sep 17 00:00:00 2001 From: Anselm Kruis Date: Tue, 22 Jun 2021 18:21:21 +0200 Subject: [PATCH 3/3] Stackless issue #270: Remove duplicated code Back to PyEval_EvalFrameEx_slp and slp_eval_frame_value. But without duplicated code. Revert the gdb changes from the previous commit. --- Lib/test/test_gdb.py | 5 +--- Python/ceval.c | 58 +++++++++++++++++++++++++++-------------- Stackless/changelog.txt | 10 +++---- Tools/gdb/libpython.py | 4 +-- 4 files changed, 44 insertions(+), 33 deletions(-) diff --git a/Lib/test/test_gdb.py b/Lib/test/test_gdb.py index b22f5f39fd5229..4d1ce4ed96c06d 100644 --- a/Lib/test/test_gdb.py +++ b/Lib/test/test_gdb.py @@ -741,11 +741,8 @@ def test_down_at_bottom(self): @unittest.skipUnless(HAS_PYUP_PYDOWN, "test requires py-up/py-down commands") def test_up_at_top(self): 'Verify handling of "py-up" at the top of the stack' - n = 5 - if support.stackless: - n += 1 bt = self.get_stack_trace(script=self.get_sample_script(), - cmds_after_breakpoint=['py-up'] * n) + cmds_after_breakpoint=['py-up'] * 5) self.assertEndsWith(bt, 'Unable to find an older python frame\n') diff --git a/Python/ceval.c b/Python/ceval.c index 567ec0b0475d86..7c9ce5a0564dc7 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -671,10 +671,11 @@ do { \ #endif -PyObject* _Py_HOT_FUNCTION #ifdef STACKLESS -PyEval_EvalFrameEx_slp(PyFrameObject *f, int throwflag, PyObject *retval_arg) +static PyObject* _Py_HOT_FUNCTION +slp_eval_frame_value(PyFrameObject *f, int throwflag, PyObject *retval_arg) #else +PyObject* _Py_HOT_FUNCTION _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) #endif { @@ -966,7 +967,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) Py_XDECREF(traceback); \ } while(0) -/* Stackless specific defines start here.. */ +/* Stackless specific macros and code start here. */ #ifdef STACKLESS #define SLP_CHECK_INTERRUPT() \ if (tstate->st.interrupt && !tstate->curexc_type) { \ @@ -986,26 +987,11 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) tstate->st.tick_counter++; int executing = f->f_executing; - if (executing == SLP_FRAME_EXECUTING_INVALID) { - --tstate->recursion_depth; - return slp_cannot_execute((PyCFrameObject *)f, "PyEval_EvalFrameEx_slp", retval_arg); - } else if (executing != SLP_FRAME_EXECUTING_NO) { + assert(executing != SLP_FRAME_EXECUTING_INVALID); + if (executing != SLP_FRAME_EXECUTING_NO) { goto slp_setup_completed; } - /* Check, if an extension module has changed tstate->interp->eval_frame. - * PEP 523 defines this function pointer as an API to customise the frame - * evaluation. Stackless can not support this API. In order to prevent - * undefined behavior, we terminate the interpreter. - */ - if (tstate->interp->eval_frame != _PyEval_EvalFrameDefault) - Py_FatalError("An extension module has set a custom frame evaluation function (see PEP 523).\n" - "Stackless Python does not support the frame evaluation API defined by PEP 523.\n" - "The programm now terminates to prevent undefined behavior.\n"); - - if (SLP_CSTACK_SAVE_NOW(tstate, f)) - return slp_eval_frame_newstack(f, throwflag, retval_arg); - /* push frame */ if (Py_EnterRecursiveCall("")) { Py_XDECREF(retval_arg); @@ -3907,6 +3893,38 @@ handle_unwinding(int lineno, PyFrameObject *f, return 0; } +PyObject* +PyEval_EvalFrameEx_slp(PyFrameObject *f, int throwflag, PyObject *retval_arg) +{ + PyThreadState *tstate = _PyThreadState_GET(); + int executing = f->f_executing; + if (executing == SLP_FRAME_EXECUTING_INVALID) { + --tstate->recursion_depth; + return slp_cannot_execute((PyCFrameObject *)f, "PyEval_EvalFrameEx_slp", retval_arg); + } else if (executing == SLP_FRAME_EXECUTING_NO) { + /* Processing of a frame starts here */ + + /* Check, if an extension module has changed tstate->interp->eval_frame. + * PEP 523 defines this function pointer as an API to customise the frame + * evaluation. Stackless can not support this API. In order to prevent + * undefined behavior, we terminate the interpreter. + */ + if (tstate->interp->eval_frame != _PyEval_EvalFrameDefault) + Py_FatalError("An extension module has set a custom frame evaluation function (see PEP 523).\n" + "Stackless Python does not support the frame evaluation API defined by PEP 523.\n" + "The programm now terminates to prevent undefined behavior.\n"); + + if (SLP_CSTACK_SAVE_NOW(tstate, f)) { + /* Setup the C-stack and recursively call PyEval_EvalFrameEx_slp with the same arguments. + * SLP_CSTACK_SAVE_NOW(tstate, f) will be false then. + */ + return slp_eval_frame_newstack(f, throwflag, retval_arg); + } + } + return slp_eval_frame_value(f, throwflag, retval_arg); +} + + static PyObject * run_frame_dispatch(PyCFrameObject *cf, int exc, PyObject *retval) { diff --git a/Stackless/changelog.txt b/Stackless/changelog.txt index 46e95036fdc4de..237b3595eeccc9 100644 --- a/Stackless/changelog.txt +++ b/Stackless/changelog.txt @@ -10,12 +10,10 @@ What's New in Stackless 3.X.X? *Release date: 20XX-XX-XX* - https://github.com/stackless-dev/stackless/issues/270 - Stackless now uses an unmodified PyFrameObject structure. - The internal C-function "slp_eval_frame_value" has been integrated into the - C-function "PyEval_EvalFrameEx_slp". As a consequence a gdb backtrace shows - "PyEval_EvalFrameEx_slp" instead of "slp_eval_frame_value". Because - "PyEval_EvalFrameEx_slp" invokes itself recursively to setup the C-stack, - you may observe two C stack frames for the first Python frame. + Stackless now uses an unmodified PyFrameObject structure. Stackless now + stores more state information in the field f->f_executing than C-Python. + If an extension module modifies f->f_executing, undefined behavior can occur. + Reading the field as boolean still works. - https://github.com/stackless-dev/stackless/issues/269 A failure to unpickle a frame could cause a NULL pointer access when diff --git a/Tools/gdb/libpython.py b/Tools/gdb/libpython.py index 3b836296330b25..4be4b2a527fa15 100755 --- a/Tools/gdb/libpython.py +++ b/Tools/gdb/libpython.py @@ -1536,9 +1536,7 @@ def is_evalframe(self): try: gdb.lookup_type("PyCFrameObject") # it is Stackless Python - # - 'PyEval_EvalFrameEx_slp': Stackless starting from version 3.8.0a1 - # - 'slp_eval_frame_value': versions released after Dec 2016 - EVALFRAMEEX_FUNCTION_NAME = ('PyEval_EvalFrameEx_slp', 'slp_eval_frame_value') + EVALFRAMEEX_FUNCTION_NAME = ('slp_eval_frame_value', 'PyEval_EvalFrame_value') except gdb.error: # regular CPython EVALFRAMEEX_FUNCTION_NAME = (EVALFRAME,)