Skip to content

Commit f5559f3

Browse files
vstinnervstinner
authored
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.
1 parent 154477b commit f5559f3

File tree

4 files changed

+105
-32
lines changed

4 files changed

+105
-32
lines changed

‎Python/assemble.c‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
#define ERROR -1
1919

2020
#define RETURN_IF_ERROR(X) \
21-
if ((X) == -1) { \
21+
if ((X) < 0) { \
2222
return ERROR; \
2323
}
2424

@@ -448,13 +448,17 @@ static PyObject *
448448
dict_keys_inorder(PyObject *dict, Py_ssize_t offset)
449449
{
450450
PyObject *tuple, *k, *v;
451-
Py_ssize_t i, pos = 0, size = PyDict_GET_SIZE(dict);
451+
Py_ssize_t pos = 0, size = PyDict_GET_SIZE(dict);
452452

453453
tuple = PyTuple_New(size);
454454
if (tuple == NULL)
455455
return NULL;
456456
while (PyDict_Next(dict, &pos, &k, &v)) {
457-
i = PyLong_AS_LONG(v);
457+
Py_ssize_t i = PyLong_AsSsize_t(v);
458+
if (i == -1 && PyErr_Occurred()) {
459+
Py_DECREF(tuple);
460+
return NULL;
461+
}
458462
assert((i - offset) < size);
459463
assert((i - offset) >= 0);
460464
PyTuple_SET_ITEM(tuple, i - offset, Py_NewRef(k));
@@ -466,24 +470,34 @@ dict_keys_inorder(PyObject *dict, Py_ssize_t offset)
466470
extern void _Py_set_localsplus_info(int, PyObject *, unsigned char,
467471
PyObject *, PyObject *);
468472

469-
static void
473+
static int
470474
compute_localsplus_info(_PyCompile_CodeUnitMetadata *umd, int nlocalsplus,
471475
PyObject *names, PyObject *kinds)
472476
{
473477
PyObject *k, *v;
474478
Py_ssize_t pos = 0;
475479
while (PyDict_Next(umd->u_varnames, &pos, &k, &v)) {
476-
int offset = (int)PyLong_AS_LONG(v);
480+
int offset = _PyLong_AsInt(v);
481+
if (offset == -1 && PyErr_Occurred()) {
482+
return ERROR;
483+
}
477484
assert(offset >= 0);
478485
assert(offset < nlocalsplus);
486+
479487
// For now we do not distinguish arg kinds.
480488
_PyLocals_Kind kind = CO_FAST_LOCAL;
481-
if (PyDict_Contains(umd->u_fasthidden, k)) {
489+
int has_key = PyDict_Contains(umd->u_fasthidden, k);
490+
RETURN_IF_ERROR(has_key);
491+
if (has_key) {
482492
kind |= CO_FAST_HIDDEN;
483493
}
484-
if (PyDict_GetItem(umd->u_cellvars, k) != NULL) {
494+
495+
has_key = PyDict_Contains(umd->u_cellvars, k);
496+
RETURN_IF_ERROR(has_key);
497+
if (has_key) {
485498
kind |= CO_FAST_CELL;
486499
}
500+
487501
_Py_set_localsplus_info(offset, k, kind, names, kinds);
488502
}
489503
int nlocals = (int)PyDict_GET_SIZE(umd->u_varnames);
@@ -492,12 +506,18 @@ compute_localsplus_info(_PyCompile_CodeUnitMetadata *umd, int nlocalsplus,
492506
int numdropped = 0;
493507
pos = 0;
494508
while (PyDict_Next(umd->u_cellvars, &pos, &k, &v)) {
495-
if (PyDict_GetItem(umd->u_varnames, k) != NULL) {
509+
int has_name = PyDict_Contains(umd->u_varnames, k);
510+
RETURN_IF_ERROR(has_name);
511+
if (has_name) {
496512
// Skip cells that are already covered by locals.
497513
numdropped += 1;
498514
continue;
499515
}
500-
int offset = (int)PyLong_AS_LONG(v);
516+
517+
int offset = _PyLong_AsInt(v);
518+
if (offset == -1 && PyErr_Occurred()) {
519+
return ERROR;
520+
}
501521
assert(offset >= 0);
502522
offset += nlocals - numdropped;
503523
assert(offset < nlocalsplus);
@@ -506,12 +526,16 @@ compute_localsplus_info(_PyCompile_CodeUnitMetadata *umd, int nlocalsplus,
506526

507527
pos = 0;
508528
while (PyDict_Next(umd->u_freevars, &pos, &k, &v)) {
509-
int offset = (int)PyLong_AS_LONG(v);
529+
int offset = _PyLong_AsInt(v);
530+
if (offset == -1 && PyErr_Occurred()) {
531+
return ERROR;
532+
}
510533
assert(offset >= 0);
511534
offset += nlocals - numdropped;
512535
assert(offset < nlocalsplus);
513536
_Py_set_localsplus_info(offset, k, CO_FAST_FREE, names, kinds);
514537
}
538+
return SUCCESS;
515539
}
516540

517541
static PyCodeObject *
@@ -556,7 +580,10 @@ makecode(_PyCompile_CodeUnitMetadata *umd, struct assembler *a, PyObject *const_
556580
if (localspluskinds == NULL) {
557581
goto error;
558582
}
559-
compute_localsplus_info(umd, nlocalsplus, localsplusnames, localspluskinds);
583+
if (compute_localsplus_info(umd, nlocalsplus,
584+
localsplusnames, localspluskinds) == ERROR) {
585+
goto error;
586+
}
560587

561588
struct _PyCodeConstructor con = {
562589
.filename = filename,

‎Python/compile.c‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4212,9 +4212,20 @@ compiler_nameop(struct compiler *c, location loc,
42124212
optype = OP_DEREF;
42134213
break;
42144214
case LOCAL:
4215-
if (_PyST_IsFunctionLike(c->u->u_ste) ||
4216-
(PyDict_GetItem(c->u->u_metadata.u_fasthidden, mangled) == Py_True))
4215+
if (_PyST_IsFunctionLike(c->u->u_ste)) {
42174216
optype = OP_FAST;
4217+
}
4218+
else {
4219+
PyObject *item;
4220+
if (PyDict_GetItemRef(c->u->u_metadata.u_fasthidden, mangled,
4221+
&item) < 0) {
4222+
goto error;
4223+
}
4224+
if (item == Py_True) {
4225+
optype = OP_FAST;
4226+
}
4227+
Py_XDECREF(item);
4228+
}
42184229
break;
42194230
case GLOBAL_IMPLICIT:
42204231
if (_PyST_IsFunctionLike(c->u->u_ste))
@@ -4239,15 +4250,15 @@ compiler_nameop(struct compiler *c, location loc,
42394250
op = LOAD_FROM_DICT_OR_DEREF;
42404251
// First load the locals
42414252
if (codegen_addop_noarg(INSTR_SEQUENCE(c), LOAD_LOCALS, loc) < 0) {
4242-
return ERROR;
4253+
goto error;
42434254
}
42444255
}
42454256
else if (c->u->u_ste->ste_can_see_class_scope) {
42464257
op = LOAD_FROM_DICT_OR_DEREF;
42474258
// First load the classdict
42484259
if (compiler_addop_o(c->u, loc, LOAD_DEREF,
42494260
c->u->u_metadata.u_freevars, &_Py_ID(__classdict__)) < 0) {
4250-
return ERROR;
4261+
goto error;
42514262
}
42524263
}
42534264
else {
@@ -4274,7 +4285,7 @@ compiler_nameop(struct compiler *c, location loc,
42744285
// First load the classdict
42754286
if (compiler_addop_o(c->u, loc, LOAD_DEREF,
42764287
c->u->u_metadata.u_freevars, &_Py_ID(__classdict__)) < 0) {
4277-
return ERROR;
4288+
goto error;
42784289
}
42794290
} else {
42804291
op = LOAD_GLOBAL;
@@ -4308,6 +4319,10 @@ compiler_nameop(struct compiler *c, location loc,
43084319
arg <<= 1;
43094320
}
43104321
return codegen_addop_i(INSTR_SEQUENCE(c), op, arg, loc);
4322+
4323+
error:
4324+
Py_DECREF(mangled);
4325+
return ERROR;
43114326
}
43124327

43134328
static int
@@ -5536,8 +5551,13 @@ push_inlined_comprehension_state(struct compiler *c, location loc,
55365551
if ((symbol & DEF_LOCAL && !(symbol & DEF_NONLOCAL)) || in_class_block) {
55375552
if (!_PyST_IsFunctionLike(c->u->u_ste)) {
55385553
// non-function scope: override this name to use fast locals
5539-
PyObject *orig = PyDict_GetItem(c->u->u_metadata.u_fasthidden, k);
5540-
if (orig != Py_True) {
5554+
PyObject *orig;
5555+
if (PyDict_GetItemRef(c->u->u_metadata.u_fasthidden, k, &orig) < 0) {
5556+
return ERROR;
5557+
}
5558+
int orig_is_true = (orig == Py_True);
5559+
Py_XDECREF(orig);
5560+
if (!orig_is_true) {
55415561
if (PyDict_SetItem(c->u->u_metadata.u_fasthidden, k, Py_True) < 0) {
55425562
return ERROR;
55435563
}

‎Python/flowgraph.c‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2404,17 +2404,31 @@ build_cellfixedoffsets(_PyCompile_CodeUnitMetadata *umd)
24042404
PyObject *varname, *cellindex;
24052405
Py_ssize_t pos = 0;
24062406
while (PyDict_Next(umd->u_cellvars, &pos, &varname, &cellindex)) {
2407-
PyObject *varindex = PyDict_GetItem(umd->u_varnames, varname);
2408-
if (varindex != NULL) {
2409-
assert(PyLong_AS_LONG(cellindex) < INT_MAX);
2410-
assert(PyLong_AS_LONG(varindex) < INT_MAX);
2411-
int oldindex = (int)PyLong_AS_LONG(cellindex);
2412-
int argoffset = (int)PyLong_AS_LONG(varindex);
2413-
fixed[oldindex] = argoffset;
2407+
PyObject *varindex;
2408+
if (PyDict_GetItemRef(umd->u_varnames, varname, &varindex) < 0) {
2409+
goto error;
2410+
}
2411+
if (varindex == NULL) {
2412+
continue;
2413+
}
2414+
2415+
int argoffset = _PyLong_AsInt(varindex);
2416+
Py_DECREF(varindex);
2417+
if (argoffset == -1 && PyErr_Occurred()) {
2418+
goto error;
24142419
}
2415-
}
24162420

2421+
int oldindex = _PyLong_AsInt(cellindex);
2422+
if (oldindex == -1 && PyErr_Occurred()) {
2423+
goto error;
2424+
}
2425+
fixed[oldindex] = argoffset;
2426+
}
24172427
return fixed;
2428+
2429+
error:
2430+
PyMem_Free(fixed);
2431+
return NULL;
24182432
}
24192433

24202434
#define IS_GENERATOR(CF) \

‎Python/pylifecycle.c‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -762,18 +762,30 @@ pycore_init_builtins(PyThreadState *tstate)
762762
}
763763
interp->builtins = Py_NewRef(builtins_dict);
764764

765-
PyObject *isinstance = PyDict_GetItem(builtins_dict, &_Py_ID(isinstance));
766-
assert(isinstance);
765+
PyObject *isinstance = PyDict_GetItemWithError(builtins_dict, &_Py_ID(isinstance));
766+
if (!isinstance) {
767+
goto error;
768+
}
767769
interp->callable_cache.isinstance = isinstance;
768-
PyObject *len = PyDict_GetItem(builtins_dict, &_Py_ID(len));
769-
assert(len);
770+
771+
PyObject *len = PyDict_GetItemWithError(builtins_dict, &_Py_ID(len));
772+
if (!len) {
773+
goto error;
774+
}
770775
interp->callable_cache.len = len;
776+
771777
PyObject *list_append = _PyType_Lookup(&PyList_Type, &_Py_ID(append));
772-
assert(list_append);
778+
if (list_append == NULL) {
779+
goto error;
780+
}
773781
interp->callable_cache.list_append = list_append;
782+
774783
PyObject *object__getattribute__ = _PyType_Lookup(&PyBaseObject_Type, &_Py_ID(__getattribute__));
775-
assert(object__getattribute__);
784+
if (object__getattribute__ == NULL) {
785+
goto error;
786+
}
776787
interp->callable_cache.object__getattribute__ = object__getattribute__;
788+
777789
if (_PyBuiltins_AddExceptions(bimod) < 0) {
778790
return _PyStatus_ERR("failed to add exceptions to builtins");
779791
}

0 commit comments

Comments
 (0)