From a278314375c1501e1420f115fd0814eaedb21412 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 12 Nov 2021 14:11:26 +0000 Subject: [PATCH 01/16] Move eval-breaker to CFrame and refactor exit code a bit. --- Include/cpython/pystate.h | 1 + Python/ceval.c | 90 +++++++++++++++++++++++++-------------- Python/pystate.c | 1 + 3 files changed, 60 insertions(+), 32 deletions(-) diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h index cf69c72028a3ac..345450be9b29fb 100644 --- a/Include/cpython/pystate.h +++ b/Include/cpython/pystate.h @@ -48,6 +48,7 @@ typedef struct _cframe { int use_tracing; /* Pointer to the currently executing frame (it can be NULL) */ struct _interpreter_frame *current_frame; + void *eval_breaker; struct _cframe *previous; } CFrame; diff --git a/Python/ceval.c b/Python/ceval.c index 030d98396780b8..de5cce8de40db7 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1301,7 +1301,7 @@ eval_frame_handle_pending(PyThreadState *tstate) } #define CHECK_EVAL_BREAKER() \ - if (_Py_atomic_load_relaxed(eval_breaker)) { \ + if (_Py_atomic_load_relaxed((_Py_atomic_int * const)cframe.eval_breaker)) { \ goto check_eval_breaker; \ } @@ -1514,6 +1514,24 @@ trace_function_entry(PyThreadState *tstate, InterpreterFrame *frame) return 0; } +static int +trace_function_exit(PyThreadState *tstate, InterpreterFrame *frame, PyObject *retval) +{ + if (tstate->c_tracefunc) { + if (call_trace_protected(tstate->c_tracefunc, tstate->c_traceobj, + tstate, frame, PyTrace_RETURN, retval)) { + return -1; + } + } + if (tstate->c_profilefunc) { + if (call_trace_protected(tstate->c_profilefunc, tstate->c_profileobj, + tstate, frame, PyTrace_RETURN, retval)) { + return -1; + } + } + return 0; +} + static PyObject * make_coro(PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals, @@ -1545,7 +1563,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr int opcode; /* Current opcode */ int oparg; /* Current opcode argument, if any */ PyObject *retval = NULL; /* Return value */ - _Py_atomic_int * const eval_breaker = &tstate->interp->ceval.eval_breaker; CFrame cframe; @@ -1555,6 +1572,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr */ CFrame *prev_cframe = tstate->cframe; cframe.use_tracing = prev_cframe->use_tracing; + cframe.eval_breaker = prev_cframe->eval_breaker; cframe.previous = prev_cframe; tstate->cframe = &cframe; @@ -1566,7 +1584,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr start_frame: if (_Py_EnterRecursiveCall(tstate, "")) { tstate->recursion_depth++; - goto exit_eval_frame; + goto exit_unwind_notrace; } assert(tstate->cframe == &cframe); @@ -1574,7 +1592,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr if (cframe.use_tracing) { if (trace_function_entry(tstate, frame)) { - goto exit_eval_frame; + goto exit_unwind_notrace; } } @@ -1588,7 +1606,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr PyCodeObject_IncrementWarmup(co); if (PyCodeObject_IsWarmedUp(co)) { if (_Py_Quicken(co)) { - goto exit_eval_frame; + goto exit_unwind_notrace; } } } @@ -1628,7 +1646,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr { int r = _PyDict_ContainsId(GLOBALS(), &PyId___ltrace__); if (r < 0) { - goto exit_eval_frame; + goto exit_unwind_notrace; } lltrace = r; } @@ -1660,7 +1678,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr async I/O handler); see Py_AddPendingCall() and Py_MakePendingCalls() above. */ - if (_Py_atomic_load_relaxed(eval_breaker)) { + if (_Py_atomic_load_relaxed((_Py_atomic_int * const)cframe.eval_breaker)) { opcode = _Py_OPCODE(*next_instr); if (opcode != BEFORE_ASYNC_WITH && opcode != YIELD_FROM) { @@ -2234,7 +2252,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr assert(EMPTY()); frame->f_state = FRAME_RETURNED; _PyFrame_SetStackPointer(frame, stack_pointer); - goto exiting; + goto exit_return; } TARGET(GET_AITER) { @@ -2422,7 +2440,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr frame->f_lasti -= 1; frame->f_state = FRAME_SUSPENDED; _PyFrame_SetStackPointer(frame, stack_pointer); - goto exiting; + goto exit_return; } TARGET(YIELD_VALUE) { @@ -2439,7 +2457,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } frame->f_state = FRAME_SUSPENDED; _PyFrame_SetStackPointer(frame, stack_pointer); - goto exiting; + goto exit_return; } TARGET(GEN_START) { @@ -4956,7 +4974,7 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) assert(STACK_LEVEL() == 0); _PyFrame_SetStackPointer(frame, stack_pointer); frame->f_state = FRAME_RAISED; - goto exiting; + goto exit_unwind; } assert(STACK_LEVEL() >= level); @@ -4999,43 +5017,51 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) DISPATCH_GOTO(); } -exiting: +exit_unwind: + assert(retval == NULL); if (cframe.use_tracing) { - if (tstate->c_tracefunc) { - if (call_trace_protected(tstate->c_tracefunc, tstate->c_traceobj, - tstate, frame, PyTrace_RETURN, retval)) { - Py_CLEAR(retval); - } - } - if (tstate->c_profilefunc) { - if (call_trace_protected(tstate->c_profilefunc, tstate->c_profileobj, - tstate, frame, PyTrace_RETURN, retval)) { - Py_CLEAR(retval); - } - } + trace_function_exit(tstate, frame, NULL); } - - /* pop frame */ -exit_eval_frame: if (PyDTrace_FUNCTION_RETURN_ENABLED()) dtrace_function_return(frame); + +exit_unwind_notrace: _Py_LeaveRecursiveCall(tstate); + if (frame->depth) { + cframe.current_frame = frame->previous; + _PyEvalFrameClearAndPop(tstate, frame); + frame = cframe.current_frame; + throwflag = 1; + goto resume_frame; + } + + goto exit_eval_frame; + +exit_return: + assert(retval != NULL); + if (cframe.use_tracing) { + if (trace_function_exit(tstate, frame, retval)) { + retval = NULL; + throwflag = 1; + } + } + if (PyDTrace_FUNCTION_RETURN_ENABLED()) + dtrace_function_return(frame); + _Py_LeaveRecursiveCall(tstate); if (frame->depth) { cframe.current_frame = frame->previous; _PyFrame_StackPush(cframe.current_frame, retval); if (_PyEvalFrameClearAndPop(tstate, frame)) { - retval = NULL; - } - frame = cframe.current_frame; - if (retval == NULL) { - assert(_PyErr_Occurred(tstate)); throwflag = 1; } + frame = cframe.current_frame; retval = NULL; goto resume_frame; } +exit_eval_frame: + /* Restore previous cframe. */ tstate->cframe = cframe.previous; tstate->cframe->use_tracing = cframe.use_tracing; diff --git a/Python/pystate.c b/Python/pystate.c index a9ed08a7e34be3..b4880a195e6395 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -642,6 +642,7 @@ new_threadstate(PyInterpreterState *interp, int init) tstate->tracing = 0; tstate->root_cframe.use_tracing = 0; tstate->root_cframe.current_frame = NULL; + tstate->root_cframe.eval_breaker = &interp->ceval.eval_breaker; tstate->cframe = &tstate->root_cframe; tstate->gilstate_counter = 0; tstate->async_exc = NULL; From ba1f6d8a2752a47b4411aca0cedd68de876620a2 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 12 Nov 2021 15:37:59 +0000 Subject: [PATCH 02/16] Split exit paths into exceptional and non-exceptional. --- Python/ceval.c | 104 ++++++++++++++++++++++++++++++------------------- Python/frame.c | 1 + 2 files changed, 64 insertions(+), 41 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index de5cce8de40db7..8b51e6be2d4034 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1475,6 +1475,7 @@ eval_frame_handle_pending(PyThreadState *tstate) STAT_INC(LOAD_##attr_or_method, hit); \ Py_INCREF(res); + static int trace_function_entry(PyThreadState *tstate, InterpreterFrame *frame) { @@ -1539,7 +1540,8 @@ make_coro(PyThreadState *tstate, PyFrameConstructor *con, PyObject *kwnames); static int -skip_backwards_over_extended_args(PyCodeObject *code, int offset) { +skip_backwards_over_extended_args(PyCodeObject *code, int offset) +{ _Py_CODEUNIT *instrs = (_Py_CODEUNIT *)PyBytes_AS_STRING(code->co_code); while (offset > 0 && _Py_OPCODE(instrs[offset-1]) == EXTENDED_ARG) { offset--; @@ -1547,6 +1549,14 @@ skip_backwards_over_extended_args(PyCodeObject *code, int offset) { return offset; } +static InterpreterFrame * +pop_frame(PyThreadState *tstate, InterpreterFrame *frame) +{ + InterpreterFrame *prev_frame = frame->previous; + _PyEvalFrameClearAndPop(tstate, frame); + return prev_frame; +} + PyObject* _Py_HOT_FUNCTION _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int throwflag) { @@ -1581,10 +1591,14 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr frame->previous = prev_cframe->current_frame; cframe.current_frame = frame; + if (throwflag) { /* support for generator.throw() */ + goto throw; + } + start_frame: if (_Py_EnterRecursiveCall(tstate, "")) { tstate->recursion_depth++; - goto exit_unwind_notrace; + goto exit_unwind; } assert(tstate->cframe == &cframe); @@ -1592,7 +1606,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr if (cframe.use_tracing) { if (trace_function_entry(tstate, frame)) { - goto exit_unwind_notrace; + goto exit_unwind; } } @@ -1606,7 +1620,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr PyCodeObject_IncrementWarmup(co); if (PyCodeObject_IsWarmedUp(co)) { if (_Py_Quicken(co)) { - goto exit_unwind_notrace; + goto exit_unwind; } } } @@ -1646,17 +1660,12 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr { int r = _PyDict_ContainsId(GLOBALS(), &PyId___ltrace__); if (r < 0) { - goto exit_unwind_notrace; + goto exit_unwind; } lltrace = r; } #endif - if (throwflag) { /* support for generator.throw() */ - throwflag = 0; - goto error; - } - #ifdef Py_DEBUG /* _PyEval_EvalFrameDefault() must not be called with an exception set, because it can clear it (directly or indirectly) and so the @@ -2392,6 +2401,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } TARGET(YIELD_FROM) { + assert(frame->depth == 0); PyObject *v = POP(); PyObject *receiver = TOP(); PySendResult gen_status; @@ -2444,6 +2454,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } TARGET(YIELD_VALUE) { + assert(frame->depth == 0); retval = POP(); if (co->co_flags & CO_ASYNC_GENERATOR) { @@ -4974,6 +4985,12 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) assert(STACK_LEVEL() == 0); _PyFrame_SetStackPointer(frame, stack_pointer); frame->f_state = FRAME_RAISED; + if (cframe.use_tracing) { + trace_function_exit(tstate, frame, NULL); + } + if (PyDTrace_FUNCTION_RETURN_ENABLED()) { + dtrace_function_return(frame); + } goto exit_unwind; } @@ -5018,55 +5035,63 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) } exit_unwind: - assert(retval == NULL); - if (cframe.use_tracing) { - trace_function_exit(tstate, frame, NULL); - } - if (PyDTrace_FUNCTION_RETURN_ENABLED()) - dtrace_function_return(frame); - -exit_unwind_notrace: + assert(_PyErr_Occurred(tstate)); _Py_LeaveRecursiveCall(tstate); if (frame->depth) { - cframe.current_frame = frame->previous; - _PyEvalFrameClearAndPop(tstate, frame); - frame = cframe.current_frame; - throwflag = 1; - goto resume_frame; + frame = cframe.current_frame = pop_frame(tstate, frame); + goto update_locals_then_error; } - - goto exit_eval_frame; - + /* Restore previous cframe. */ + tstate->cframe = cframe.previous; + tstate->cframe->use_tracing = cframe.use_tracing; + assert(tstate->cframe->current_frame == frame->previous); + return NULL; exit_return: assert(retval != NULL); if (cframe.use_tracing) { if (trace_function_exit(tstate, frame, retval)) { retval = NULL; - throwflag = 1; + goto exit_unwind; } } if (PyDTrace_FUNCTION_RETURN_ENABLED()) dtrace_function_return(frame); _Py_LeaveRecursiveCall(tstate); if (frame->depth) { - cframe.current_frame = frame->previous; - _PyFrame_StackPush(cframe.current_frame, retval); - if (_PyEvalFrameClearAndPop(tstate, frame)) { - throwflag = 1; - } - frame = cframe.current_frame; + frame = cframe.current_frame = pop_frame(tstate, frame); + _PyFrame_StackPush(frame, retval); retval = NULL; goto resume_frame; } - -exit_eval_frame: - /* Restore previous cframe. */ tstate->cframe = cframe.previous; tstate->cframe->use_tracing = cframe.use_tracing; assert(tstate->cframe->current_frame == frame->previous); - return _Py_CheckFunctionResult(tstate, NULL, retval, __func__); + return retval; + +throw: + if (_Py_EnterRecursiveCall(tstate, "")) { + tstate->recursion_depth++; + goto exit_unwind; + } + if (cframe.use_tracing) { + if (trace_function_entry(tstate, frame)) { + goto exit_unwind; + } + } +update_locals_then_error: + co = frame->f_code; + names = co->co_names; + consts = co->co_consts; + first_instr = co->co_firstinstr; + assert(frame->f_lasti >= -1); + next_instr = first_instr + frame->f_lasti + 1; + stack_pointer = _PyFrame_GetStackPointer(frame); + frame->stacktop = -1; + frame->f_state = FRAME_EXECUTING; + goto error; + } static void @@ -5699,10 +5724,7 @@ _PyEvalFrameClearAndPop(PyThreadState *tstate, InterpreterFrame * frame) { ++tstate->recursion_depth; assert(frame->frame_obj == NULL || frame->frame_obj->f_own_locals_memory == 0); - if (_PyFrame_Clear(frame, 0)) { - --tstate->recursion_depth; - return -1; - } + _PyFrame_Clear(frame, 0); --tstate->recursion_depth; _PyThreadState_PopFrame(tstate, frame); return 0; diff --git a/Python/frame.c b/Python/frame.c index 3d2415fee70978..dd57ec3d479be8 100644 --- a/Python/frame.c +++ b/Python/frame.c @@ -96,6 +96,7 @@ take_ownership(PyFrameObject *f, InterpreterFrame *frame) } } +/* FIXME -- See bpo-45786 */ int _PyFrame_Clear(InterpreterFrame * frame, int take) { From aba6ad04893dab6f6e09f356c014a413c55c129f Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 12 Nov 2021 16:14:59 +0000 Subject: [PATCH 03/16] Move exit tracing code to individual bytecodes. --- Python/ceval.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 8b51e6be2d4034..59e57ba60d629e 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1475,6 +1475,13 @@ eval_frame_handle_pending(PyThreadState *tstate) STAT_INC(LOAD_##attr_or_method, hit); \ Py_INCREF(res); +#define TRACE_FUNCTION_EXIT() \ + if (cframe.use_tracing) { \ + if (trace_function_exit(tstate, frame, retval)) { \ + retval = NULL; \ + goto exit_unwind; \ + } \ + } static int trace_function_entry(PyThreadState *tstate, InterpreterFrame *frame) @@ -2261,6 +2268,16 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr assert(EMPTY()); frame->f_state = FRAME_RETURNED; _PyFrame_SetStackPointer(frame, stack_pointer); + TRACE_FUNCTION_EXIT() + if (PyDTrace_FUNCTION_RETURN_ENABLED()) + dtrace_function_return(frame); + _Py_LeaveRecursiveCall(tstate); + if (frame->depth) { + frame = cframe.current_frame = pop_frame(tstate, frame); + _PyFrame_StackPush(frame, retval); + retval = NULL; + goto resume_frame; + } goto exit_return; } @@ -2450,6 +2467,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr frame->f_lasti -= 1; frame->f_state = FRAME_SUSPENDED; _PyFrame_SetStackPointer(frame, stack_pointer); + TRACE_FUNCTION_EXIT() + if (PyDTrace_FUNCTION_RETURN_ENABLED()) + dtrace_function_return(frame); + _Py_LeaveRecursiveCall(tstate); goto exit_return; } @@ -2468,6 +2489,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } frame->f_state = FRAME_SUSPENDED; _PyFrame_SetStackPointer(frame, stack_pointer); + TRACE_FUNCTION_EXIT() + if (PyDTrace_FUNCTION_RETURN_ENABLED()) + dtrace_function_return(frame); + _Py_LeaveRecursiveCall(tstate); goto exit_return; } @@ -5049,21 +5074,6 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) exit_return: assert(retval != NULL); - if (cframe.use_tracing) { - if (trace_function_exit(tstate, frame, retval)) { - retval = NULL; - goto exit_unwind; - } - } - if (PyDTrace_FUNCTION_RETURN_ENABLED()) - dtrace_function_return(frame); - _Py_LeaveRecursiveCall(tstate); - if (frame->depth) { - frame = cframe.current_frame = pop_frame(tstate, frame); - _PyFrame_StackPush(frame, retval); - retval = NULL; - goto resume_frame; - } /* Restore previous cframe. */ tstate->cframe = cframe.previous; tstate->cframe->use_tracing = cframe.use_tracing; From bc9ecb9db5a37bbf1092bf1f8138e6f7a546cf83 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 12 Nov 2021 16:47:02 +0000 Subject: [PATCH 04/16] Wrap all trace entry and exit events in macros to make them clearer and easier to enhance. --- Python/ceval.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 59e57ba60d629e..02f1297063b0fd 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1481,8 +1481,27 @@ eval_frame_handle_pending(PyThreadState *tstate) retval = NULL; \ goto exit_unwind; \ } \ + } \ + if (PyDTrace_FUNCTION_RETURN_ENABLED()) { \ + dtrace_function_return(frame); \ + } + +#define TRACE_FUNCTION_RETURN() TRACE_FUNCTION_EXIT() +#define TRACE_FUNCTION_YIELD() TRACE_FUNCTION_EXIT() + +#define TRACE_FUNCTION_ENTRY() \ + if (cframe.use_tracing) { \ + if (trace_function_entry(tstate, frame)) { \ + goto exit_unwind; \ + } \ + } \ + if (PyDTrace_FUNCTION_ENTRY_ENABLED()) { \ + dtrace_function_entry(frame); \ } +#define TRACE_FUNCTION_THROW_ENTRY() TRACE_FUNCTION_ENTRY() + + static int trace_function_entry(PyThreadState *tstate, InterpreterFrame *frame) { @@ -1611,14 +1630,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr assert(tstate->cframe == &cframe); assert(frame == cframe.current_frame); - if (cframe.use_tracing) { - if (trace_function_entry(tstate, frame)) { - goto exit_unwind; - } - } - - if (PyDTrace_FUNCTION_ENTRY_ENABLED()) - dtrace_function_entry(frame); + TRACE_FUNCTION_ENTRY(); PyCodeObject *co = frame->f_code; /* Increment the warmup counter and quicken if warm enough @@ -2268,9 +2280,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr assert(EMPTY()); frame->f_state = FRAME_RETURNED; _PyFrame_SetStackPointer(frame, stack_pointer); - TRACE_FUNCTION_EXIT() - if (PyDTrace_FUNCTION_RETURN_ENABLED()) - dtrace_function_return(frame); + TRACE_FUNCTION_RETURN(); _Py_LeaveRecursiveCall(tstate); if (frame->depth) { frame = cframe.current_frame = pop_frame(tstate, frame); @@ -2467,9 +2477,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr frame->f_lasti -= 1; frame->f_state = FRAME_SUSPENDED; _PyFrame_SetStackPointer(frame, stack_pointer); - TRACE_FUNCTION_EXIT() - if (PyDTrace_FUNCTION_RETURN_ENABLED()) - dtrace_function_return(frame); + TRACE_FUNCTION_YIELD(); _Py_LeaveRecursiveCall(tstate); goto exit_return; } @@ -2489,9 +2497,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } frame->f_state = FRAME_SUSPENDED; _PyFrame_SetStackPointer(frame, stack_pointer); - TRACE_FUNCTION_EXIT() - if (PyDTrace_FUNCTION_RETURN_ENABLED()) - dtrace_function_return(frame); + TRACE_FUNCTION_YIELD(); _Py_LeaveRecursiveCall(tstate); goto exit_return; } @@ -5085,11 +5091,7 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) tstate->recursion_depth++; goto exit_unwind; } - if (cframe.use_tracing) { - if (trace_function_entry(tstate, frame)) { - goto exit_unwind; - } - } + TRACE_FUNCTION_THROW_ENTRY(); update_locals_then_error: co = frame->f_code; names = co->co_names; From b7c34448e4a01e96204c7fbe095a5a76a10414e9 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 15 Nov 2021 13:52:40 +0000 Subject: [PATCH 05/16] Move return sequence into RETURN_VALUE, YIELD_VALUE and YIELD_FROM. Distinguish between normal trace events and dtrace events. --- Python/ceval.c | 53 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 02f1297063b0fd..c333bbe5213006 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1481,7 +1481,9 @@ eval_frame_handle_pending(PyThreadState *tstate) retval = NULL; \ goto exit_unwind; \ } \ - } \ + } + +#define DTRACE_FUNCTION_EXIT() \ if (PyDTrace_FUNCTION_RETURN_ENABLED()) { \ dtrace_function_return(frame); \ } @@ -1489,12 +1491,19 @@ eval_frame_handle_pending(PyThreadState *tstate) #define TRACE_FUNCTION_RETURN() TRACE_FUNCTION_EXIT() #define TRACE_FUNCTION_YIELD() TRACE_FUNCTION_EXIT() +#define TRACE_FUNCTION_UNWIND() \ + if (cframe.use_tracing) { \ + trace_function_exit(tstate, frame, NULL); \ + } + #define TRACE_FUNCTION_ENTRY() \ if (cframe.use_tracing) { \ if (trace_function_entry(tstate, frame)) { \ goto exit_unwind; \ } \ - } \ + } + +#define DTRACE_FUNCTION_ENTRY() \ if (PyDTrace_FUNCTION_ENTRY_ENABLED()) { \ dtrace_function_entry(frame); \ } @@ -1631,6 +1640,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr assert(frame == cframe.current_frame); TRACE_FUNCTION_ENTRY(); + DTRACE_FUNCTION_ENTRY(); PyCodeObject *co = frame->f_code; /* Increment the warmup counter and quicken if warm enough @@ -2281,6 +2291,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr frame->f_state = FRAME_RETURNED; _PyFrame_SetStackPointer(frame, stack_pointer); TRACE_FUNCTION_RETURN(); + DTRACE_FUNCTION_EXIT(); _Py_LeaveRecursiveCall(tstate); if (frame->depth) { frame = cframe.current_frame = pop_frame(tstate, frame); @@ -2288,7 +2299,11 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr retval = NULL; goto resume_frame; } - goto exit_return; + /* Restore previous cframe and return. */ + tstate->cframe = cframe.previous; + tstate->cframe->use_tracing = cframe.use_tracing; + assert(tstate->cframe->current_frame == frame->previous); + return retval; } TARGET(GET_AITER) { @@ -2478,8 +2493,13 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr frame->f_state = FRAME_SUSPENDED; _PyFrame_SetStackPointer(frame, stack_pointer); TRACE_FUNCTION_YIELD(); + DTRACE_FUNCTION_EXIT(); _Py_LeaveRecursiveCall(tstate); - goto exit_return; + /* Restore previous cframe and return. */ + tstate->cframe = cframe.previous; + tstate->cframe->use_tracing = cframe.use_tracing; + assert(tstate->cframe->current_frame == frame->previous); + return retval; } TARGET(YIELD_VALUE) { @@ -2498,8 +2518,13 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr frame->f_state = FRAME_SUSPENDED; _PyFrame_SetStackPointer(frame, stack_pointer); TRACE_FUNCTION_YIELD(); + DTRACE_FUNCTION_EXIT(); _Py_LeaveRecursiveCall(tstate); - goto exit_return; + /* Restore previous cframe and return. */ + tstate->cframe = cframe.previous; + tstate->cframe->use_tracing = cframe.use_tracing; + assert(tstate->cframe->current_frame == frame->previous); + return retval; } TARGET(GEN_START) { @@ -5016,12 +5041,8 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) assert(STACK_LEVEL() == 0); _PyFrame_SetStackPointer(frame, stack_pointer); frame->f_state = FRAME_RAISED; - if (cframe.use_tracing) { - trace_function_exit(tstate, frame, NULL); - } - if (PyDTrace_FUNCTION_RETURN_ENABLED()) { - dtrace_function_return(frame); - } + TRACE_FUNCTION_UNWIND(); + DTRACE_FUNCTION_EXIT(); goto exit_unwind; } @@ -5078,20 +5099,14 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) assert(tstate->cframe->current_frame == frame->previous); return NULL; -exit_return: - assert(retval != NULL); - /* Restore previous cframe. */ - tstate->cframe = cframe.previous; - tstate->cframe->use_tracing = cframe.use_tracing; - assert(tstate->cframe->current_frame == frame->previous); - return retval; - throw: if (_Py_EnterRecursiveCall(tstate, "")) { tstate->recursion_depth++; goto exit_unwind; } TRACE_FUNCTION_THROW_ENTRY(); + DTRACE_FUNCTION_ENTRY(); + update_locals_then_error: co = frame->f_code; names = co->co_names; From 98a4b05638d8d5a686e46181f04039b391de4222 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 15 Nov 2021 14:08:53 +0000 Subject: [PATCH 06/16] A bit more tidying up of exceptional paths. --- Python/ceval.c | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index c333bbe5213006..5a50ea71362519 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1626,8 +1626,15 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr frame->previous = prev_cframe->current_frame; cframe.current_frame = frame; - if (throwflag) { /* support for generator.throw() */ - goto throw; + /* support for generator.throw() */ + if (throwflag) { + if (_Py_EnterRecursiveCall(tstate, "")) { + tstate->recursion_depth++; + goto exit_unwind; + } + TRACE_FUNCTION_THROW_ENTRY(); + DTRACE_FUNCTION_ENTRY(); + goto update_locals_then_error; } start_frame: @@ -5089,23 +5096,14 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) exit_unwind: assert(_PyErr_Occurred(tstate)); _Py_LeaveRecursiveCall(tstate); - if (frame->depth) { - frame = cframe.current_frame = pop_frame(tstate, frame); - goto update_locals_then_error; - } - /* Restore previous cframe. */ - tstate->cframe = cframe.previous; - tstate->cframe->use_tracing = cframe.use_tracing; - assert(tstate->cframe->current_frame == frame->previous); - return NULL; - -throw: - if (_Py_EnterRecursiveCall(tstate, "")) { - tstate->recursion_depth++; - goto exit_unwind; + if (frame->depth == 0) { + /* Restore previous cframe and exit */ + tstate->cframe = cframe.previous; + tstate->cframe->use_tracing = cframe.use_tracing; + assert(tstate->cframe->current_frame == frame->previous); + return NULL; } - TRACE_FUNCTION_THROW_ENTRY(); - DTRACE_FUNCTION_ENTRY(); + frame = cframe.current_frame = pop_frame(tstate, frame); update_locals_then_error: co = frame->f_code; From cffe72df4c8242a3264641b1bcf2bd8629b725bd Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 15 Nov 2021 14:23:33 +0000 Subject: [PATCH 07/16] Tidy up quickening check. --- Include/internal/pycore_code.h | 23 ++++++++++++----------- Python/ceval.c | 29 +++++++++-------------------- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index 7fe9e74b21cfeb..310de6c1e3acb7 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -136,24 +136,25 @@ _GetSpecializedCacheEntryForInstruction(const _Py_CODEUNIT *first_instr, int nex #define QUICKENING_INITIAL_WARMUP_VALUE (-QUICKENING_WARMUP_DELAY) #define QUICKENING_WARMUP_COLDEST 1 -static inline void -PyCodeObject_IncrementWarmup(PyCodeObject * co) -{ - co->co_warmup++; -} +int _Py_Quicken(PyCodeObject *code); -/* Used by the interpreter to determine when a code object should be quickened */ +/* Returns 1 if quickening occurs. + * -1 if an error occurs + * 0 otherwise */ static inline int -PyCodeObject_IsWarmedUp(PyCodeObject * co) +_Py_IncrementCountAndMaybeQuicken(PyCodeObject *code) { - return (co->co_warmup == 0); + if (code->co_warmup != 0) { + code->co_warmup++; + if (code->co_warmup == 0) { + return _Py_Quicken(code) ? -1 : 1; + } + } + return 0; } -int _Py_Quicken(PyCodeObject *code); - extern Py_ssize_t _Py_QuickenedCount; - /* "Locals plus" for a code object is the set of locals + cell vars + * free vars. This relates to variable names as well as offsets into * the "fast locals" storage array of execution frames. The compiler diff --git a/Python/ceval.c b/Python/ceval.c index 5a50ea71362519..9b5a83f2661ffd 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1650,15 +1650,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr DTRACE_FUNCTION_ENTRY(); PyCodeObject *co = frame->f_code; - /* Increment the warmup counter and quicken if warm enough - * _Py_Quicken is idempotent so we don't worry about overflow */ - if (!PyCodeObject_IsWarmedUp(co)) { - PyCodeObject_IncrementWarmup(co); - if (PyCodeObject_IsWarmedUp(co)) { - if (_Py_Quicken(co)) { - goto exit_unwind; - } - } + if (_Py_IncrementCountAndMaybeQuicken(co)) { + goto exit_unwind; } resume_frame: @@ -3823,18 +3816,14 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr TARGET(JUMP_ABSOLUTE) { PREDICTED(JUMP_ABSOLUTE); assert(oparg < INSTR_OFFSET()); - /* Increment the warmup counter and quicken if warm enough - * _Py_Quicken is idempotent so we don't worry about overflow */ - if (!PyCodeObject_IsWarmedUp(co)) { - PyCodeObject_IncrementWarmup(co); - if (PyCodeObject_IsWarmedUp(co)) { - if (_Py_Quicken(co)) { - goto error; - } - int nexti = INSTR_OFFSET(); - first_instr = co->co_firstinstr; - next_instr = first_instr + nexti; + int err = _Py_IncrementCountAndMaybeQuicken(co); + if (err) { + if (err < 0) { + goto error; } + int nexti = INSTR_OFFSET(); + first_instr = co->co_firstinstr; + next_instr = first_instr + nexti; } JUMPTO(oparg); CHECK_EVAL_BREAKER(); From a28e7d6232d8af4742e5b37c3cf253232c096f6a Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 15 Nov 2021 15:29:52 +0000 Subject: [PATCH 08/16] Revert change to eval_breaker --- Include/cpython/pystate.h | 1 - Python/ceval.c | 8 ++++---- Python/pystate.c | 1 - 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h index 345450be9b29fb..cf69c72028a3ac 100644 --- a/Include/cpython/pystate.h +++ b/Include/cpython/pystate.h @@ -48,7 +48,6 @@ typedef struct _cframe { int use_tracing; /* Pointer to the currently executing frame (it can be NULL) */ struct _interpreter_frame *current_frame; - void *eval_breaker; struct _cframe *previous; } CFrame; diff --git a/Python/ceval.c b/Python/ceval.c index 9b5a83f2661ffd..205c5fd4620569 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1301,7 +1301,7 @@ eval_frame_handle_pending(PyThreadState *tstate) } #define CHECK_EVAL_BREAKER() \ - if (_Py_atomic_load_relaxed((_Py_atomic_int * const)cframe.eval_breaker)) { \ + if (_Py_atomic_load_relaxed(eval_breaker)) { \ goto check_eval_breaker; \ } @@ -1608,6 +1608,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr int opcode; /* Current opcode */ int oparg; /* Current opcode argument, if any */ PyObject *retval = NULL; /* Return value */ + _Py_atomic_int * const eval_breaker = &tstate->interp->ceval.eval_breaker; CFrame cframe; @@ -1617,7 +1618,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr */ CFrame *prev_cframe = tstate->cframe; cframe.use_tracing = prev_cframe->use_tracing; - cframe.eval_breaker = prev_cframe->eval_breaker; cframe.previous = prev_cframe; tstate->cframe = &cframe; @@ -1650,7 +1650,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr DTRACE_FUNCTION_ENTRY(); PyCodeObject *co = frame->f_code; - if (_Py_IncrementCountAndMaybeQuicken(co)) { + if (_Py_IncrementCountAndMaybeQuicken(co) < 0) { goto exit_unwind; } @@ -1716,7 +1716,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr async I/O handler); see Py_AddPendingCall() and Py_MakePendingCalls() above. */ - if (_Py_atomic_load_relaxed((_Py_atomic_int * const)cframe.eval_breaker)) { + if (_Py_atomic_load_relaxed(eval_breaker)) { opcode = _Py_OPCODE(*next_instr); if (opcode != BEFORE_ASYNC_WITH && opcode != YIELD_FROM) { diff --git a/Python/pystate.c b/Python/pystate.c index b4880a195e6395..a9ed08a7e34be3 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -642,7 +642,6 @@ new_threadstate(PyInterpreterState *interp, int init) tstate->tracing = 0; tstate->root_cframe.use_tracing = 0; tstate->root_cframe.current_frame = NULL; - tstate->root_cframe.eval_breaker = &interp->ceval.eval_breaker; tstate->cframe = &tstate->root_cframe; tstate->gilstate_counter = 0; tstate->async_exc = NULL; From 85cf4690e9e18dbbde2f6e2839252d095b564ee5 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 15 Nov 2021 16:09:59 +0000 Subject: [PATCH 09/16] Remove 'co' and 'retval' from set of interpreter registers. --- Python/ceval.c | 79 ++++++++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 35 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 205c5fd4620569..a2be0f59e62d3a 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1328,7 +1328,7 @@ eval_frame_handle_pending(PyThreadState *tstate) /* Get opcode and oparg from original instructions, not quickened form. */ #define TRACING_NEXTOPARG() do { \ - _Py_CODEUNIT word = ((_Py_CODEUNIT *)PyBytes_AS_STRING(co->co_code))[INSTR_OFFSET()]; \ + _Py_CODEUNIT word = ((_Py_CODEUNIT *)PyBytes_AS_STRING(frame->f_code->co_code))[INSTR_OFFSET()]; \ opcode = _Py_OPCODE(word); \ oparg = _Py_OPARG(word); \ } while (0) @@ -1400,20 +1400,20 @@ eval_frame_handle_pending(PyThreadState *tstate) #ifdef LLTRACE #define PUSH(v) { (void)(BASIC_PUSH(v), \ lltrace && prtrace(tstate, TOP(), "push")); \ - assert(STACK_LEVEL() <= co->co_stacksize); } + assert(STACK_LEVEL() <= frame->f_code->co_stacksize); } #define POP() ((void)(lltrace && prtrace(tstate, TOP(), "pop")), \ BASIC_POP()) #define STACK_GROW(n) do { \ assert(n >= 0); \ (void)(BASIC_STACKADJ(n), \ lltrace && prtrace(tstate, TOP(), "stackadj")); \ - assert(STACK_LEVEL() <= co->co_stacksize); \ + assert(STACK_LEVEL() <= frame->f_code->co_stacksize); \ } while (0) #define STACK_SHRINK(n) do { \ assert(n >= 0); \ (void)(lltrace && prtrace(tstate, TOP(), "stackadj")); \ (void)(BASIC_STACKADJ(-(n))); \ - assert(STACK_LEVEL() <= co->co_stacksize); \ + assert(STACK_LEVEL() <= frame->f_code->co_stacksize); \ } while (0) #define EXT_POP(STACK_POINTER) ((void)(lltrace && \ prtrace(tstate, (STACK_POINTER)[-1], "ext_pop")), \ @@ -1607,7 +1607,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr #endif int opcode; /* Current opcode */ int oparg; /* Current opcode argument, if any */ - PyObject *retval = NULL; /* Return value */ _Py_atomic_int * const eval_breaker = &tstate->interp->ceval.eval_breaker; CFrame cframe; @@ -1637,6 +1636,14 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr goto update_locals_then_error; } + /* Local "register" variables. + * These are cached values from the frame and code object. */ + PyObject *names; + PyObject *consts; + _Py_CODEUNIT *first_instr; + _Py_CODEUNIT *next_instr; + PyObject **stack_pointer; + start_frame: if (_Py_EnterRecursiveCall(tstate, "")) { tstate->recursion_depth++; @@ -1649,16 +1656,17 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr TRACE_FUNCTION_ENTRY(); DTRACE_FUNCTION_ENTRY(); - PyCodeObject *co = frame->f_code; - if (_Py_IncrementCountAndMaybeQuicken(co) < 0) { + if (_Py_IncrementCountAndMaybeQuicken(frame->f_code) < 0) { goto exit_unwind; } resume_frame: - co = frame->f_code; - PyObject *names = co->co_names; - PyObject *consts = co->co_consts; - _Py_CODEUNIT *first_instr = co->co_firstinstr; + { + PyCodeObject *co = frame->f_code; + names = co->co_names; + consts = co->co_consts; + first_instr = co->co_firstinstr; + } /* frame->f_lasti refers to the index of the last instruction, unless it's -1 in which case next_instr should be first_instr. @@ -1673,8 +1681,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr or lookups of frame->f_lasti are aways correct when the macros are used. */ assert(frame->f_lasti >= -1); - _Py_CODEUNIT *next_instr = first_instr + frame->f_lasti + 1; - PyObject **stack_pointer = _PyFrame_GetStackPointer(frame); + next_instr = first_instr + frame->f_lasti + 1; + stack_pointer = _PyFrame_GetStackPointer(frame); /* Set stackdepth to -1. * Update when returning or calling trace function. Having stackdepth <= 0 ensures that invalid @@ -1705,7 +1713,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr check_eval_breaker: { assert(STACK_LEVEL() >= 0); /* else underflow */ - assert(STACK_LEVEL() <= co->co_stacksize); /* else overflow */ + assert(STACK_LEVEL() <= frame->f_code->co_stacksize); /* else overflow */ assert(!_PyErr_Occurred(tstate)); /* Do periodic things. Doing this every time through @@ -2286,7 +2294,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } TARGET(RETURN_VALUE) { - retval = POP(); + PyObject *retval = POP(); assert(EMPTY()); frame->f_state = FRAME_RETURNED; _PyFrame_SetStackPointer(frame, stack_pointer); @@ -2296,7 +2304,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr if (frame->depth) { frame = cframe.current_frame = pop_frame(tstate, frame); _PyFrame_StackPush(frame, retval); - retval = NULL; goto resume_frame; } /* Restore previous cframe and return. */ @@ -2447,6 +2454,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr PyObject *v = POP(); PyObject *receiver = TOP(); PySendResult gen_status; + PyObject *retval; if (tstate->c_tracefunc == NULL) { gen_status = PyIter_Send(receiver, v, &retval); } else { @@ -2504,9 +2512,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr TARGET(YIELD_VALUE) { assert(frame->depth == 0); - retval = POP(); + PyObject *retval = POP(); - if (co->co_flags & CO_ASYNC_GENERATOR) { + if (frame->f_code->co_flags & CO_ASYNC_GENERATOR) { PyObject *w = _PyAsyncGenValueWrapperNew(retval); Py_DECREF(retval); if (w == NULL) { @@ -2992,7 +3000,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr format_exc_check_arg( tstate, PyExc_UnboundLocalError, UNBOUNDLOCAL_ERROR_MSG, - PyTuple_GetItem(co->co_localsplusnames, oparg) + PyTuple_GetItem(frame->f_code->co_localsplusnames, oparg) ); goto error; } @@ -3017,15 +3025,15 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr Py_DECREF(oldobj); DISPATCH(); } - format_exc_unbound(tstate, co, oparg); + format_exc_unbound(tstate, frame->f_code, oparg); goto error; } TARGET(LOAD_CLASSDEREF) { PyObject *name, *value, *locals = LOCALS(); assert(locals); - assert(oparg >= 0 && oparg < co->co_nlocalsplus); - name = PyTuple_GET_ITEM(co->co_localsplusnames, oparg); + assert(oparg >= 0 && oparg < frame->f_code->co_nlocalsplus); + name = PyTuple_GET_ITEM(frame->f_code->co_localsplusnames, oparg); if (PyDict_CheckExact(locals)) { value = PyDict_GetItemWithError(locals, name); if (value != NULL) { @@ -3048,7 +3056,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr PyObject *cell = GETLOCAL(oparg); value = PyCell_GET(cell); if (value == NULL) { - format_exc_unbound(tstate, co, oparg); + format_exc_unbound(tstate, frame->f_code, oparg); goto error; } Py_INCREF(value); @@ -3061,7 +3069,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr PyObject *cell = GETLOCAL(oparg); PyObject *value = PyCell_GET(cell); if (value == NULL) { - format_exc_unbound(tstate, co, oparg); + format_exc_unbound(tstate, frame->f_code, oparg); goto error; } Py_INCREF(value); @@ -3816,13 +3824,13 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr TARGET(JUMP_ABSOLUTE) { PREDICTED(JUMP_ABSOLUTE); assert(oparg < INSTR_OFFSET()); - int err = _Py_IncrementCountAndMaybeQuicken(co); + int err = _Py_IncrementCountAndMaybeQuicken(frame->f_code); if (err) { if (err < 0) { goto error; } int nexti = INSTR_OFFSET(); - first_instr = co->co_firstinstr; + first_instr = frame->f_code->co_firstinstr; next_instr = first_instr + nexti; } JUMPTO(oparg); @@ -3930,7 +3938,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr PyObject *iter; if (PyCoro_CheckExact(iterable)) { /* `iterable` is a coroutine */ - if (!(co->co_flags & (CO_COROUTINE | CO_ITERABLE_COROUTINE))) { + if (!(frame->f_code->co_flags & (CO_COROUTINE | CO_ITERABLE_COROUTINE))) { /* and it is used in a 'yield from' expression of a regular generator. */ Py_DECREF(iterable); @@ -4879,7 +4887,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr #else case DO_TRACING: { #endif - int instr_prev = skip_backwards_over_extended_args(co, frame->f_lasti); + int instr_prev = skip_backwards_over_extended_args(frame->f_code, frame->f_lasti); frame->f_lasti = INSTR_OFFSET(); TRACING_NEXTOPARG(); if (PyDTrace_LINE_ENABLED()) { @@ -4988,7 +4996,7 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) { format_exc_check_arg(tstate, PyExc_UnboundLocalError, UNBOUNDLOCAL_ERROR_MSG, - PyTuple_GetItem(co->co_localsplusnames, oparg) + PyTuple_GetItem(frame->f_code->co_localsplusnames, oparg) ); goto error; } @@ -5023,9 +5031,8 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) /* We can't use frame->f_lasti here, as RERAISE may have set it */ int offset = INSTR_OFFSET()-1; int level, handler, lasti; - if (get_exception_handler(co, offset, &level, &handler, &lasti) == 0) { + if (get_exception_handler(frame->f_code, offset, &level, &handler, &lasti) == 0) { // No handlers, so exit. - assert(retval == NULL); assert(_PyErr_Occurred(tstate)); /* Pop remaining stack entries. */ @@ -5095,10 +5102,12 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) frame = cframe.current_frame = pop_frame(tstate, frame); update_locals_then_error: - co = frame->f_code; - names = co->co_names; - consts = co->co_consts; - first_instr = co->co_firstinstr; + { + PyCodeObject * co = frame->f_code; + names = co->co_names; + consts = co->co_consts; + first_instr = co->co_firstinstr; + } assert(frame->f_lasti >= -1); next_instr = first_instr + frame->f_lasti + 1; stack_pointer = _PyFrame_GetStackPointer(frame); From 76ee6e95d5e1b84fa80e6163980fb1225c54e478 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 15 Nov 2021 16:23:18 +0000 Subject: [PATCH 10/16] Rename label --- Python/ceval.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index a2be0f59e62d3a..ebf8c6f4224991 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1633,7 +1633,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } TRACE_FUNCTION_THROW_ENTRY(); DTRACE_FUNCTION_ENTRY(); - goto update_locals_then_error; + goto resume_with_error; } /* Local "register" variables. @@ -5101,7 +5101,7 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) } frame = cframe.current_frame = pop_frame(tstate, frame); -update_locals_then_error: +resume_with_error: { PyCodeObject * co = frame->f_code; names = co->co_names; From 8ac6a75e9f7c3a3a5cd74b1e2ed2282134f117a0 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 18 Nov 2021 15:29:56 +0000 Subject: [PATCH 11/16] Make _PyFrame_Clear return void. --- Include/internal/pycore_frame.h | 2 +- Python/ceval.c | 9 +++------ Python/frame.c | 6 ++---- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index b025ca0b8d766c..203bb0d6e40866 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -137,7 +137,7 @@ _PyFrame_GetFrameObject(InterpreterFrame *frame) * take should be set to 1 for heap allocated * frames like the ones in generators and coroutines. */ -int +void _PyFrame_Clear(InterpreterFrame * frame, int take); int diff --git a/Python/ceval.c b/Python/ceval.c index acceeaf12db6de..27b2d8e386640e 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -100,7 +100,7 @@ static InterpreterFrame * _PyEvalFramePushAndInit(PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals, PyObject* const* args, size_t argcount, PyObject *kwnames); -static int +static void _PyEvalFrameClearAndPop(PyThreadState *tstate, InterpreterFrame * frame); #define NAME_ERROR_MSG \ @@ -5766,7 +5766,7 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, PyFrameConstructor *con, return NULL; } -static int +static void _PyEvalFrameClearAndPop(PyThreadState *tstate, InterpreterFrame * frame) { --tstate->recursion_remaining; @@ -5774,7 +5774,6 @@ _PyEvalFrameClearAndPop(PyThreadState *tstate, InterpreterFrame * frame) _PyFrame_Clear(frame, 0); ++tstate->recursion_remaining; _PyThreadState_PopFrame(tstate, frame); - return 0; } PyObject * @@ -5808,9 +5807,7 @@ _PyEval_Vector(PyThreadState *tstate, PyFrameConstructor *con, } PyObject *retval = _PyEval_EvalFrame(tstate, frame, 0); assert(_PyFrame_GetStackPointer(frame) == _PyFrame_Stackbase(frame)); - if (_PyEvalFrameClearAndPop(tstate, frame)) { - retval = NULL; - } + _PyEvalFrameClearAndPop(tstate, frame); return retval; } diff --git a/Python/frame.c b/Python/frame.c index dd57ec3d479be8..057f6885409eb3 100644 --- a/Python/frame.c +++ b/Python/frame.c @@ -97,7 +97,7 @@ take_ownership(PyFrameObject *f, InterpreterFrame *frame) } /* FIXME -- See bpo-45786 */ -int +void _PyFrame_Clear(InterpreterFrame * frame, int take) { if (frame->frame_obj) { @@ -107,12 +107,11 @@ _PyFrame_Clear(InterpreterFrame * frame, int take) if (!take) { frame = copy_frame_to_heap(frame); if (frame == NULL) { - return -1; + return; } } take_ownership(f, frame); Py_DECREF(f); - return 0; } Py_DECREF(f); } @@ -124,5 +123,4 @@ _PyFrame_Clear(InterpreterFrame * frame, int take) if (take) { PyMem_Free(frame); } - return 0; } From 2eb7fcbb1b0eae605af43eff883040c6b4df7f95 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 18 Nov 2021 15:31:57 +0000 Subject: [PATCH 12/16] Remove some redundant macros. --- Python/ceval.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 27b2d8e386640e..63796c94a0470c 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1512,9 +1512,6 @@ eval_frame_handle_pending(PyThreadState *tstate) dtrace_function_return(frame); \ } -#define TRACE_FUNCTION_RETURN() TRACE_FUNCTION_EXIT() -#define TRACE_FUNCTION_YIELD() TRACE_FUNCTION_EXIT() - #define TRACE_FUNCTION_UNWIND() \ if (cframe.use_tracing) { \ trace_function_exit(tstate, frame, NULL); \ @@ -1532,7 +1529,6 @@ eval_frame_handle_pending(PyThreadState *tstate) dtrace_function_entry(frame); \ } -#define TRACE_FUNCTION_THROW_ENTRY() TRACE_FUNCTION_ENTRY() static int @@ -1655,7 +1651,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr tstate->recursion_remaining--; goto exit_unwind; } - TRACE_FUNCTION_THROW_ENTRY(); + TRACE_FUNCTION_ENTRY(); DTRACE_FUNCTION_ENTRY(); goto resume_with_error; } @@ -2322,7 +2318,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr assert(EMPTY()); frame->f_state = FRAME_RETURNED; _PyFrame_SetStackPointer(frame, stack_pointer); - TRACE_FUNCTION_RETURN(); + TRACE_FUNCTION_EXIT(); DTRACE_FUNCTION_EXIT(); _Py_LeaveRecursiveCall(tstate); if (frame->depth) { @@ -2524,7 +2520,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr frame->f_lasti -= 1; frame->f_state = FRAME_SUSPENDED; _PyFrame_SetStackPointer(frame, stack_pointer); - TRACE_FUNCTION_YIELD(); + TRACE_FUNCTION_EXIT(); DTRACE_FUNCTION_EXIT(); _Py_LeaveRecursiveCall(tstate); /* Restore previous cframe and return. */ @@ -2549,7 +2545,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } frame->f_state = FRAME_SUSPENDED; _PyFrame_SetStackPointer(frame, stack_pointer); - TRACE_FUNCTION_YIELD(); + TRACE_FUNCTION_EXIT(); DTRACE_FUNCTION_EXIT(); _Py_LeaveRecursiveCall(tstate); /* Restore previous cframe and return. */ From 8c527fc3df1f248105238a7c2430d67572d6b13e Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 18 Nov 2021 15:36:32 +0000 Subject: [PATCH 13/16] Fix refleak --- Python/ceval.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index 63796c94a0470c..70c26052385621 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1502,7 +1502,7 @@ eval_frame_handle_pending(PyThreadState *tstate) #define TRACE_FUNCTION_EXIT() \ if (cframe.use_tracing) { \ if (trace_function_exit(tstate, frame, retval)) { \ - retval = NULL; \ + Py_DECREF(retval); \ goto exit_unwind; \ } \ } From 22cb8a4a05de5ac574e853734e1541e7b78598e3 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 18 Nov 2021 15:50:19 +0000 Subject: [PATCH 14/16] Restore mistakenly deleted return. --- Python/frame.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Python/frame.c b/Python/frame.c index 057f6885409eb3..872180582ca9bb 100644 --- a/Python/frame.c +++ b/Python/frame.c @@ -112,6 +112,7 @@ _PyFrame_Clear(InterpreterFrame * frame, int take) } take_ownership(f, frame); Py_DECREF(f); + return; } Py_DECREF(f); } From ae277da0eaafe62aff073e79bdb318e78ec1e287 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 19 Nov 2021 12:06:24 +0000 Subject: [PATCH 15/16] Add macro for initializing interpreter's local variables from frame. --- Include/internal/pycore_frame.h | 5 +++ Python/ceval.c | 68 ++++++++++++--------------------- 2 files changed, 30 insertions(+), 43 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index 203bb0d6e40866..c91ca981505bde 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -19,6 +19,11 @@ enum _framestate { typedef signed char PyFrameState; +/* + frame->f_lasti refers to the index of the last instruction, + unless it's -1 in which case next_instr should be first_instr. +*/ + typedef struct _interpreter_frame { PyObject *f_globals; PyObject *f_builtins; diff --git a/Python/ceval.c b/Python/ceval.c index 70c26052385621..884722248d3165 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1658,12 +1658,33 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr /* Local "register" variables. * These are cached values from the frame and code object. */ + PyObject *names; PyObject *consts; _Py_CODEUNIT *first_instr; _Py_CODEUNIT *next_instr; PyObject **stack_pointer; +/* Sets the above local variables from the frame */ +#define SET_LOCALS_FROM_FRAME() \ + { \ + PyCodeObject *co = frame->f_code; \ + names = co->co_names; \ + consts = co->co_consts; \ + first_instr = co->co_firstinstr; \ + } \ + assert(frame->f_lasti >= -1); \ + next_instr = first_instr + frame->f_lasti + 1; \ + stack_pointer = _PyFrame_GetStackPointer(frame); \ + /* Set stackdepth to -1. \ + Update when returning or calling trace function. \ + Having stackdepth <= 0 ensures that invalid \ + values are not visible to the cycle GC. \ + We choose -1 rather than 0 to assist debugging. \ + */ \ + frame->stacktop = -1; + + start_frame: if (_Py_EnterRecursiveCall(tstate, "")) { tstate->recursion_remaining--; @@ -1679,38 +1700,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr if (_Py_IncrementCountAndMaybeQuicken(frame->f_code) < 0) { goto exit_unwind; } + frame->f_state = FRAME_EXECUTING; resume_frame: - { - PyCodeObject *co = frame->f_code; - names = co->co_names; - consts = co->co_consts; - first_instr = co->co_firstinstr; - } - /* - frame->f_lasti refers to the index of the last instruction, - unless it's -1 in which case next_instr should be first_instr. - - YIELD_FROM sets frame->f_lasti to itself, in order to repeatedly yield - multiple values. - - When the PREDICT() macros are enabled, some opcode pairs follow in - direct succession. A successful prediction effectively links the two - codes together as if they were a single new opcode, but the value - of frame->f_lasti is correctly updated so potential inlined calls - or lookups of frame->f_lasti are aways correct when the macros are used. - */ - assert(frame->f_lasti >= -1); - next_instr = first_instr + frame->f_lasti + 1; - stack_pointer = _PyFrame_GetStackPointer(frame); - /* Set stackdepth to -1. - * Update when returning or calling trace function. - Having stackdepth <= 0 ensures that invalid - values are not visible to the cycle GC. - We choose -1 rather than 0 to assist debugging. - */ - frame->stacktop = -1; - frame->f_state = FRAME_EXECUTING; + SET_LOCALS_FROM_FRAME(); #ifdef LLTRACE _Py_IDENTIFIER(__ltrace__); @@ -5038,8 +5031,7 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) } if (tstate->c_tracefunc != NULL) { - /* Make sure state is set to FRAME_EXECUTING for tracing */ - assert(frame->f_state == FRAME_EXECUTING); + /* Make sure state is set to FRAME_UNWINDING for tracing */ frame->f_state = FRAME_UNWINDING; call_exc_trace(tstate->c_tracefunc, tstate->c_traceobj, tstate, frame); @@ -5122,17 +5114,7 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) frame = cframe.current_frame = pop_frame(tstate, frame); resume_with_error: - { - PyCodeObject * co = frame->f_code; - names = co->co_names; - consts = co->co_consts; - first_instr = co->co_firstinstr; - } - assert(frame->f_lasti >= -1); - next_instr = first_instr + frame->f_lasti + 1; - stack_pointer = _PyFrame_GetStackPointer(frame); - frame->stacktop = -1; - frame->f_state = FRAME_EXECUTING; + SET_LOCALS_FROM_FRAME(); goto error; } From bec7c00808a3cacd5e7dbe56c49288397647007a Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 1 Dec 2021 10:11:02 +0000 Subject: [PATCH 16/16] Add some asserts and clarifying comments. --- Python/ceval.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index fd54cd1c9c8d23..78cf31d953188f 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -103,7 +103,7 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, PyObject *locals, PyObject* const* args, size_t argcount, PyObject *kwnames); static void -_PyEvalFrameClearAndPop(PyThreadState *tstate, InterpreterFrame * frame); +_PyEvalFrameClearAndPop(PyThreadState *tstate, InterpreterFrame *frame); #define NAME_ERROR_MSG \ "name '%.200s' is not defined" @@ -1552,6 +1552,8 @@ eval_frame_handle_pending(PyThreadState *tstate) #define TRACE_FUNCTION_UNWIND() \ if (cframe.use_tracing) { \ + /* Since we are already unwinding, \ + * we dont't care if this raises */ \ trace_function_exit(tstate, frame, NULL); \ } @@ -2487,6 +2489,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr tstate->cframe = cframe.previous; tstate->cframe->use_tracing = cframe.use_tracing; assert(tstate->cframe->current_frame == frame->previous); + assert(!_PyErr_Occurred(tstate)); return retval; } @@ -2684,6 +2687,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr tstate->cframe = cframe.previous; tstate->cframe->use_tracing = cframe.use_tracing; assert(tstate->cframe->current_frame == frame->previous); + assert(!_PyErr_Occurred(tstate)); return retval; } @@ -2709,6 +2713,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr tstate->cframe = cframe.previous; tstate->cframe->use_tracing = cframe.use_tracing; assert(tstate->cframe->current_frame == frame->previous); + assert(!_PyErr_Occurred(tstate)); return retval; } @@ -4005,6 +4010,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr if (err < 0) { goto error; } + /* Update first_instr and next_instr to point to newly quickened code */ int nexti = INSTR_OFFSET(); first_instr = frame->f_code->co_firstinstr; next_instr = first_instr + nexti; @@ -5825,10 +5831,10 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, static void _PyEvalFrameClearAndPop(PyThreadState *tstate, InterpreterFrame * frame) { - --tstate->recursion_remaining; + tstate->recursion_remaining--; assert(frame->frame_obj == NULL || frame->frame_obj->f_owns_frame == 0); _PyFrame_Clear(frame); - ++tstate->recursion_remaining; + tstate->recursion_remaining++; _PyThreadState_PopFrame(tstate, frame); }