Skip to content

Commit

Permalink
gh-108308: Replace PyDict_GetItem() with PyDict_GetItemRef() (#108309)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@vstinner
vstinner committed Aug 23, 2023
1 parent 154477b commit f5559f3
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 32 deletions.
49 changes: 38 additions & 11 deletions Python/assemble.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#define ERROR -1

#define RETURN_IF_ERROR(X) \
if ((X) == -1) { \
if ((X) < 0) { \
return ERROR; \
}

Expand Down Expand Up @@ -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));
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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 *
Expand Down Expand Up @@ -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,
Expand Down
34 changes: 27 additions & 7 deletions Python/compile.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -4239,15 +4250,15 @@ 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) {
op = LOAD_FROM_DICT_OR_DEREF;
// 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 {
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
30 changes: 22 additions & 8 deletions Python/flowgraph.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down
24 changes: 18 additions & 6 deletions Python/pylifecycle.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down

0 comments on commit f5559f3

Please sign in to comment.