[Python-checkins] gh-108308: Replace PyDict_GetItem() with PyDict_GetItemRef() (#108309)
vstinner
webhook-mailer at python.org
Wed Aug 23 11:40:54 EDT 2023
https://github.com/python/cpython/commit/f5559f38d9831e7e55a518e516bcd620ec13af14
commit: f5559f38d9831e7e55a518e516bcd620ec13af14
branch: main
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at python.org>
date: 2023-08-23T17:40:26+02:00
summary:
gh-108308: Replace PyDict_GetItem() with PyDict_GetItemRef() (#108309)
Replace PyDict_GetItem() calls with PyDict_GetItemRef()
or PyDict_GetItemWithError() to handle errors.
* Replace PyLong_AS_LONG() with _PyLong_AsInt()
and check for errors.
* Check for PyDict_Contains() error.
* pycore_init_builtins() checks for _PyType_Lookup() failure.
files:
M Python/assemble.c
M Python/compile.c
M Python/flowgraph.c
M Python/pylifecycle.c
diff --git a/Python/assemble.c b/Python/assemble.c
index b7012534d6cc4..4f66cf294e38c 100644
--- a/Python/assemble.c
+++ b/Python/assemble.c
@@ -18,7 +18,7 @@
#define ERROR -1
#define RETURN_IF_ERROR(X) \
- if ((X) == -1) { \
+ if ((X) < 0) { \
return ERROR; \
}
@@ -448,13 +448,17 @@ static PyObject *
dict_keys_inorder(PyObject *dict, Py_ssize_t offset)
{
PyObject *tuple, *k, *v;
- Py_ssize_t i, pos = 0, size = PyDict_GET_SIZE(dict);
+ Py_ssize_t pos = 0, size = PyDict_GET_SIZE(dict);
tuple = PyTuple_New(size);
if (tuple == NULL)
return NULL;
while (PyDict_Next(dict, &pos, &k, &v)) {
- i = PyLong_AS_LONG(v);
+ Py_ssize_t i = PyLong_AsSsize_t(v);
+ if (i == -1 && PyErr_Occurred()) {
+ Py_DECREF(tuple);
+ return NULL;
+ }
assert((i - offset) < size);
assert((i - offset) >= 0);
PyTuple_SET_ITEM(tuple, i - offset, Py_NewRef(k));
@@ -466,24 +470,34 @@ dict_keys_inorder(PyObject *dict, Py_ssize_t offset)
extern void _Py_set_localsplus_info(int, PyObject *, unsigned char,
PyObject *, PyObject *);
-static void
+static int
compute_localsplus_info(_PyCompile_CodeUnitMetadata *umd, int nlocalsplus,
PyObject *names, PyObject *kinds)
{
PyObject *k, *v;
Py_ssize_t pos = 0;
while (PyDict_Next(umd->u_varnames, &pos, &k, &v)) {
- int offset = (int)PyLong_AS_LONG(v);
+ int offset = _PyLong_AsInt(v);
+ if (offset == -1 && PyErr_Occurred()) {
+ return ERROR;
+ }
assert(offset >= 0);
assert(offset < nlocalsplus);
+
// For now we do not distinguish arg kinds.
_PyLocals_Kind kind = CO_FAST_LOCAL;
- if (PyDict_Contains(umd->u_fasthidden, k)) {
+ int has_key = PyDict_Contains(umd->u_fasthidden, k);
+ RETURN_IF_ERROR(has_key);
+ if (has_key) {
kind |= CO_FAST_HIDDEN;
}
- if (PyDict_GetItem(umd->u_cellvars, k) != NULL) {
+
+ has_key = PyDict_Contains(umd->u_cellvars, k);
+ RETURN_IF_ERROR(has_key);
+ if (has_key) {
kind |= CO_FAST_CELL;
}
+
_Py_set_localsplus_info(offset, k, kind, names, kinds);
}
int nlocals = (int)PyDict_GET_SIZE(umd->u_varnames);
@@ -492,12 +506,18 @@ compute_localsplus_info(_PyCompile_CodeUnitMetadata *umd, int nlocalsplus,
int numdropped = 0;
pos = 0;
while (PyDict_Next(umd->u_cellvars, &pos, &k, &v)) {
- if (PyDict_GetItem(umd->u_varnames, k) != NULL) {
+ int has_name = PyDict_Contains(umd->u_varnames, k);
+ RETURN_IF_ERROR(has_name);
+ if (has_name) {
// Skip cells that are already covered by locals.
numdropped += 1;
continue;
}
- int offset = (int)PyLong_AS_LONG(v);
+
+ int offset = _PyLong_AsInt(v);
+ if (offset == -1 && PyErr_Occurred()) {
+ return ERROR;
+ }
assert(offset >= 0);
offset += nlocals - numdropped;
assert(offset < nlocalsplus);
@@ -506,12 +526,16 @@ compute_localsplus_info(_PyCompile_CodeUnitMetadata *umd, int nlocalsplus,
pos = 0;
while (PyDict_Next(umd->u_freevars, &pos, &k, &v)) {
- int offset = (int)PyLong_AS_LONG(v);
+ int offset = _PyLong_AsInt(v);
+ if (offset == -1 && PyErr_Occurred()) {
+ return ERROR;
+ }
assert(offset >= 0);
offset += nlocals - numdropped;
assert(offset < nlocalsplus);
_Py_set_localsplus_info(offset, k, CO_FAST_FREE, names, kinds);
}
+ return SUCCESS;
}
static PyCodeObject *
@@ -556,7 +580,10 @@ makecode(_PyCompile_CodeUnitMetadata *umd, struct assembler *a, PyObject *const_
if (localspluskinds == NULL) {
goto error;
}
- compute_localsplus_info(umd, nlocalsplus, localsplusnames, localspluskinds);
+ if (compute_localsplus_info(umd, nlocalsplus,
+ localsplusnames, localspluskinds) == ERROR) {
+ goto error;
+ }
struct _PyCodeConstructor con = {
.filename = filename,
diff --git a/Python/compile.c b/Python/compile.c
index 4b2f70a7ef01d..b67a1885ddc98 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -4212,9 +4212,20 @@ compiler_nameop(struct compiler *c, location loc,
optype = OP_DEREF;
break;
case LOCAL:
- if (_PyST_IsFunctionLike(c->u->u_ste) ||
- (PyDict_GetItem(c->u->u_metadata.u_fasthidden, mangled) == Py_True))
+ if (_PyST_IsFunctionLike(c->u->u_ste)) {
optype = OP_FAST;
+ }
+ else {
+ PyObject *item;
+ if (PyDict_GetItemRef(c->u->u_metadata.u_fasthidden, mangled,
+ &item) < 0) {
+ goto error;
+ }
+ if (item == Py_True) {
+ optype = OP_FAST;
+ }
+ Py_XDECREF(item);
+ }
break;
case GLOBAL_IMPLICIT:
if (_PyST_IsFunctionLike(c->u->u_ste))
@@ -4239,7 +4250,7 @@ compiler_nameop(struct compiler *c, location loc,
op = LOAD_FROM_DICT_OR_DEREF;
// First load the locals
if (codegen_addop_noarg(INSTR_SEQUENCE(c), LOAD_LOCALS, loc) < 0) {
- return ERROR;
+ goto error;
}
}
else if (c->u->u_ste->ste_can_see_class_scope) {
@@ -4247,7 +4258,7 @@ compiler_nameop(struct compiler *c, location loc,
// First load the classdict
if (compiler_addop_o(c->u, loc, LOAD_DEREF,
c->u->u_metadata.u_freevars, &_Py_ID(__classdict__)) < 0) {
- return ERROR;
+ goto error;
}
}
else {
@@ -4274,7 +4285,7 @@ compiler_nameop(struct compiler *c, location loc,
// First load the classdict
if (compiler_addop_o(c->u, loc, LOAD_DEREF,
c->u->u_metadata.u_freevars, &_Py_ID(__classdict__)) < 0) {
- return ERROR;
+ goto error;
}
} else {
op = LOAD_GLOBAL;
@@ -4308,6 +4319,10 @@ compiler_nameop(struct compiler *c, location loc,
arg <<= 1;
}
return codegen_addop_i(INSTR_SEQUENCE(c), op, arg, loc);
+
+error:
+ Py_DECREF(mangled);
+ return ERROR;
}
static int
@@ -5536,8 +5551,13 @@ push_inlined_comprehension_state(struct compiler *c, location loc,
if ((symbol & DEF_LOCAL && !(symbol & DEF_NONLOCAL)) || in_class_block) {
if (!_PyST_IsFunctionLike(c->u->u_ste)) {
// non-function scope: override this name to use fast locals
- PyObject *orig = PyDict_GetItem(c->u->u_metadata.u_fasthidden, k);
- if (orig != Py_True) {
+ PyObject *orig;
+ if (PyDict_GetItemRef(c->u->u_metadata.u_fasthidden, k, &orig) < 0) {
+ return ERROR;
+ }
+ int orig_is_true = (orig == Py_True);
+ Py_XDECREF(orig);
+ if (!orig_is_true) {
if (PyDict_SetItem(c->u->u_metadata.u_fasthidden, k, Py_True) < 0) {
return ERROR;
}
diff --git a/Python/flowgraph.c b/Python/flowgraph.c
index 719ed92105074..e620e7cf1b9e9 100644
--- a/Python/flowgraph.c
+++ b/Python/flowgraph.c
@@ -2404,17 +2404,31 @@ build_cellfixedoffsets(_PyCompile_CodeUnitMetadata *umd)
PyObject *varname, *cellindex;
Py_ssize_t pos = 0;
while (PyDict_Next(umd->u_cellvars, &pos, &varname, &cellindex)) {
- PyObject *varindex = PyDict_GetItem(umd->u_varnames, varname);
- if (varindex != NULL) {
- assert(PyLong_AS_LONG(cellindex) < INT_MAX);
- assert(PyLong_AS_LONG(varindex) < INT_MAX);
- int oldindex = (int)PyLong_AS_LONG(cellindex);
- int argoffset = (int)PyLong_AS_LONG(varindex);
- fixed[oldindex] = argoffset;
+ PyObject *varindex;
+ if (PyDict_GetItemRef(umd->u_varnames, varname, &varindex) < 0) {
+ goto error;
+ }
+ if (varindex == NULL) {
+ continue;
+ }
+
+ int argoffset = _PyLong_AsInt(varindex);
+ Py_DECREF(varindex);
+ if (argoffset == -1 && PyErr_Occurred()) {
+ goto error;
}
- }
+ int oldindex = _PyLong_AsInt(cellindex);
+ if (oldindex == -1 && PyErr_Occurred()) {
+ goto error;
+ }
+ fixed[oldindex] = argoffset;
+ }
return fixed;
+
+error:
+ PyMem_Free(fixed);
+ return NULL;
}
#define IS_GENERATOR(CF) \
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 9837e0f0d52e6..8c321fbcfe410 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -762,18 +762,30 @@ pycore_init_builtins(PyThreadState *tstate)
}
interp->builtins = Py_NewRef(builtins_dict);
- PyObject *isinstance = PyDict_GetItem(builtins_dict, &_Py_ID(isinstance));
- assert(isinstance);
+ PyObject *isinstance = PyDict_GetItemWithError(builtins_dict, &_Py_ID(isinstance));
+ if (!isinstance) {
+ goto error;
+ }
interp->callable_cache.isinstance = isinstance;
- PyObject *len = PyDict_GetItem(builtins_dict, &_Py_ID(len));
- assert(len);
+
+ PyObject *len = PyDict_GetItemWithError(builtins_dict, &_Py_ID(len));
+ if (!len) {
+ goto error;
+ }
interp->callable_cache.len = len;
+
PyObject *list_append = _PyType_Lookup(&PyList_Type, &_Py_ID(append));
- assert(list_append);
+ if (list_append == NULL) {
+ goto error;
+ }
interp->callable_cache.list_append = list_append;
+
PyObject *object__getattribute__ = _PyType_Lookup(&PyBaseObject_Type, &_Py_ID(__getattribute__));
- assert(object__getattribute__);
+ if (object__getattribute__ == NULL) {
+ goto error;
+ }
interp->callable_cache.object__getattribute__ = object__getattribute__;
+
if (_PyBuiltins_AddExceptions(bimod) < 0) {
return _PyStatus_ERR("failed to add exceptions to builtins");
}
More information about the Python-checkins
mailing list