Issue #24115: Update uses of PyObject_IsTrue(), PyObject_Not(),

PyObject_IsInstance(), PyObject_RichCompareBool() and _PyDict_Contains()
to check for and handle errors correctly.

10 files changed, 92 insertions, 40 deletions

diff --git a/Misc/NEWS b/Misc/NEWS
index 2ccbae6..d6ff9e4 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,10 @@ Release date: 2015-07-05
 Core and Builtins
 -----------------
 
+- Issue #24115: Update uses of PyObject_IsTrue(), PyObject_Not(),
+  PyObject_IsInstance(), PyObject_RichCompareBool() and _PyDict_Contains()
+  to check for and handle errors correctly.
+
 - Issue #24328: Fix importing one character extension modules.
 
 - Issue #11205: In dictionary displays, evaluate the key before the value.
diff --git a/Modules/_json.c b/Modules/_json.c
index e2e8bf3..e4478ef 100644
--- a/Modules/_json.c
+++ b/Modules/_json.c
@@ -710,6 +710,9 @@ _parse_object_unicode(PyScannerObject *s, PyObject *pystr, Py_ssize_t idx, Py_ss
     int has_pairs_hook = (s->object_pairs_hook != Py_None);
     Py_ssize_t next_idx;
 
+    if (strict < 0)
+        return NULL;
+
     if (PyUnicode_READY(pystr) == -1)
         return NULL;
 
@@ -1055,6 +1058,7 @@ scan_once_unicode(PyScannerObject *s, PyObject *pystr, Py_ssize_t idx, Py_ssize_
     void *str;
     int kind;
     Py_ssize_t length;
+    int strict;
 
     if (PyUnicode_READY(pystr) == -1)
         return NULL;
@@ -1075,9 +1079,10 @@ scan_once_unicode(PyScannerObject *s, PyObject *pystr, Py_ssize_t idx, Py_ssize_
     switch (PyUnicode_READ(kind, str, idx)) {
         case '"':
             /* string */
-            return scanstring_unicode(pystr, idx + 1,
-                PyObject_IsTrue(s->strict),
-                next_idx_ptr);
+            strict = PyObject_IsTrue(s->strict);
+            if (strict < 0)
+                return NULL;
+            return scanstring_unicode(pystr, idx + 1, strict, next_idx_ptr);
         case '{':
             /* object */
             if (Py_EnterRecursiveCall(" while decoding a JSON object "
@@ -1327,12 +1332,13 @@ encoder_init(PyObject *self, PyObject *args, PyObject *kwds)
 
     PyEncoderObject *s;
     PyObject *markers, *defaultfn, *encoder, *indent, *key_separator;
-    PyObject *item_separator, *sort_keys, *skipkeys, *allow_nan;
+    PyObject *item_separator, *sort_keys, *skipkeys;
+    int allow_nan;
 
     assert(PyEncoder_Check(self));
     s = (PyEncoderObject *)self;
 
-    if (!PyArg_ParseTupleAndKeywords(args, kwds, "OOOOOOOOO:make_encoder", kwlist,
+    if (!PyArg_ParseTupleAndKeywords(args, kwds, "OOOOOOOOp:make_encoder", kwlist,
         &markers, &defaultfn, &encoder, &indent, &key_separator, &item_separator,
         &sort_keys, &skipkeys, &allow_nan))
         return -1;
@@ -1353,7 +1359,7 @@ encoder_init(PyObject *self, PyObject *args, PyObject *kwds)
             s->fast_encode = f;
         }
     }
-    s->allow_nan = PyObject_IsTrue(allow_nan);
+    s->allow_nan = allow_nan;
 
     Py_INCREF(s->markers);
     Py_INCREF(s->defaultfn);
@@ -1622,6 +1628,7 @@ encoder_listencode_dict(PyEncoderObject *s, _PyAccu *acc,
     PyObject *items;
     PyObject *item = NULL;
     int skipkeys;
+    int sortkeys;
     Py_ssize_t idx;
 
     if (open_dict == NULL || close_dict == NULL || empty_dict == NULL) {
@@ -1666,13 +1673,16 @@ encoder_listencode_dict(PyEncoderObject *s, _PyAccu *acc,
     items = PyMapping_Items(dct);
     if (items == NULL)
         goto bail;
-    if (PyObject_IsTrue(s->sort_keys) && PyList_Sort(items) < 0)
+    sortkeys = PyObject_IsTrue(s->sort_keys);
+    if (sortkeys < 0 || (sortkeys && PyList_Sort(items) < 0))
         goto bail;
     it = PyObject_GetIter(items);
     Py_DECREF(items);
     if (it == NULL)
         goto bail;
     skipkeys = PyObject_IsTrue(s->skipkeys);
+    if (skipkeys < 0)
+        goto bail;
     idx = 0;
     while ((item = PyIter_Next(it)) != NULL) {
         PyObject *encoded, *key, *value;
diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
index 812c8a3..bcb3aee 100644
--- a/Modules/_threadmodule.c
+++ b/Modules/_threadmodule.c
@@ -711,12 +711,18 @@ local_new(PyTypeObject *type, PyObject *args, PyObject *kw)
         "_localdummy_destroyed", (PyCFunction) _localdummy_destroyed, METH_O
     };
 
-    if (type->tp_init == PyBaseObject_Type.tp_init
-        && ((args && PyObject_IsTrue(args))
-        || (kw && PyObject_IsTrue(kw)))) {
-        PyErr_SetString(PyExc_TypeError,
-                  "Initialization arguments are not supported");
-        return NULL;
+    if (type->tp_init == PyBaseObject_Type.tp_init) {
+        int rc = 0;
+        if (args != NULL)
+            rc = PyObject_IsTrue(args);
+        if (rc == 0 && kw != NULL)
+            rc = PyObject_IsTrue(kw);
+        if (rc != 0) {
+            if (rc > 0)
+                PyErr_SetString(PyExc_TypeError,
+                          "Initialization arguments are not supported");
+            return NULL;
+        }
     }
 
     self = (localobject *)type->tp_alloc(type, 0);
diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c
index f53ea3f..530ddc7 100644
--- a/Modules/faulthandler.c
+++ b/Modules/faulthandler.c
@@ -1115,8 +1115,8 @@ faulthandler_env_options(void)
 
         has_key = PyDict_Contains(xoptions, key);
         Py_DECREF(key);
-        if (!has_key)
-            return 0;
+        if (has_key <= 0)
+            return has_key;
     }
 
     module = PyImport_ImportModule("faulthandler");
diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c
index 28b1f68..49db367 100644
--- a/Objects/bytearrayobject.c
+++ b/Objects/bytearrayobject.c
@@ -1011,13 +1011,17 @@ bytearray_richcompare(PyObject *self, PyObject *other, int op)
     Py_buffer self_bytes, other_bytes;
     PyObject *res;
     Py_ssize_t minsize;
-    int cmp;
+    int cmp, rc;
 
     /* Bytes can be compared to anything that supports the (binary)
        buffer API.  Except that a comparison with Unicode is always an
        error, even if the comparison is for equality. */
-    if (PyObject_IsInstance(self, (PyObject*)&PyUnicode_Type) ||
-        PyObject_IsInstance(other, (PyObject*)&PyUnicode_Type)) {
+    rc = PyObject_IsInstance(self, (PyObject*)&PyUnicode_Type);
+    if (!rc)
+        rc = PyObject_IsInstance(other, (PyObject*)&PyUnicode_Type);
+    if (rc < 0)
+        return NULL;
+    if (rc) {
         if (Py_BytesWarningFlag && (op == Py_EQ || op == Py_NE)) {
             if (PyErr_WarnEx(PyExc_BytesWarning,
                             "Comparison between bytearray and string", 1))
diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c
index c576678..6a6e930 100644
--- a/Objects/bytesobject.c
+++ b/Objects/bytesobject.c
@@ -1419,25 +1419,36 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op)
     Py_ssize_t len_a, len_b;
     Py_ssize_t min_len;
     PyObject *result;
+    int rc;
 
     /* Make sure both arguments are strings. */
     if (!(PyBytes_Check(a) && PyBytes_Check(b))) {
         if (Py_BytesWarningFlag && (op == Py_EQ || op == Py_NE)) {
-            if (PyObject_IsInstance((PyObject*)a,
-                                    (PyObject*)&PyUnicode_Type) ||
-                PyObject_IsInstance((PyObject*)b,
-                                    (PyObject*)&PyUnicode_Type)) {
+            rc = PyObject_IsInstance((PyObject*)a,
+                                     (PyObject*)&PyUnicode_Type);
+            if (!rc)
+                rc = PyObject_IsInstance((PyObject*)b,
+                                         (PyObject*)&PyUnicode_Type);
+            if (rc < 0)
+                return NULL;
+            if (rc) {
                 if (PyErr_WarnEx(PyExc_BytesWarning,
-                            "Comparison between bytes and string", 1))
+                                 "Comparison between bytes and string", 1))
                     return NULL;
             }
-            else if (PyObject_IsInstance((PyObject*)a,
-                                    (PyObject*)&PyLong_Type) ||
-                PyObject_IsInstance((PyObject*)b,
-                                    (PyObject*)&PyLong_Type)) {
-                if (PyErr_WarnEx(PyExc_BytesWarning,
-                            "Comparison between bytes and int", 1))
+            else {
+                rc = PyObject_IsInstance((PyObject*)a,
+                                         (PyObject*)&PyLong_Type);
+                if (!rc)
+                    rc = PyObject_IsInstance((PyObject*)b,
+                                             (PyObject*)&PyLong_Type);
+                if (rc < 0)
                     return NULL;
+                if (rc) {
+                    if (PyErr_WarnEx(PyExc_BytesWarning,
+                                     "Comparison between bytes and int", 1))
+                        return NULL;
+                }
             }
         }
         result = Py_NotImplemented;
diff --git a/Objects/rangeobject.c b/Objects/rangeobject.c
index 17fef42..da1d703 100644
--- a/Objects/rangeobject.c
+++ b/Objects/rangeobject.c
@@ -194,8 +194,11 @@ compute_range_length(PyObject *start, PyObject *stop, PyObject *step)
     }
 
     /* if (lo >= hi), return length of 0. */
-    if (PyObject_RichCompareBool(lo, hi, Py_GE) == 1) {
+    cmp_result = PyObject_RichCompareBool(lo, hi, Py_GE);
+    if (cmp_result != 0) {
         Py_XDECREF(step);
+        if (cmp_result < 0)
+            return NULL;
         return PyLong_FromLong(0);
     }
 
diff --git a/Objects/setobject.c b/Objects/setobject.c
index 1805deb..d962c1e 100644
--- a/Objects/setobject.c
+++ b/Objects/setobject.c
@@ -1519,9 +1519,15 @@ set_difference(PySetObject *so, PyObject *other)
     if (PyDict_CheckExact(other)) {
         while (set_next(so, &pos, &entry)) {
             setentry entrycopy;
+            int rv;
             entrycopy.hash = entry->hash;
             entrycopy.key = entry->key;
-            if (!_PyDict_Contains(other, entry->key, entry->hash)) {
+            rv = _PyDict_Contains(other, entry->key, entry->hash);
+            if (rv < 0) {
+                Py_DECREF(result);
+                return NULL;
+            }
+            if (!rv) {
                 if (set_add_entry((PySetObject *)result, &entrycopy)) {
                     Py_DECREF(result);
                     return NULL;
@@ -1757,7 +1763,8 @@ PyDoc_STRVAR(issuperset_doc, "Report whether this set contains another set.");
 static PyObject *
 set_richcompare(PySetObject *v, PyObject *w, int op)
 {
-    PyObject *r1, *r2;
+    PyObject *r1;
+    int r2;
 
     if(!PyAnySet_Check(w))
         Py_RETURN_NOTIMPLEMENTED;
@@ -1775,9 +1782,11 @@ set_richcompare(PySetObject *v, PyObject *w, int op)
         r1 = set_richcompare(v, w, Py_EQ);
         if (r1 == NULL)
             return NULL;
-        r2 = PyBool_FromLong(PyObject_Not(r1));
+        r2 = PyObject_IsTrue(r1);
         Py_DECREF(r1);
-        return r2;
+        if (r2 < 0)
+            return NULL;
+        return PyBool_FromLong(!r2);
     case Py_LE:
         return set_issubset(v, w);
     case Py_GE:
diff --git a/Python/codecs.c b/Python/codecs.c
index 38b0c2c..596bd80 100644
--- a/Python/codecs.c
+++ b/Python/codecs.c
@@ -550,12 +550,13 @@ PyObject * _PyCodec_LookupTextEncoding(const char *encoding,
         } else {
             is_text_codec = PyObject_IsTrue(attr);
             Py_DECREF(attr);
-            if (!is_text_codec) {
+            if (is_text_codec <= 0) {
                 Py_DECREF(codec);
-                PyErr_Format(PyExc_LookupError,
-                             "'%.400s' is not a text encoding; "
-                             "use %s to handle arbitrary codecs",
-                             encoding, alternate_command);
+                if (!is_text_codec)
+                    PyErr_Format(PyExc_LookupError,
+                                 "'%.400s' is not a text encoding; "
+                                 "use %s to handle arbitrary codecs",
+                                 encoding, alternate_command);
                 return NULL;
             }
         }
diff --git a/Python/import.c b/Python/import.c
index 3e27715..44aae80 100644
--- a/Python/import.c
+++ b/Python/import.c
@@ -1366,6 +1366,7 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *given_globals,
     PyObject *globals = NULL;
     PyObject *fromlist = NULL;
     PyInterpreterState *interp = PyThreadState_GET()->interp;
+    int has_from;
 
     /* Make sure to use default values so as to not have
        PyObject_CallMethodObjArgs() truncate the parameter list because of a
@@ -1596,7 +1597,10 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *given_globals,
     }
     /* From now on we don't hold the import lock anymore. */
 
-    if (PyObject_Not(fromlist)) {
+    has_from = PyObject_IsTrue(fromlist);
+    if (has_from < 0)
+        goto error;
+    if (!has_from) {
         if (level == 0 || PyUnicode_GET_LENGTH(name) > 0) {
             PyObject *front = NULL;
             PyObject *partition = NULL;