From 2d370da5702f07d044f0e612df68172852b89f42 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 13 Mar 2023 18:35:37 +0000 Subject: [PATCH] GH-100987: Don't cache references to the names and consts array in `_PyEval_EvalFrameDefault`. (#102640) * Rename local variables, names and consts, from the interpeter loop. Will allow non-code objects in frames for better introspection of C builtins and extensions. * Remove unused dummy variables. --- Python/bytecodes.c | 44 +++++++++++++++++------------------- Python/ceval.c | 7 ------ Python/generated_cases.c.h | 46 +++++++++++++++++++------------------- 3 files changed, 44 insertions(+), 53 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 178519fcb63ced..a715bfd3289ec9 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -70,8 +70,6 @@ dummy_func( unsigned int oparg, _Py_atomic_int * const eval_breaker, _PyCFrame cframe, - PyObject *names, - PyObject *consts, _Py_CODEUNIT *next_instr, PyObject **stack_pointer, PyObject *kwnames, @@ -115,7 +113,7 @@ dummy_func( } inst(LOAD_CONST, (-- value)) { - value = GETITEM(consts, oparg); + value = GETITEM(frame->f_code->co_consts, oparg); Py_INCREF(value); } @@ -554,7 +552,7 @@ dummy_func( } inst(RETURN_CONST, (--)) { - PyObject *retval = GETITEM(consts, oparg); + PyObject *retval = GETITEM(frame->f_code->co_consts, oparg); Py_INCREF(retval); assert(EMPTY()); _PyFrame_SetStackPointer(frame, stack_pointer); @@ -844,7 +842,7 @@ dummy_func( } inst(STORE_NAME, (v -- )) { - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); PyObject *ns = LOCALS(); int err; if (ns == NULL) { @@ -862,7 +860,7 @@ dummy_func( } inst(DELETE_NAME, (--)) { - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); PyObject *ns = LOCALS(); int err; if (ns == NULL) { @@ -956,7 +954,7 @@ dummy_func( #if ENABLE_SPECIALIZATION if (ADAPTIVE_COUNTER_IS_ZERO(counter)) { assert(cframe.use_tracing == 0); - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); next_instr--; _Py_Specialize_StoreAttr(owner, next_instr, name); DISPATCH_SAME_OPARG(); @@ -967,7 +965,7 @@ dummy_func( #else (void)counter; // Unused. #endif /* ENABLE_SPECIALIZATION */ - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); int err = PyObject_SetAttr(owner, name, v); Py_DECREF(v); Py_DECREF(owner); @@ -975,21 +973,21 @@ dummy_func( } inst(DELETE_ATTR, (owner --)) { - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); int err = PyObject_SetAttr(owner, name, (PyObject *)NULL); Py_DECREF(owner); ERROR_IF(err, error); } inst(STORE_GLOBAL, (v --)) { - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); int err = PyDict_SetItem(GLOBALS(), name, v); Py_DECREF(v); ERROR_IF(err, error); } inst(DELETE_GLOBAL, (--)) { - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); int err; err = PyDict_DelItem(GLOBALS(), name); // Can't use ERROR_IF here. @@ -1003,7 +1001,7 @@ dummy_func( } inst(LOAD_NAME, ( -- v)) { - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); PyObject *locals = LOCALS(); if (locals == NULL) { _PyErr_Format(tstate, PyExc_SystemError, @@ -1074,7 +1072,7 @@ dummy_func( _PyLoadGlobalCache *cache = (_PyLoadGlobalCache *)next_instr; if (ADAPTIVE_COUNTER_IS_ZERO(cache->counter)) { assert(cframe.use_tracing == 0); - PyObject *name = GETITEM(names, oparg>>1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1); next_instr--; _Py_Specialize_LoadGlobal(GLOBALS(), BUILTINS(), next_instr, name); DISPATCH_SAME_OPARG(); @@ -1082,7 +1080,7 @@ dummy_func( STAT_INC(LOAD_GLOBAL, deferred); DECREMENT_ADAPTIVE_COUNTER(cache->counter); #endif /* ENABLE_SPECIALIZATION */ - PyObject *name = GETITEM(names, oparg>>1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1); if (PyDict_CheckExact(GLOBALS()) && PyDict_CheckExact(BUILTINS())) { @@ -1436,7 +1434,7 @@ dummy_func( _PyAttrCache *cache = (_PyAttrCache *)next_instr; if (ADAPTIVE_COUNTER_IS_ZERO(cache->counter)) { assert(cframe.use_tracing == 0); - PyObject *name = GETITEM(names, oparg>>1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1); next_instr--; _Py_Specialize_LoadAttr(owner, next_instr, name); DISPATCH_SAME_OPARG(); @@ -1444,7 +1442,7 @@ dummy_func( STAT_INC(LOAD_ATTR, deferred); DECREMENT_ADAPTIVE_COUNTER(cache->counter); #endif /* ENABLE_SPECIALIZATION */ - PyObject *name = GETITEM(names, oparg >> 1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg >> 1); if (oparg & 1) { /* Designed to work in tandem with CALL, pushes two values. */ PyObject* meth = NULL; @@ -1525,7 +1523,7 @@ dummy_func( PyDictObject *dict = (PyDictObject *)_PyDictOrValues_GetDict(dorv); DEOPT_IF(dict == NULL, LOAD_ATTR); assert(PyDict_CheckExact((PyObject *)dict)); - PyObject *name = GETITEM(names, oparg>>1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1); uint16_t hint = index; DEOPT_IF(hint >= (size_t)dict->ma_keys->dk_nentries, LOAD_ATTR); if (DK_IS_UNICODE(dict->ma_keys)) { @@ -1616,7 +1614,7 @@ dummy_func( DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), LOAD_ATTR); STAT_INC(LOAD_ATTR, hit); - PyObject *name = GETITEM(names, oparg >> 1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg >> 1); Py_INCREF(f); _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f, 2); // Manipulate stack directly because we exit with DISPATCH_INLINED(). @@ -1661,7 +1659,7 @@ dummy_func( PyDictObject *dict = (PyDictObject *)_PyDictOrValues_GetDict(dorv); DEOPT_IF(dict == NULL, STORE_ATTR); assert(PyDict_CheckExact((PyObject *)dict)); - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); DEOPT_IF(hint >= (size_t)dict->ma_keys->dk_nentries, STORE_ATTR); PyObject *old_value; uint64_t new_version; @@ -1855,14 +1853,14 @@ dummy_func( } inst(IMPORT_NAME, (level, fromlist -- res)) { - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); res = import_name(tstate, frame, name, fromlist, level); DECREF_INPUTS(); ERROR_IF(res == NULL, error); } inst(IMPORT_FROM, (from -- from, res)) { - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); res = import_from(tstate, from, name); ERROR_IF(res == NULL, error); } @@ -2360,8 +2358,8 @@ dummy_func( inst(KW_NAMES, (--)) { assert(kwnames == NULL); - assert(oparg < PyTuple_GET_SIZE(consts)); - kwnames = GETITEM(consts, oparg); + assert(oparg < PyTuple_GET_SIZE(frame->f_code->co_consts)); + kwnames = GETITEM(frame->f_code->co_consts, oparg); } // Cache layout: counter/1, func_version/2, min_args/1 diff --git a/Python/ceval.c b/Python/ceval.c index 92235519c993b6..7d60cf987e9c47 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -773,18 +773,11 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int /* Local "register" variables. * These are cached values from the frame and code object. */ - PyObject *names; - PyObject *consts; _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; \ - } \ assert(_PyInterpreterFrame_LASTI(frame) >= -1); \ /* Jump back to the last instruction executed... */ \ next_instr = frame->prev_instr + 1; \ diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 21073cbf978ce0..631b7844f9ed88 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -50,7 +50,7 @@ TARGET(LOAD_CONST) { PREDICTED(LOAD_CONST); PyObject *value; - value = GETITEM(consts, oparg); + value = GETITEM(frame->f_code->co_consts, oparg); Py_INCREF(value); STACK_GROW(1); stack_pointer[-1] = value; @@ -101,7 +101,7 @@ oparg = (next_instr++)->op.arg; { PyObject *value; - value = GETITEM(consts, oparg); + value = GETITEM(frame->f_code->co_consts, oparg); Py_INCREF(value); _tmp_1 = value; } @@ -150,7 +150,7 @@ PyObject *_tmp_2; { PyObject *value; - value = GETITEM(consts, oparg); + value = GETITEM(frame->f_code->co_consts, oparg); Py_INCREF(value); _tmp_2 = value; } @@ -744,7 +744,7 @@ } TARGET(RETURN_CONST) { - PyObject *retval = GETITEM(consts, oparg); + PyObject *retval = GETITEM(frame->f_code->co_consts, oparg); Py_INCREF(retval); assert(EMPTY()); _PyFrame_SetStackPointer(frame, stack_pointer); @@ -1083,7 +1083,7 @@ TARGET(STORE_NAME) { PyObject *v = stack_pointer[-1]; - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); PyObject *ns = LOCALS(); int err; if (ns == NULL) { @@ -1103,7 +1103,7 @@ } TARGET(DELETE_NAME) { - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); PyObject *ns = LOCALS(); int err; if (ns == NULL) { @@ -1217,7 +1217,7 @@ #if ENABLE_SPECIALIZATION if (ADAPTIVE_COUNTER_IS_ZERO(counter)) { assert(cframe.use_tracing == 0); - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); next_instr--; _Py_Specialize_StoreAttr(owner, next_instr, name); DISPATCH_SAME_OPARG(); @@ -1228,7 +1228,7 @@ #else (void)counter; // Unused. #endif /* ENABLE_SPECIALIZATION */ - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); int err = PyObject_SetAttr(owner, name, v); Py_DECREF(v); Py_DECREF(owner); @@ -1240,7 +1240,7 @@ TARGET(DELETE_ATTR) { PyObject *owner = stack_pointer[-1]; - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); int err = PyObject_SetAttr(owner, name, (PyObject *)NULL); Py_DECREF(owner); if (err) goto pop_1_error; @@ -1250,7 +1250,7 @@ TARGET(STORE_GLOBAL) { PyObject *v = stack_pointer[-1]; - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); int err = PyDict_SetItem(GLOBALS(), name, v); Py_DECREF(v); if (err) goto pop_1_error; @@ -1259,7 +1259,7 @@ } TARGET(DELETE_GLOBAL) { - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); int err; err = PyDict_DelItem(GLOBALS(), name); // Can't use ERROR_IF here. @@ -1275,7 +1275,7 @@ TARGET(LOAD_NAME) { PyObject *v; - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); PyObject *locals = LOCALS(); if (locals == NULL) { _PyErr_Format(tstate, PyExc_SystemError, @@ -1347,7 +1347,7 @@ _PyLoadGlobalCache *cache = (_PyLoadGlobalCache *)next_instr; if (ADAPTIVE_COUNTER_IS_ZERO(cache->counter)) { assert(cframe.use_tracing == 0); - PyObject *name = GETITEM(names, oparg>>1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1); next_instr--; _Py_Specialize_LoadGlobal(GLOBALS(), BUILTINS(), next_instr, name); DISPATCH_SAME_OPARG(); @@ -1355,7 +1355,7 @@ STAT_INC(LOAD_GLOBAL, deferred); DECREMENT_ADAPTIVE_COUNTER(cache->counter); #endif /* ENABLE_SPECIALIZATION */ - PyObject *name = GETITEM(names, oparg>>1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1); if (PyDict_CheckExact(GLOBALS()) && PyDict_CheckExact(BUILTINS())) { @@ -1797,7 +1797,7 @@ _PyAttrCache *cache = (_PyAttrCache *)next_instr; if (ADAPTIVE_COUNTER_IS_ZERO(cache->counter)) { assert(cframe.use_tracing == 0); - PyObject *name = GETITEM(names, oparg>>1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1); next_instr--; _Py_Specialize_LoadAttr(owner, next_instr, name); DISPATCH_SAME_OPARG(); @@ -1805,7 +1805,7 @@ STAT_INC(LOAD_ATTR, deferred); DECREMENT_ADAPTIVE_COUNTER(cache->counter); #endif /* ENABLE_SPECIALIZATION */ - PyObject *name = GETITEM(names, oparg >> 1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg >> 1); if (oparg & 1) { /* Designed to work in tandem with CALL, pushes two values. */ PyObject* meth = NULL; @@ -1916,7 +1916,7 @@ PyDictObject *dict = (PyDictObject *)_PyDictOrValues_GetDict(dorv); DEOPT_IF(dict == NULL, LOAD_ATTR); assert(PyDict_CheckExact((PyObject *)dict)); - PyObject *name = GETITEM(names, oparg>>1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1); uint16_t hint = index; DEOPT_IF(hint >= (size_t)dict->ma_keys->dk_nentries, LOAD_ATTR); if (DK_IS_UNICODE(dict->ma_keys)) { @@ -2040,7 +2040,7 @@ DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), LOAD_ATTR); STAT_INC(LOAD_ATTR, hit); - PyObject *name = GETITEM(names, oparg >> 1); + PyObject *name = GETITEM(frame->f_code->co_names, oparg >> 1); Py_INCREF(f); _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f, 2); // Manipulate stack directly because we exit with DISPATCH_INLINED(). @@ -2096,7 +2096,7 @@ PyDictObject *dict = (PyDictObject *)_PyDictOrValues_GetDict(dorv); DEOPT_IF(dict == NULL, STORE_ATTR); assert(PyDict_CheckExact((PyObject *)dict)); - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); DEOPT_IF(hint >= (size_t)dict->ma_keys->dk_nentries, STORE_ATTR); PyObject *old_value; uint64_t new_version; @@ -2349,7 +2349,7 @@ PyObject *fromlist = stack_pointer[-1]; PyObject *level = stack_pointer[-2]; PyObject *res; - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); res = import_name(tstate, frame, name, fromlist, level); Py_DECREF(level); Py_DECREF(fromlist); @@ -2362,7 +2362,7 @@ TARGET(IMPORT_FROM) { PyObject *from = stack_pointer[-1]; PyObject *res; - PyObject *name = GETITEM(names, oparg); + PyObject *name = GETITEM(frame->f_code->co_names, oparg); res = import_from(tstate, from, name); if (res == NULL) goto error; STACK_GROW(1); @@ -2997,8 +2997,8 @@ TARGET(KW_NAMES) { assert(kwnames == NULL); - assert(oparg < PyTuple_GET_SIZE(consts)); - kwnames = GETITEM(consts, oparg); + assert(oparg < PyTuple_GET_SIZE(frame->f_code->co_consts)); + kwnames = GETITEM(frame->f_code->co_consts, oparg); DISPATCH(); }