bpo-42006: Stop using PyDict_GetItem, PyDict_GetItemString and _PyDict_GetItemId. (GH-22648)
These functions are considered not safe because they suppress all internal errors
and can return wrong result. PyDict_GetItemString and _PyDict_GetItemId can
also silence current exception in rare cases.
Remove no longer used _PyDict_GetItemId.
Add _PyDict_ContainsId and rename _PyDict_Contains into
_PyDict_Contains_KnownHash.
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 42d71e5..00d6ab3 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -3434,7 +3434,7 @@ PyDict_Contains(PyObject *op, PyObject *key)
/* Internal version of PyDict_Contains used when the hash value is already known */
int
-_PyDict_Contains(PyObject *op, PyObject *key, Py_hash_t hash)
+_PyDict_Contains_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash)
{
PyDictObject *mp = (PyDictObject *)op;
PyObject *value;
@@ -3446,6 +3446,16 @@ _PyDict_Contains(PyObject *op, PyObject *key, Py_hash_t hash)
return (ix != DKIX_EMPTY && value != NULL);
}
+int
+_PyDict_ContainsId(PyObject *op, struct _Py_Identifier *key)
+{
+ PyObject *kv = _PyUnicode_FromId(key); /* borrowed */
+ if (kv == NULL) {
+ return -1;
+ }
+ return PyDict_Contains(op, kv);
+}
+
/* Hack to implement "key in dict" */
static PySequenceMethods dict_as_sequence = {
0, /* sq_length */
@@ -3590,18 +3600,6 @@ PyTypeObject PyDict_Type = {
.tp_vectorcall = dict_vectorcall,
};
-PyObject *
-_PyDict_GetItemId(PyObject *dp, struct _Py_Identifier *key)
-{
- PyObject *kv;
- kv = _PyUnicode_FromId(key); /* borrowed */
- if (kv == NULL) {
- PyErr_Clear();
- return NULL;
- }
- return PyDict_GetItem(dp, kv);
-}
-
/* For backward compatibility with old dictionary interface */
PyObject *
diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c
index ee4ed97..c3ceb78 100644
--- a/Objects/moduleobject.c
+++ b/Objects/moduleobject.c
@@ -477,10 +477,12 @@ PyModule_GetNameObject(PyObject *m)
}
d = ((PyModuleObject *)m)->md_dict;
if (d == NULL ||
- (name = _PyDict_GetItemId(d, &PyId___name__)) == NULL ||
+ (name = _PyDict_GetItemIdWithError(d, &PyId___name__)) == NULL ||
!PyUnicode_Check(name))
{
- PyErr_SetString(PyExc_SystemError, "nameless module");
+ if (!PyErr_Occurred()) {
+ PyErr_SetString(PyExc_SystemError, "nameless module");
+ }
return NULL;
}
Py_INCREF(name);
@@ -509,10 +511,12 @@ PyModule_GetFilenameObject(PyObject *m)
}
d = ((PyModuleObject *)m)->md_dict;
if (d == NULL ||
- (fileobj = _PyDict_GetItemId(d, &PyId___file__)) == NULL ||
+ (fileobj = _PyDict_GetItemIdWithError(d, &PyId___file__)) == NULL ||
!PyUnicode_Check(fileobj))
{
- PyErr_SetString(PyExc_SystemError, "module filename missing");
+ if (!PyErr_Occurred()) {
+ PyErr_SetString(PyExc_SystemError, "module filename missing");
+ }
return NULL;
}
Py_INCREF(fileobj);
@@ -721,14 +725,21 @@ module_getattro(PyModuleObject *m, PyObject *name)
PyErr_Clear();
if (m->md_dict) {
_Py_IDENTIFIER(__getattr__);
- getattr = _PyDict_GetItemId(m->md_dict, &PyId___getattr__);
+ getattr = _PyDict_GetItemIdWithError(m->md_dict, &PyId___getattr__);
if (getattr) {
return PyObject_CallOneArg(getattr, name);
}
- mod_name = _PyDict_GetItemId(m->md_dict, &PyId___name__);
+ if (PyErr_Occurred()) {
+ return NULL;
+ }
+ mod_name = _PyDict_GetItemIdWithError(m->md_dict, &PyId___name__);
if (mod_name && PyUnicode_Check(mod_name)) {
Py_INCREF(mod_name);
- PyObject *spec = _PyDict_GetItemId(m->md_dict, &PyId___spec__);
+ PyObject *spec = _PyDict_GetItemIdWithError(m->md_dict, &PyId___spec__);
+ if (spec == NULL && PyErr_Occurred()) {
+ Py_DECREF(mod_name);
+ return NULL;
+ }
Py_XINCREF(spec);
if (_PyModuleSpec_IsInitializing(spec)) {
PyErr_Format(PyExc_AttributeError,
@@ -746,6 +757,9 @@ module_getattro(PyModuleObject *m, PyObject *name)
Py_DECREF(mod_name);
return NULL;
}
+ else if (PyErr_Occurred()) {
+ return NULL;
+ }
}
PyErr_Format(PyExc_AttributeError,
"module has no attribute '%U'", name);
diff --git a/Objects/setobject.c b/Objects/setobject.c
index b271149..af8ee03 100644
--- a/Objects/setobject.c
+++ b/Objects/setobject.c
@@ -1498,7 +1498,7 @@ set_difference(PySetObject *so, PyObject *other)
while (set_next(so, &pos, &entry)) {
key = entry->key;
hash = entry->hash;
- rv = _PyDict_Contains(other, key, hash);
+ rv = _PyDict_Contains_KnownHash(other, key, hash);
if (rv < 0) {
Py_DECREF(result);
return NULL;
diff --git a/Objects/structseq.c b/Objects/structseq.c
index 8ae8f28..5caa3bd 100644
--- a/Objects/structseq.c
+++ b/Objects/structseq.c
@@ -19,20 +19,33 @@ static const char unnamed_fields_key[] = "n_unnamed_fields";
/* Fields with this name have only a field index, not a field name.
They are only allowed for indices < n_visible_fields. */
const char * const PyStructSequence_UnnamedField = "unnamed field";
+
_Py_IDENTIFIER(n_sequence_fields);
_Py_IDENTIFIER(n_fields);
_Py_IDENTIFIER(n_unnamed_fields);
-#define VISIBLE_SIZE(op) Py_SIZE(op)
-#define VISIBLE_SIZE_TP(tp) PyLong_AsSsize_t( \
- _PyDict_GetItemId((tp)->tp_dict, &PyId_n_sequence_fields))
+static ssize_t
+get_type_attr_as_size(PyTypeObject *tp, _Py_Identifier *id)
+{
+ PyObject *name = _PyUnicode_FromId(id);
+ if (name == NULL) {
+ return -1;
+ }
+ PyObject *v = PyDict_GetItemWithError(tp->tp_dict, name);
+ if (v == NULL && !PyErr_Occurred()) {
+ PyErr_Format(PyExc_TypeError,
+ "Missed attribute '%U' of type %s",
+ name, tp->tp_name);
+ }
+ return PyLong_AsSsize_t(v);
+}
-#define REAL_SIZE_TP(tp) PyLong_AsSsize_t( \
- _PyDict_GetItemId((tp)->tp_dict, &PyId_n_fields))
+#define VISIBLE_SIZE(op) Py_SIZE(op)
+#define VISIBLE_SIZE_TP(tp) get_type_attr_as_size(tp, &PyId_n_sequence_fields)
+#define REAL_SIZE_TP(tp) get_type_attr_as_size(tp, &PyId_n_fields)
#define REAL_SIZE(op) REAL_SIZE_TP(Py_TYPE(op))
-#define UNNAMED_FIELDS_TP(tp) PyLong_AsSsize_t( \
- _PyDict_GetItemId((tp)->tp_dict, &PyId_n_unnamed_fields))
+#define UNNAMED_FIELDS_TP(tp) get_type_attr_as_size(tp, &PyId_n_unnamed_fields)
#define UNNAMED_FIELDS(op) UNNAMED_FIELDS_TP(Py_TYPE(op))
@@ -41,13 +54,20 @@ PyStructSequence_New(PyTypeObject *type)
{
PyStructSequence *obj;
Py_ssize_t size = REAL_SIZE_TP(type), i;
+ if (size < 0) {
+ return NULL;
+ }
+ Py_ssize_t vsize = VISIBLE_SIZE_TP(type);
+ if (vsize < 0) {
+ return NULL;
+ }
obj = PyObject_GC_NewVar(PyStructSequence, type, size);
if (obj == NULL)
return NULL;
/* Hack the size of the variable object, so invisible fields don't appear
to Python code. */
- Py_SET_SIZE(obj, VISIBLE_SIZE_TP(type));
+ Py_SET_SIZE(obj, vsize);
for (i = 0; i < size; i++)
obj->ob_item[i] = NULL;
@@ -121,6 +141,19 @@ structseq_new_impl(PyTypeObject *type, PyObject *arg, PyObject *dict)
PyStructSequence *res = NULL;
Py_ssize_t len, min_len, max_len, i, n_unnamed_fields;
+ min_len = VISIBLE_SIZE_TP(type);
+ if (min_len < 0) {
+ return NULL;
+ }
+ max_len = REAL_SIZE_TP(type);
+ if (max_len < 0) {
+ return NULL;
+ }
+ n_unnamed_fields = UNNAMED_FIELDS_TP(type);
+ if (n_unnamed_fields < 0) {
+ return NULL;
+ }
+
arg = PySequence_Fast(arg, "constructor requires a sequence");
if (!arg) {
@@ -136,10 +169,6 @@ structseq_new_impl(PyTypeObject *type, PyObject *arg, PyObject *dict)
}
len = PySequence_Fast_GET_SIZE(arg);
- min_len = VISIBLE_SIZE_TP(type);
- max_len = REAL_SIZE_TP(type);
- n_unnamed_fields = UNNAMED_FIELDS_TP(type);
-
if (min_len != max_len) {
if (len < min_len) {
PyErr_Format(PyExc_TypeError,
@@ -177,18 +206,26 @@ structseq_new_impl(PyTypeObject *type, PyObject *arg, PyObject *dict)
Py_INCREF(v);
res->ob_item[i] = v;
}
+ Py_DECREF(arg);
for (; i < max_len; ++i) {
- if (dict && (ob = PyDict_GetItemString(
- dict, type->tp_members[i-n_unnamed_fields].name))) {
+ if (dict == NULL) {
+ ob = Py_None;
}
else {
- ob = Py_None;
+ ob = _PyDict_GetItemStringWithError(dict,
+ type->tp_members[i-n_unnamed_fields].name);
+ if (ob == NULL) {
+ if (PyErr_Occurred()) {
+ Py_DECREF(res);
+ return NULL;
+ }
+ ob = Py_None;
+ }
}
Py_INCREF(ob);
res->ob_item[i] = ob;
}
- Py_DECREF(arg);
_PyObject_GC_TRACK(res);
return (PyObject*) res;
}
@@ -288,8 +325,14 @@ structseq_reduce(PyStructSequence* self, PyObject *Py_UNUSED(ignored))
Py_ssize_t n_fields, n_visible_fields, n_unnamed_fields, i;
n_fields = REAL_SIZE(self);
+ if (n_fields < 0) {
+ return NULL;
+ }
n_visible_fields = VISIBLE_SIZE(self);
n_unnamed_fields = UNNAMED_FIELDS(self);
+ if (n_unnamed_fields < 0) {
+ return NULL;
+ }
tup = _PyTuple_FromArray(self->ob_item, n_visible_fields);
if (!tup)
goto error;
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 36c7662..6626169 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -4767,8 +4767,11 @@ object___reduce_ex___impl(PyObject *self, int protocol)
_Py_IDENTIFIER(__reduce__);
if (objreduce == NULL) {
- objreduce = _PyDict_GetItemId(PyBaseObject_Type.tp_dict,
- &PyId___reduce__);
+ objreduce = _PyDict_GetItemIdWithError(PyBaseObject_Type.tp_dict,
+ &PyId___reduce__);
+ if (objreduce == NULL && PyErr_Occurred()) {
+ return NULL;
+ }
}
if (_PyObject_LookupAttrId(self, &PyId___reduce__, &reduce) < 0) {
@@ -5181,14 +5184,14 @@ overrides_hash(PyTypeObject *type)
_Py_IDENTIFIER(__eq__);
assert(dict != NULL);
- if (_PyDict_GetItemId(dict, &PyId___eq__) != NULL)
- return 1;
- if (_PyDict_GetItemId(dict, &PyId___hash__) != NULL)
- return 1;
- return 0;
+ int r = _PyDict_ContainsId(dict, &PyId___eq__);
+ if (r == 0) {
+ r = _PyDict_ContainsId(dict, &PyId___hash__);
+ }
+ return r;
}
-static void
+static int
inherit_slots(PyTypeObject *type, PyTypeObject *base)
{
PyTypeObject *basebase;
@@ -5331,11 +5334,16 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
/* Copy comparison-related slots only when
not overriding them anywhere */
if (type->tp_richcompare == NULL &&
- type->tp_hash == NULL &&
- !overrides_hash(type))
+ type->tp_hash == NULL)
{
- type->tp_richcompare = base->tp_richcompare;
- type->tp_hash = base->tp_hash;
+ int r = overrides_hash(type);
+ if (r < 0) {
+ return -1;
+ }
+ if (!r) {
+ type->tp_richcompare = base->tp_richcompare;
+ type->tp_hash = base->tp_hash;
+ }
}
}
{
@@ -5378,6 +5386,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
* obvious to be done -- the type is on its own.
*/
}
+ return 0;
}
static int add_operators(PyTypeObject *);
@@ -5507,8 +5516,11 @@ PyType_Ready(PyTypeObject *type)
n = PyTuple_GET_SIZE(bases);
for (i = 1; i < n; i++) {
PyObject *b = PyTuple_GET_ITEM(bases, i);
- if (PyType_Check(b))
- inherit_slots(type, (PyTypeObject *)b);
+ if (PyType_Check(b)) {
+ if (inherit_slots(type, (PyTypeObject *)b) < 0) {
+ goto error;
+ }
+ }
}
/* All bases of statically allocated type should be statically allocated */