bpo-29838: Add asserts for checking results of sq_length and mq_length slots. (python#700)

serhiy-storchaka · web-flow · commit 813f943c592c · 2017-04-16T09:21:44.000+03:00
Negative result should be returned only when an error is set.
diff --git a/Objects/abstract.c b/Objects/abstract.c
@@ -52,8 +52,11 @@ PyObject_Size(PyObject *o)
     }
 
     m = o->ob_type->tp_as_sequence;
-    if (m && m->sq_length)
-        return m->sq_length(o);
+    if (m && m->sq_length) {
+        Py_ssize_t len = m->sq_length(o);
+        assert(len >= 0 || PyErr_Occurred());
+        return len;
+    }
 
     return PyMapping_Size(o);
 }
@@ -86,7 +89,8 @@ PyObject_LengthHint(PyObject *o, Py_ssize_t defaultvalue)
     _Py_IDENTIFIER(__length_hint__);
     if (_PyObject_HasLen(o)) {
         res = PyObject_Length(o);
-        if (res < 0 && PyErr_Occurred()) {
+        if (res < 0) {
+            assert(PyErr_Occurred());
             if (!PyErr_ExceptionMatches(PyExc_TypeError)) {
                 return -1;
             }
@@ -1483,8 +1487,11 @@ PySequence_Size(PyObject *s)
     }
 
     m = s->ob_type->tp_as_sequence;
-    if (m && m->sq_length)
-        return m->sq_length(s);
+    if (m && m->sq_length) {
+        Py_ssize_t len = m->sq_length(s);
+        assert(len >= 0 || PyErr_Occurred());
+        return len;
+    }
 
     type_error("object of type '%.200s' has no len()", s);
     return -1;
@@ -1673,8 +1680,10 @@ PySequence_SetItem(PyObject *s, Py_ssize_t i, PyObject *o)
         if (i < 0) {
             if (m->sq_length) {
                 Py_ssize_t l = (*m->sq_length)(s);
-                if (l < 0)
+                if (l < 0) {
+                    assert(PyErr_Occurred());
                     return -1;
+                }
                 i += l;
             }
         }
@@ -1700,8 +1709,10 @@ PySequence_DelItem(PyObject *s, Py_ssize_t i)
         if (i < 0) {
             if (m->sq_length) {
                 Py_ssize_t l = (*m->sq_length)(s);
-                if (l < 0)
+                if (l < 0) {
+                    assert(PyErr_Occurred());
                     return -1;
+                }
                 i += l;
             }
         }
@@ -2038,8 +2049,11 @@ PyMapping_Size(PyObject *o)
     }
 
     m = o->ob_type->tp_as_mapping;
-    if (m && m->mp_length)
-        return m->mp_length(o);
+    if (m && m->mp_length) {
+        Py_ssize_t len = m->mp_length(o);
+        assert(len >= 0 || PyErr_Occurred());
+        return len;
+    }
 
     type_error("object of type '%.200s' has no len()", o);
     return -1;
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
@@ -5427,8 +5427,10 @@ getindex(PyObject *self, PyObject *arg)
         PySequenceMethods *sq = Py_TYPE(self)->tp_as_sequence;
         if (sq && sq->sq_length) {
             Py_ssize_t n = (*sq->sq_length)(self);
-            if (n < 0)
+            if (n < 0) {
+                assert(PyErr_Occurred());
                 return -1;
+            }
             i += n;
         }
     }
diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
@@ -1479,8 +1479,10 @@ builtin_len(PyObject *module, PyObject *obj)
     Py_ssize_t res;
 
     res = PyObject_Size(obj);
-    if (res < 0 && PyErr_Occurred())
+    if (res < 0) {
+        assert(PyErr_Occurred());
         return NULL;
+    }
     return PyLong_FromSsize_t(res);
 }
 

Original file line number	Diff line number	Diff line change
`@@ -5427,8 +5427,10 @@ getindex(PyObject self, PyObject arg)`
`5427`	`5427`	`PySequenceMethods *sq = Py_TYPE(self)->tp_as_sequence;`
`5428`	`5428`	`if (sq && sq->sq_length) {`
`5429`	`5429`	`Py_ssize_t n = (*sq->sq_length)(self);`
`5430`		`- if (n < 0)`
	`5430`	`+ if (n < 0) {`
	`5431`	`+ assert(PyErr_Occurred());`
`5431`	`5432`	`return -1;`
	`5433`	`+ }`
`5432`	`5434`	`i += n;`
`5433`	`5435`	`}`
`5434`	`5436`	`}`