From db12634dd93dfa722c95660b5402532ca263cbca Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 11 Sep 2024 19:04:19 +0000 Subject: [PATCH 1/3] gh-123923: Defer refcounting for `f_funcobj` in `_PyInterpreterFrame` Use a `_PyStackRef` and defer the reference to `f_funcobj` when possible. This avoids some reference count contention in the common case of executing the same code object from multiple threads concurrently in the free-threaded build. --- Include/internal/pycore_frame.h | 20 +++++---- Include/internal/pycore_gc.h | 11 +++++ Modules/_testinternalcapi.c | 6 +-- Objects/frameobject.c | 9 ++-- Objects/genobject.c | 5 +-- Objects/typevarobject.c | 4 +- Python/bytecodes.c | 45 +++++++++---------- Python/ceval.c | 27 +++++------ Python/executor_cases.c.h | 51 ++++++++------------- Python/frame.c | 9 ++-- Python/gc_free_threading.c | 17 ++++--- Python/generated_cases.c.h | 52 ++++++++++------------ Python/sysmodule.c | 5 ++- Tools/cases_generator/analyzer.py | 12 +++++ Tools/cases_generator/generators_common.py | 9 ++-- 15 files changed, 144 insertions(+), 138 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index a77658134fae8c..4738d0ca4cd06b 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -62,7 +62,7 @@ enum _frameowner { typedef struct _PyInterpreterFrame { _PyStackRef f_executable; /* Deferred or strong reference (code object or None) */ struct _PyInterpreterFrame *previous; - PyObject *f_funcobj; /* Strong reference. Only valid if not on C stack */ + _PyStackRef f_funcobj; /* Deferred or strong reference. Only valid if not on C stack */ PyObject *f_globals; /* Borrowed reference. Only valid if not on C stack */ PyObject *f_builtins; /* Borrowed reference. Only valid if not on C stack */ PyObject *f_locals; /* Strong reference, may be NULL. Only valid if not on C stack */ @@ -144,14 +144,15 @@ static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame * */ static inline void _PyFrame_Initialize( - _PyInterpreterFrame *frame, PyFunctionObject *func, + _PyInterpreterFrame *frame, _PyStackRef func, PyObject *locals, PyCodeObject *code, int null_locals_from, _PyInterpreterFrame *previous) { frame->previous = previous; - frame->f_funcobj = (PyObject *)func; + frame->f_funcobj = func; frame->f_executable = PyStackRef_FromPyObjectNew(code); - frame->f_builtins = func->func_builtins; - frame->f_globals = func->func_globals; + PyFunctionObject *func_obj = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(func); + frame->f_builtins = func_obj->func_builtins; + frame->f_globals = func_obj->func_globals; frame->f_locals = locals; frame->stackpointer = frame->localsplus + code->co_nlocalsplus; frame->frame_obj = NULL; @@ -300,10 +301,11 @@ PyAPI_FUNC(void) _PyThreadState_PopFrame(PyThreadState *tstate, _PyInterpreterFr * Must be guarded by _PyThreadState_HasStackSpace() * Consumes reference to func. */ static inline _PyInterpreterFrame * -_PyFrame_PushUnchecked(PyThreadState *tstate, PyFunctionObject *func, int null_locals_from, _PyInterpreterFrame * previous) +_PyFrame_PushUnchecked(PyThreadState *tstate, _PyStackRef func, int null_locals_from, _PyInterpreterFrame * previous) { CALL_STAT_INC(frames_pushed); - PyCodeObject *code = (PyCodeObject *)func->func_code; + PyFunctionObject *func_obj = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(func); + PyCodeObject *code = (PyCodeObject *)func_obj->func_code; _PyInterpreterFrame *new_frame = (_PyInterpreterFrame *)tstate->datastack_top; tstate->datastack_top += code->co_framesize; assert(tstate->datastack_top < tstate->datastack_limit); @@ -321,7 +323,7 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int tstate->datastack_top += code->co_framesize; assert(tstate->datastack_top < tstate->datastack_limit); frame->previous = previous; - frame->f_funcobj = Py_None; + frame->f_funcobj = PyStackRef_None; frame->f_executable = PyStackRef_FromPyObjectNew(code); #ifdef Py_DEBUG frame->f_builtins = NULL; @@ -345,7 +347,7 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int } PyAPI_FUNC(_PyInterpreterFrame *) -_PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, +_PyEvalFramePushAndInit(PyThreadState *tstate, _PyStackRef func, PyObject *locals, _PyStackRef const* args, size_t argcount, PyObject *kwnames, _PyInterpreterFrame *previous); diff --git a/Include/internal/pycore_gc.h b/Include/internal/pycore_gc.h index b674313d97ff82..a7f7581276c055 100644 --- a/Include/internal/pycore_gc.h +++ b/Include/internal/pycore_gc.h @@ -387,6 +387,17 @@ union _PyStackRef; extern int _PyGC_VisitFrameStack(struct _PyInterpreterFrame *frame, visitproc visit, void *arg); extern int _PyGC_VisitStackRef(union _PyStackRef *ref, visitproc visit, void *arg); +// Like Py_VISIT but for _PyStackRef fields +#define _Py_VISIT_STACKREF(ref) \ + do { \ + if (!PyStackRef_IsNull(ref)) { \ + int vret = _PyGC_VisitStackRef(&(ref), visit, arg); \ + if (vret) \ + return vret; \ + } \ + } while (0) + + #ifdef __cplusplus } #endif diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 0451688a46c75f..246bf8f483a1ae 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -681,13 +681,13 @@ set_eval_frame_default(PyObject *self, PyObject *Py_UNUSED(args)) static PyObject * record_eval(PyThreadState *tstate, struct _PyInterpreterFrame *f, int exc) { - if (PyFunction_Check(f->f_funcobj)) { + if (PyStackRef_FunctionCheck(f->f_funcobj)) { + PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(f->f_funcobj); PyObject *module = _get_current_module(); assert(module != NULL); module_state *state = get_module_state(module); Py_DECREF(module); - int res = PyList_Append(state->record_list, - ((PyFunctionObject *)f->f_funcobj)->func_name); + int res = PyList_Append(state->record_list, func->func_name); if (res < 0) { return NULL; } diff --git a/Objects/frameobject.c b/Objects/frameobject.c index b567327f970836..035818becabdae 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -1634,7 +1634,7 @@ frame_dealloc(PyFrameObject *f) /* Kill all local variables including specials, if we own them */ if (f->f_frame == frame && frame->owner == FRAME_OWNED_BY_FRAME_OBJECT) { PyStackRef_CLEAR(frame->f_executable); - Py_CLEAR(frame->f_funcobj); + PyStackRef_CLEAR(frame->f_funcobj); Py_CLEAR(frame->f_locals); _PyStackRef *locals = _PyFrame_GetLocalsArray(frame); _PyStackRef *sp = frame->stackpointer; @@ -1790,7 +1790,7 @@ static void init_frame(_PyInterpreterFrame *frame, PyFunctionObject *func, PyObject *locals) { PyCodeObject *code = (PyCodeObject *)func->func_code; - _PyFrame_Initialize(frame, (PyFunctionObject*)Py_NewRef(func), + _PyFrame_Initialize(frame, PyStackRef_FromPyObjectNew(func), Py_XNewRef(locals), code, 0, NULL); } @@ -1861,14 +1861,15 @@ frame_init_get_vars(_PyInterpreterFrame *frame) PyCodeObject *co = _PyFrame_GetCode(frame); int lasti = _PyInterpreterFrame_LASTI(frame); if (!(lasti < 0 && _PyCode_CODE(co)->op.code == COPY_FREE_VARS - && PyFunction_Check(frame->f_funcobj))) + && PyStackRef_FunctionCheck(frame->f_funcobj))) { /* Free vars are initialized */ return; } /* Free vars have not been initialized -- Do that */ - PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure; + PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj); + PyObject *closure = func->func_closure; int offset = PyUnstable_Code_GetFirstFree(co); for (int i = 0; i < co->co_nfreevars; ++i) { PyObject *o = PyTuple_GET_ITEM(closure, i); diff --git a/Objects/genobject.c b/Objects/genobject.c index 5dc8f926557b52..41cf8fdcc9dee8 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -58,10 +58,7 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg) else { // We still need to visit the code object when the frame is cleared to // ensure that it's kept alive if the reference is deferred. - int err = _PyGC_VisitStackRef(&gen->gi_iframe.f_executable, visit, arg); - if (err) { - return err; - } + _Py_VISIT_STACKREF(gen->gi_iframe.f_executable); } /* No need to visit cr_origin, because it's just tuples/str/int, so can't participate in a reference cycle. */ diff --git a/Objects/typevarobject.c b/Objects/typevarobject.c index 3c96850589d378..d3656155fae330 100644 --- a/Objects/typevarobject.c +++ b/Objects/typevarobject.c @@ -372,10 +372,10 @@ caller(void) if (f == NULL) { Py_RETURN_NONE; } - if (f == NULL || f->f_funcobj == NULL) { + if (f == NULL || PyStackRef_IsNull(f->f_funcobj)) { Py_RETURN_NONE; } - PyObject *r = PyFunction_GetModule(f->f_funcobj); + PyObject *r = PyFunction_GetModule(PyStackRef_AsPyObjectBorrow(f->f_funcobj)); if (!r) { PyErr_Clear(); Py_RETURN_NONE; diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 078f06d697cc3c..953334cf2e1660 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -808,14 +808,13 @@ dummy_func( assert(code->co_argcount == 2); DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize)); STAT_INC(BINARY_SUBSCR, hit); - Py_INCREF(getitem); } op(_BINARY_SUBSCR_INIT_CALL, (container, sub -- new_frame: _PyInterpreterFrame* )) { PyTypeObject *tp = Py_TYPE(PyStackRef_AsPyObjectBorrow(container)); PyHeapTypeObject *ht = (PyHeapTypeObject *)tp; PyObject *getitem = ht->_spec_cache.getitem; - new_frame = _PyFrame_PushUnchecked(tstate, (PyFunctionObject *)getitem, 2, frame); + new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame); SYNC_SP(); new_frame->localsplus[0] = container; new_frame->localsplus[1] = sub; @@ -1666,8 +1665,9 @@ dummy_func( inst(COPY_FREE_VARS, (--)) { /* Copy closure variables to free variables */ PyCodeObject *co = _PyFrame_GetCode(frame); - assert(PyFunction_Check(frame->f_funcobj)); - PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure; + assert(PyStackRef_FunctionCheck(frame->f_funcobj)); + PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj); + PyObject *closure = func->func_closure; assert(oparg == co->co_nfreevars); int offset = co->co_nlocalsplus - oparg; for (int i = 0; i < oparg; ++i) { @@ -2170,8 +2170,7 @@ dummy_func( DEOPT_IF(code->co_argcount != 1); DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize)); STAT_INC(LOAD_ATTR, hit); - Py_INCREF(fget); - new_frame = _PyFrame_PushUnchecked(tstate, f, 1, frame); + new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(fget), 1, frame); new_frame->localsplus[0] = owner; } @@ -2202,8 +2201,8 @@ dummy_func( STAT_INC(LOAD_ATTR, hit); PyObject *name = GETITEM(FRAME_CO_NAMES, oparg >> 1); - Py_INCREF(f); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f, 2, frame); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked( + tstate, PyStackRef_FromPyObjectNew(f), 2, frame); // Manipulate stack directly because we exit with DISPATCH_INLINED(). STACK_SHRINK(1); new_frame->localsplus[0] = owner; @@ -3251,7 +3250,7 @@ dummy_func( int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, total_args, NULL, frame ); // Manipulate stack directly since we leave using DISPATCH_INLINED(). @@ -3340,7 +3339,7 @@ dummy_func( int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, total_args, NULL, frame ); // The frame has stolen all the arguments from the stack, @@ -3475,11 +3474,9 @@ dummy_func( } replicate(5) pure op(_INIT_CALL_PY_EXACT_ARGS, (callable, self_or_null[1], args[oparg] -- new_frame: _PyInterpreterFrame*)) { - PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); int has_self = !PyStackRef_IsNull(self_or_null[0]); STAT_INC(CALL, hit); - PyFunctionObject *func = (PyFunctionObject *)callable_o; - new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self, frame); + new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame); _PyStackRef *first_non_self_local = new_frame->localsplus + has_self; new_frame->localsplus[0] = self_or_null[0]; for (int i = 0; i < oparg; i++) { @@ -3601,10 +3598,9 @@ dummy_func( assert(_PyCode_CODE(_PyFrame_GetCode(shim))[0].op.code == EXIT_INIT_CHECK); /* Push self onto stack of shim */ shim->localsplus[0] = PyStackRef_DUP(self); - PyFunctionObject *init_func = (PyFunctionObject *)PyStackRef_AsPyObjectSteal(init); args[-1] = self; init_frame = _PyEvalFramePushAndInit( - tstate, init_func, NULL, args-1, oparg+1, NULL, shim); + tstate, init, NULL, args-1, oparg+1, NULL, shim); SYNC_SP(); if (init_frame == NULL) { _PyEval_FrameClearAndPop(tstate, shim); @@ -4080,7 +4076,7 @@ dummy_func( int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, positional_args, kwnames_o, frame ); PyStackRef_CLOSE(kwnames); @@ -4148,7 +4144,7 @@ dummy_func( int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, positional_args, kwnames_o, frame ); PyStackRef_CLOSE(kwnames); @@ -4332,9 +4328,9 @@ dummy_func( int code_flags = ((PyCodeObject *)PyFunction_GET_CODE(func))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(func)); - _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit_Ex(tstate, - (PyFunctionObject *)PyStackRef_AsPyObjectSteal(func_st), locals, - nargs, callargs, kwargs, frame); + _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit_Ex( + tstate, func_st, locals, + nargs, callargs, kwargs, frame); // Need to manually shrink the stack since we exit with DISPATCH_INLINED. STACK_SHRINK(oparg + 3); if (new_frame == NULL) { @@ -4408,8 +4404,8 @@ dummy_func( } inst(RETURN_GENERATOR, (-- res)) { - assert(PyFunction_Check(frame->f_funcobj)); - PyFunctionObject *func = (PyFunctionObject *)frame->f_funcobj; + assert(PyStackRef_FunctionCheck(frame->f_funcobj)); + PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj); PyGenObject *gen = (PyGenObject *)_Py_MakeCoro(func); if (gen == NULL) { ERROR_NO_POP(); @@ -4771,8 +4767,9 @@ dummy_func( } tier2 op(_CHECK_FUNCTION, (func_version/2 -- )) { - assert(PyFunction_Check(frame->f_funcobj)); - DEOPT_IF(((PyFunctionObject *)frame->f_funcobj)->func_version != func_version); + assert(PyStackRef_FunctionCheck(frame->f_funcobj)); + PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj); + DEOPT_IF(func->func_version != func_version); } /* Internal -- for testing executors */ diff --git a/Python/ceval.c b/Python/ceval.c index 252833a6243293..f304aeb2c312db 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -193,7 +193,7 @@ lltrace_instruction(_PyInterpreterFrame *frame, static void lltrace_resume_frame(_PyInterpreterFrame *frame) { - PyObject *fobj = frame->f_funcobj; + PyObject *fobj = PyStackRef_AsPyObjectBorrow(frame->f_funcobj); if (!PyStackRef_CodeCheck(frame->f_executable) || fobj == NULL || !PyFunction_Check(fobj) @@ -274,7 +274,7 @@ static void monitor_throw(PyThreadState *tstate, static int check_args_iterable(PyThreadState *, PyObject *func, PyObject *vararg); static int get_exception_handler(PyCodeObject *, int, int*, int*, int*); static _PyInterpreterFrame * -_PyEvalFramePushAndInit_Ex(PyThreadState *tstate, PyFunctionObject *func, +_PyEvalFramePushAndInit_Ex(PyThreadState *tstate, _PyStackRef func, PyObject *locals, Py_ssize_t nargs, PyObject *callargs, PyObject *kwargs, _PyInterpreterFrame *previous); #ifdef HAVE_ERRNO_H @@ -778,7 +778,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int #ifdef Py_DEBUG /* Set these to invalid but identifiable values for debugging. */ - entry_frame.f_funcobj = (PyObject*)0xaaa0; + entry_frame.f_funcobj = (_PyStackRef){.bits = 0xaaa0}; entry_frame.f_locals = (PyObject*)0xaaa1; entry_frame.frame_obj = (PyFrameObject*)0xaaa2; entry_frame.f_globals = (PyObject*)0xaaa3; @@ -1716,18 +1716,19 @@ _PyEval_FrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame * frame) /* Consumes references to func, locals and all the args */ _PyInterpreterFrame * -_PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, +_PyEvalFramePushAndInit(PyThreadState *tstate, _PyStackRef func, PyObject *locals, _PyStackRef const* args, size_t argcount, PyObject *kwnames, _PyInterpreterFrame *previous) { - PyCodeObject * code = (PyCodeObject *)func->func_code; + PyFunctionObject *func_obj = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(func); + PyCodeObject * code = (PyCodeObject *)func_obj->func_code; CALL_STAT_INC(frames_pushed); _PyInterpreterFrame *frame = _PyThreadState_PushFrame(tstate, code->co_framesize); if (frame == NULL) { goto fail; } _PyFrame_Initialize(frame, func, locals, code, 0, previous); - if (initialize_locals(tstate, func, frame->localsplus, args, argcount, kwnames)) { + if (initialize_locals(tstate, func_obj, frame->localsplus, args, argcount, kwnames)) { assert(frame->owner == FRAME_OWNED_BY_THREAD); clear_thread_frame(tstate, frame); return NULL; @@ -1735,7 +1736,7 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, return frame; fail: /* Consume the references */ - Py_DECREF(func); + PyStackRef_CLOSE(func); Py_XDECREF(locals); for (size_t i = 0; i < argcount; i++) { PyStackRef_CLOSE(args[i]); @@ -1751,7 +1752,7 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, } static _PyInterpreterFrame * -_PyEvalFramePushAndInit_UnTagged(PyThreadState *tstate, PyFunctionObject *func, +_PyEvalFramePushAndInit_UnTagged(PyThreadState *tstate, _PyStackRef func, PyObject *locals, PyObject *const* args, size_t argcount, PyObject *kwnames, _PyInterpreterFrame *previous) { @@ -1781,7 +1782,7 @@ _PyEvalFramePushAndInit_UnTagged(PyThreadState *tstate, PyFunctionObject *func, Steals references to func, callargs and kwargs. */ static _PyInterpreterFrame * -_PyEvalFramePushAndInit_Ex(PyThreadState *tstate, PyFunctionObject *func, +_PyEvalFramePushAndInit_Ex(PyThreadState *tstate, _PyStackRef func, PyObject *locals, Py_ssize_t nargs, PyObject *callargs, PyObject *kwargs, _PyInterpreterFrame *previous) { bool has_dict = (kwargs != NULL && PyDict_GET_SIZE(kwargs) > 0); @@ -1790,7 +1791,7 @@ _PyEvalFramePushAndInit_Ex(PyThreadState *tstate, PyFunctionObject *func, if (has_dict) { newargs = _PyStack_UnpackDict(tstate, _PyTuple_ITEMS(callargs), nargs, kwargs, &kwnames); if (newargs == NULL) { - Py_DECREF(func); + PyStackRef_CLOSE(func); goto error; } } @@ -1802,7 +1803,7 @@ _PyEvalFramePushAndInit_Ex(PyThreadState *tstate, PyFunctionObject *func, } } _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit_UnTagged( - tstate, (PyFunctionObject *)func, locals, + tstate, func, locals, newargs, nargs, kwnames, previous ); if (has_dict) { @@ -1828,7 +1829,6 @@ _PyEval_Vector(PyThreadState *tstate, PyFunctionObject *func, { /* _PyEvalFramePushAndInit consumes the references * to func, locals and all its arguments */ - Py_INCREF(func); Py_XINCREF(locals); for (size_t i = 0; i < argcount; i++) { Py_INCREF(args[i]); @@ -1840,7 +1840,8 @@ _PyEval_Vector(PyThreadState *tstate, PyFunctionObject *func, } } _PyInterpreterFrame *frame = _PyEvalFramePushAndInit_UnTagged( - tstate, func, locals, args, argcount, kwnames, NULL); + tstate, PyStackRef_FromPyObjectNew(func), locals, + args, argcount, kwnames, NULL); if (frame == NULL) { return NULL; } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index b36fff9961febe..30ca3ea1cc980b 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1018,7 +1018,6 @@ JUMP_TO_JUMP_TARGET(); } STAT_INC(BINARY_SUBSCR, hit); - Py_INCREF(getitem); break; } @@ -1031,7 +1030,7 @@ PyTypeObject *tp = Py_TYPE(PyStackRef_AsPyObjectBorrow(container)); PyHeapTypeObject *ht = (PyHeapTypeObject *)tp; PyObject *getitem = ht->_spec_cache.getitem; - new_frame = _PyFrame_PushUnchecked(tstate, (PyFunctionObject *)getitem, 2, frame); + new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame); stack_pointer += -2; assert(WITHIN_STACK_BOUNDS()); new_frame->localsplus[0] = container; @@ -1853,8 +1852,9 @@ oparg = CURRENT_OPARG(); /* Copy closure variables to free variables */ PyCodeObject *co = _PyFrame_GetCode(frame); - assert(PyFunction_Check(frame->f_funcobj)); - PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure; + assert(PyStackRef_FunctionCheck(frame->f_funcobj)); + PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj); + PyObject *closure = func->func_closure; assert(oparg == co->co_nfreevars); int offset = co->co_nlocalsplus - oparg; for (int i = 0; i < oparg; ++i) { @@ -2554,8 +2554,7 @@ JUMP_TO_JUMP_TARGET(); } STAT_INC(LOAD_ATTR, hit); - Py_INCREF(fget); - new_frame = _PyFrame_PushUnchecked(tstate, f, 1, frame); + new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(fget), 1, frame); new_frame->localsplus[0] = owner; stack_pointer[-1].bits = (uintptr_t)new_frame; break; @@ -3604,7 +3603,7 @@ int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, total_args, NULL, frame ); // The frame has stolen all the arguments from the stack, @@ -3834,11 +3833,9 @@ args = &stack_pointer[-oparg]; self_or_null = &stack_pointer[-1 - oparg]; callable = stack_pointer[-2 - oparg]; - PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); int has_self = !PyStackRef_IsNull(self_or_null[0]); STAT_INC(CALL, hit); - PyFunctionObject *func = (PyFunctionObject *)callable_o; - new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self, frame); + new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame); _PyStackRef *first_non_self_local = new_frame->localsplus + has_self; new_frame->localsplus[0] = self_or_null[0]; for (int i = 0; i < oparg; i++) { @@ -3860,11 +3857,9 @@ args = &stack_pointer[-oparg]; self_or_null = &stack_pointer[-1 - oparg]; callable = stack_pointer[-2 - oparg]; - PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); int has_self = !PyStackRef_IsNull(self_or_null[0]); STAT_INC(CALL, hit); - PyFunctionObject *func = (PyFunctionObject *)callable_o; - new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self, frame); + new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame); _PyStackRef *first_non_self_local = new_frame->localsplus + has_self; new_frame->localsplus[0] = self_or_null[0]; for (int i = 0; i < oparg; i++) { @@ -3886,11 +3881,9 @@ args = &stack_pointer[-oparg]; self_or_null = &stack_pointer[-1 - oparg]; callable = stack_pointer[-2 - oparg]; - PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); int has_self = !PyStackRef_IsNull(self_or_null[0]); STAT_INC(CALL, hit); - PyFunctionObject *func = (PyFunctionObject *)callable_o; - new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self, frame); + new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame); _PyStackRef *first_non_self_local = new_frame->localsplus + has_self; new_frame->localsplus[0] = self_or_null[0]; for (int i = 0; i < oparg; i++) { @@ -3912,11 +3905,9 @@ args = &stack_pointer[-oparg]; self_or_null = &stack_pointer[-1 - oparg]; callable = stack_pointer[-2 - oparg]; - PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); int has_self = !PyStackRef_IsNull(self_or_null[0]); STAT_INC(CALL, hit); - PyFunctionObject *func = (PyFunctionObject *)callable_o; - new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self, frame); + new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame); _PyStackRef *first_non_self_local = new_frame->localsplus + has_self; new_frame->localsplus[0] = self_or_null[0]; for (int i = 0; i < oparg; i++) { @@ -3938,11 +3929,9 @@ args = &stack_pointer[-oparg]; self_or_null = &stack_pointer[-1 - oparg]; callable = stack_pointer[-2 - oparg]; - PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); int has_self = !PyStackRef_IsNull(self_or_null[0]); STAT_INC(CALL, hit); - PyFunctionObject *func = (PyFunctionObject *)callable_o; - new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self, frame); + new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame); _PyStackRef *first_non_self_local = new_frame->localsplus + has_self; new_frame->localsplus[0] = self_or_null[0]; for (int i = 0; i < oparg; i++) { @@ -3963,11 +3952,9 @@ args = &stack_pointer[-oparg]; self_or_null = &stack_pointer[-1 - oparg]; callable = stack_pointer[-2 - oparg]; - PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); int has_self = !PyStackRef_IsNull(self_or_null[0]); STAT_INC(CALL, hit); - PyFunctionObject *func = (PyFunctionObject *)callable_o; - new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self, frame); + new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame); _PyStackRef *first_non_self_local = new_frame->localsplus + has_self; new_frame->localsplus[0] = self_or_null[0]; for (int i = 0; i < oparg; i++) { @@ -4146,10 +4133,9 @@ assert(_PyCode_CODE(_PyFrame_GetCode(shim))[0].op.code == EXIT_INIT_CHECK); /* Push self onto stack of shim */ shim->localsplus[0] = PyStackRef_DUP(self); - PyFunctionObject *init_func = (PyFunctionObject *)PyStackRef_AsPyObjectSteal(init); args[-1] = self; init_frame = _PyEvalFramePushAndInit( - tstate, init_func, NULL, args-1, oparg+1, NULL, shim); + tstate, init, NULL, args-1, oparg+1, NULL, shim); stack_pointer += -2 - oparg; assert(WITHIN_STACK_BOUNDS()); if (init_frame == NULL) { @@ -4781,7 +4767,7 @@ int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, positional_args, kwnames_o, frame ); PyStackRef_CLOSE(kwnames); @@ -5002,8 +4988,8 @@ case _RETURN_GENERATOR: { _PyStackRef res; - assert(PyFunction_Check(frame->f_funcobj)); - PyFunctionObject *func = (PyFunctionObject *)frame->f_funcobj; + assert(PyStackRef_FunctionCheck(frame->f_funcobj)); + PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj); PyGenObject *gen = (PyGenObject *)_Py_MakeCoro(func); if (gen == NULL) { JUMP_TO_ERROR(); @@ -5377,8 +5363,9 @@ case _CHECK_FUNCTION: { uint32_t func_version = (uint32_t)CURRENT_OPERAND(); - assert(PyFunction_Check(frame->f_funcobj)); - if (((PyFunctionObject *)frame->f_funcobj)->func_version != func_version) { + assert(PyStackRef_FunctionCheck(frame->f_funcobj)); + PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj); + if (func->func_version != func_version) { UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); } diff --git a/Python/frame.c b/Python/frame.c index d7bb29811bfa50..35e6c2d0a93333 100644 --- a/Python/frame.c +++ b/Python/frame.c @@ -13,11 +13,8 @@ _PyFrame_Traverse(_PyInterpreterFrame *frame, visitproc visit, void *arg) { Py_VISIT(frame->frame_obj); Py_VISIT(frame->f_locals); - Py_VISIT(frame->f_funcobj); - int err = _PyGC_VisitStackRef(&frame->f_executable, visit, arg); - if (err) { - return err; - } + _Py_VISIT_STACKREF(frame->f_funcobj); + _Py_VISIT_STACKREF(frame->f_executable); return _PyGC_VisitFrameStack(frame, visit, arg); } @@ -126,7 +123,7 @@ _PyFrame_ClearExceptCode(_PyInterpreterFrame *frame) Py_DECREF(f); } _PyFrame_ClearLocals(frame); - Py_DECREF(frame->f_funcobj); + PyStackRef_CLEAR(frame->f_funcobj); } /* Unstable API functions */ diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index e981f87401a066..4fe37693683a4e 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -181,12 +181,15 @@ merge_refcount(PyObject *op, Py_ssize_t extra) } static void -frame_disable_deferred_refcounting(_PyInterpreterFrame *frame) +frame_disable_deferred_refcounting(_PyInterpreterFrame *frame, int is_funcobj_valid) { // Convert locals, variables, and the executable object to strong // references from (possibly) deferred references. assert(frame->stackpointer != NULL); frame->f_executable = PyStackRef_AsStrongReference(frame->f_executable); + if (is_funcobj_valid) { + frame->f_funcobj = PyStackRef_AsStrongReference(frame->f_funcobj); + } for (_PyStackRef *ref = frame->localsplus; ref < frame->stackpointer; ref++) { if (!PyStackRef_IsNull(*ref) && PyStackRef_IsDeferred(*ref)) { *ref = PyStackRef_AsStrongReference(*ref); @@ -218,10 +221,14 @@ disable_deferred_refcounting(PyObject *op) // use strong references, in case the generator or frame object is // resurrected by a finalizer. if (PyGen_CheckExact(op) || PyCoro_CheckExact(op) || PyAsyncGen_CheckExact(op)) { - frame_disable_deferred_refcounting(&((PyGenObject *)op)->gi_iframe); + // The `f_funcobj` field is invalid if the frame is a cleared generator + PyGenObject *gen = (PyGenObject *)op; + int is_funcobj_valid = gen->gi_frame_state != FRAME_CLEARED; + + frame_disable_deferred_refcounting(&gen->gi_iframe, is_funcobj_valid); } else if (PyFrame_Check(op)) { - frame_disable_deferred_refcounting(((PyFrameObject *)op)->f_frame); + frame_disable_deferred_refcounting(((PyFrameObject *)op)->f_frame, 1); } } @@ -981,9 +988,7 @@ _PyGC_VisitFrameStack(_PyInterpreterFrame *frame, visitproc visit, void *arg) _PyStackRef *ref = _PyFrame_GetLocalsArray(frame); /* locals and stack */ for (; ref < frame->stackpointer; ref++) { - if (_PyGC_VisitStackRef(ref, visit, arg) < 0) { - return -1; - } + _Py_VISIT_STACKREF(*ref); } return 0; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 3e9f0396e495b7..3c394b29a4f1e1 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -506,7 +506,6 @@ assert(code->co_argcount == 2); DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), BINARY_SUBSCR); STAT_INC(BINARY_SUBSCR, hit); - Py_INCREF(getitem); } // _BINARY_SUBSCR_INIT_CALL sub = stack_pointer[-1]; @@ -514,7 +513,7 @@ PyTypeObject *tp = Py_TYPE(PyStackRef_AsPyObjectBorrow(container)); PyHeapTypeObject *ht = (PyHeapTypeObject *)tp; PyObject *getitem = ht->_spec_cache.getitem; - new_frame = _PyFrame_PushUnchecked(tstate, (PyFunctionObject *)getitem, 2, frame); + new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame); stack_pointer += -2; assert(WITHIN_STACK_BOUNDS()); new_frame->localsplus[0] = container; @@ -892,7 +891,7 @@ int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, total_args, NULL, frame ); // Manipulate stack directly since we leave using DISPATCH_INLINED(). @@ -1021,10 +1020,9 @@ assert(_PyCode_CODE(_PyFrame_GetCode(shim))[0].op.code == EXIT_INIT_CHECK); /* Push self onto stack of shim */ shim->localsplus[0] = PyStackRef_DUP(self); - PyFunctionObject *init_func = (PyFunctionObject *)PyStackRef_AsPyObjectSteal(init); args[-1] = self; init_frame = _PyEvalFramePushAndInit( - tstate, init_func, NULL, args-1, oparg+1, NULL, shim); + tstate, init, NULL, args-1, oparg+1, NULL, shim); stack_pointer += -2 - oparg; assert(WITHIN_STACK_BOUNDS()); if (init_frame == NULL) { @@ -1119,11 +1117,9 @@ // _INIT_CALL_PY_EXACT_ARGS args = &stack_pointer[-oparg]; { - PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); int has_self = !PyStackRef_IsNull(self_or_null[0]); STAT_INC(CALL, hit); - PyFunctionObject *func = (PyFunctionObject *)callable_o; - new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self, frame); + new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame); _PyStackRef *first_non_self_local = new_frame->localsplus + has_self; new_frame->localsplus[0] = self_or_null[0]; for (int i = 0; i < oparg; i++) { @@ -1216,7 +1212,7 @@ int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, total_args, NULL, frame ); // The frame has stolen all the arguments from the stack, @@ -1616,8 +1612,8 @@ Py_ssize_t nargs = PyTuple_GET_SIZE(callargs); int code_flags = ((PyCodeObject *)PyFunction_GET_CODE(func))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(func)); - _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit_Ex(tstate, - (PyFunctionObject *)PyStackRef_AsPyObjectSteal(func_st), locals, + _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit_Ex( + tstate, func_st, locals, nargs, callargs, kwargs, frame); // Need to manually shrink the stack since we exit with DISPATCH_INLINED. STACK_SHRINK(oparg + 3); @@ -1802,7 +1798,7 @@ int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, positional_args, kwnames_o, frame ); PyStackRef_CLOSE(kwnames); @@ -1936,7 +1932,7 @@ int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, positional_args, kwnames_o, frame ); PyStackRef_CLOSE(kwnames); @@ -2104,7 +2100,7 @@ int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, positional_args, kwnames_o, frame ); PyStackRef_CLOSE(kwnames); @@ -2649,11 +2645,9 @@ // _INIT_CALL_PY_EXACT_ARGS args = &stack_pointer[-oparg]; { - PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); int has_self = !PyStackRef_IsNull(self_or_null[0]); STAT_INC(CALL, hit); - PyFunctionObject *func = (PyFunctionObject *)callable_o; - new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self, frame); + new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame); _PyStackRef *first_non_self_local = new_frame->localsplus + has_self; new_frame->localsplus[0] = self_or_null[0]; for (int i = 0; i < oparg; i++) { @@ -2726,7 +2720,7 @@ int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, total_args, NULL, frame ); // The frame has stolen all the arguments from the stack, @@ -3274,8 +3268,9 @@ INSTRUCTION_STATS(COPY_FREE_VARS); /* Copy closure variables to free variables */ PyCodeObject *co = _PyFrame_GetCode(frame); - assert(PyFunction_Check(frame->f_funcobj)); - PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure; + assert(PyStackRef_FunctionCheck(frame->f_funcobj)); + PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj); + PyObject *closure = func->func_closure; assert(oparg == co->co_nfreevars); int offset = co->co_nlocalsplus - oparg; for (int i = 0; i < oparg; ++i) { @@ -4102,7 +4097,7 @@ int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o)); _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit( - tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals, + tstate, callable, locals, args, total_args, NULL, frame ); // Manipulate stack directly since we leave using DISPATCH_INLINED(). @@ -4997,8 +4992,8 @@ DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), LOAD_ATTR); STAT_INC(LOAD_ATTR, hit); PyObject *name = GETITEM(FRAME_CO_NAMES, oparg >> 1); - Py_INCREF(f); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f, 2, frame); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked( + tstate, PyStackRef_FromPyObjectNew(f), 2, frame); // Manipulate stack directly because we exit with DISPATCH_INLINED(). STACK_SHRINK(1); new_frame->localsplus[0] = owner; @@ -5328,8 +5323,7 @@ DEOPT_IF(code->co_argcount != 1, LOAD_ATTR); DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), LOAD_ATTR); STAT_INC(LOAD_ATTR, hit); - Py_INCREF(fget); - new_frame = _PyFrame_PushUnchecked(tstate, f, 1, frame); + new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(fget), 1, frame); new_frame->localsplus[0] = owner; } // _SAVE_RETURN_OFFSET @@ -6505,8 +6499,8 @@ next_instr += 1; INSTRUCTION_STATS(RETURN_GENERATOR); _PyStackRef res; - assert(PyFunction_Check(frame->f_funcobj)); - PyFunctionObject *func = (PyFunctionObject *)frame->f_funcobj; + assert(PyStackRef_FunctionCheck(frame->f_funcobj)); + PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj); PyGenObject *gen = (PyGenObject *)_Py_MakeCoro(func); if (gen == NULL) { goto error; @@ -7719,8 +7713,8 @@ Py_ssize_t nargs = PyTuple_GET_SIZE(callargs); int code_flags = ((PyCodeObject *)PyFunction_GET_CODE(func))->co_flags; PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(func)); - _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit_Ex(tstate, - (PyFunctionObject *)PyStackRef_AsPyObjectSteal(func_st), locals, + _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit_Ex( + tstate, func_st, locals, nargs, callargs, kwargs, frame); // Need to manually shrink the stack since we exit with DISPATCH_INLINED. STACK_SHRINK(oparg + 3); diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 887a916563a2e1..ac343a8048e008 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2384,10 +2384,11 @@ sys__getframemodulename_impl(PyObject *module, int depth) while (f && (_PyFrame_IsIncomplete(f) || depth-- > 0)) { f = f->previous; } - if (f == NULL || f->f_funcobj == NULL) { + if (f == NULL || PyStackRef_IsNull(f->f_funcobj)) { Py_RETURN_NONE; } - PyObject *r = PyFunction_GetModule(f->f_funcobj); + PyObject *func = PyStackRef_AsPyObjectBorrow(f->f_funcobj); + PyObject *r = PyFunction_GetModule(func); if (!r) { PyErr_Clear(); r = Py_None; diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 3cc36b6b5841bd..0680c21a3c24c5 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -365,12 +365,24 @@ def find_assignment_target(idx: int) -> list[lexer.Token]: offset += 1 return [] + def in_frame_push(idx: int) -> bool: + for tkn in reversed(node.block.tokens[: idx - 1]): + if tkn.kind == "SEMI" or tkn.kind == "LBRACE" or tkn.kind == "RBRACE": + return False + if tkn.kind == "IDENTIFIER" and tkn.text == "_PyFrame_PushUnchecked": + return True + return False + refs: dict[lexer.Token, str | None] = {} for idx, tkn in enumerate(node.block.tokens): if tkn.kind != "IDENTIFIER" or tkn.text != "PyStackRef_FromPyObjectNew": continue if idx == 0 or node.block.tokens[idx - 1].kind != "EQUALS": + if in_frame_push(idx): + # PyStackRef_FromPyObjectNew() is called in _PyFrame_PushUnchecked() + refs[tkn] = None + continue raise analysis_error("Expected '=' before PyStackRef_FromPyObjectNew", tkn) lhs = find_assignment_target(idx) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 2f8fccec2ea409..4cfd4ad3d05988 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -200,15 +200,16 @@ def py_stack_ref_from_py_object_new( stack: Stack, inst: Instruction | None, ) -> None: - self.out.emit(tkn) - emit_to(self.out, tkn_iter, "SEMI") - self.out.emit(";\n") - target = uop.deferred_refs[tkn] if target is None: # An assignment we don't handle, such as to a pointer or array. + self.out.emit(tkn) return + self.out.emit(tkn) + emit_to(self.out, tkn_iter, "SEMI") + self.out.emit(";\n") + # Flush the assignment to the stack. Note that we don't flush the # stack pointer here, and instead are currently relying on initializing # unused portions of the stack to NULL. From 963896ae5acc5bc189f9c74aa8e26b5df4d82db2 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 12 Sep 2024 22:25:50 +0000 Subject: [PATCH 2/3] Fix tier2 optimizer --- Include/internal/pycore_frame.h | 6 ++++++ Modules/_testinternalcapi.c | 2 +- Objects/frameobject.c | 2 +- Python/optimizer.c | 2 +- Python/optimizer_analysis.c | 2 +- 5 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index 4738d0ca4cd06b..c9ac3819d0390b 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -84,6 +84,12 @@ static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f) { return (PyCodeObject *)executable; } +static inline PyFunctionObject *_PyFrame_GetFunction(_PyInterpreterFrame *f) { + PyObject *func = PyStackRef_AsPyObjectBorrow(f->f_funcobj); + assert(PyFunction_Check(func)); + return (PyFunctionObject *)func; +} + static inline _PyStackRef *_PyFrame_Stackbase(_PyInterpreterFrame *f) { return (f->localsplus + _PyFrame_GetCode(f)->co_nlocalsplus); } diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 246bf8f483a1ae..c403075fbb2501 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -682,7 +682,7 @@ static PyObject * record_eval(PyThreadState *tstate, struct _PyInterpreterFrame *f, int exc) { if (PyStackRef_FunctionCheck(f->f_funcobj)) { - PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(f->f_funcobj); + PyFunctionObject *func = _PyFrame_GetFunction(f); PyObject *module = _get_current_module(); assert(module != NULL); module_state *state = get_module_state(module); diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 035818becabdae..9f1c031dcb9a9d 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -1868,7 +1868,7 @@ frame_init_get_vars(_PyInterpreterFrame *frame) } /* Free vars have not been initialized -- Do that */ - PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj); + PyFunctionObject *func = _PyFrame_GetFunction(frame); PyObject *closure = func->func_closure; int offset = PyUnstable_Code_GetFirstFree(co); for (int i = 0; i < co->co_nfreevars; ++i) { diff --git a/Python/optimizer.c b/Python/optimizer.c index 9198e410627dd4..bb7a90b3204f40 100644 --- a/Python/optimizer.c +++ b/Python/optimizer.c @@ -533,7 +533,7 @@ translate_bytecode_to_trace( { bool first = true; PyCodeObject *code = _PyFrame_GetCode(frame); - PyFunctionObject *func = (PyFunctionObject *)frame->f_funcobj; + PyFunctionObject *func = _PyFrame_GetFunction(frame); assert(PyFunction_Check(func)); PyCodeObject *initial_code = code; _Py_BloomFilter_Add(dependencies, initial_code); diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index f7adb44c9e09ef..b202b58a8b7214 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -145,7 +145,7 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer, return 1; } PyObject *globals = frame->f_globals; - PyFunctionObject *function = (PyFunctionObject *)frame->f_funcobj; + PyFunctionObject *function = _PyFrame_GetFunction(frame); assert(PyFunction_Check(function)); assert(function->func_builtins == builtins); assert(function->func_globals == globals); From cd3d83d63c3d6ef6edd1938dcb8f9897cf2cedac Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Sat, 14 Sep 2024 18:34:51 +0000 Subject: [PATCH 3/3] Simplify code after merge --- Python/gc_free_threading.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index 47d95d85cda888..a5bc9b9b5782b2 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -181,7 +181,7 @@ merge_refcount(PyObject *op, Py_ssize_t extra) } static void -frame_disable_deferred_refcounting(_PyInterpreterFrame *frame, int is_funcobj_valid) +frame_disable_deferred_refcounting(_PyInterpreterFrame *frame) { // Convert locals, variables, and the executable object to strong // references from (possibly) deferred references. @@ -232,14 +232,10 @@ disable_deferred_refcounting(PyObject *op) // use strong references, in case the generator or frame object is // resurrected by a finalizer. if (PyGen_CheckExact(op) || PyCoro_CheckExact(op) || PyAsyncGen_CheckExact(op)) { - // The `f_funcobj` field is invalid if the frame is a cleared generator - PyGenObject *gen = (PyGenObject *)op; - int is_funcobj_valid = gen->gi_frame_state != FRAME_CLEARED; - - frame_disable_deferred_refcounting(&gen->gi_iframe, is_funcobj_valid); + frame_disable_deferred_refcounting(&((PyGenObject *)op)->gi_iframe); } else if (PyFrame_Check(op)) { - frame_disable_deferred_refcounting(((PyFrameObject *)op)->f_frame, 1); + frame_disable_deferred_refcounting(((PyFrameObject *)op)->f_frame); } }