GH-59313: Do not allow incomplete tuples, or tuples with NULLs.

Doc/whatsnew/3.13.rst

@@ -1872,6 +1872,10 @@ Porting to Python 3.13
   platforms, the ``HAVE_STDDEF_H`` macro is only defined on Windows.
   (Contributed by Victor Stinner in :gh:`108765`.)
 
+* The :c:func:`PyTuple_SET_ITEM` inline function may not be passed ``NULL``.
+  This has always been the documented behavior, but was not enforced for
+  the value until now. :c:var:``Py_None` should be used instead of ``NULL``.
+
 Deprecated
 ----------
 

Include/cpython/tupleobject.h

@@ -30,6 +30,7 @@ static inline Py_ssize_t PyTuple_GET_SIZE(PyObject *op) {
 static inline void
 PyTuple_SET_ITEM(PyObject *op, Py_ssize_t index, PyObject *value) {
     PyTupleObject *tuple = _PyTuple_CAST(op);
+    assert(value != NULL);
     assert(0 <= index);
     assert(index < Py_SIZE(tuple));
     tuple->ob_item[index] = value;

Include/internal/pycore_list.h

@@ -56,6 +56,9 @@ typedef struct {
 
 PyAPI_FUNC(PyObject *)_PyList_FromArraySteal(PyObject *const *src, Py_ssize_t n);
 
+/* Creates tuple from the list, leaving the list empty */
+PyObject *_PyList_AsTupleTakeItems(PyObject *);
+
 #ifdef __cplusplus
 }
 #endif

Lib/test/test_tuple.py

@@ -416,6 +416,29 @@ def test_lexicographic_ordering(self):
         self.assertLess(a, b)
         self.assertLess(b, c)
 
+    def test_bug_59313(self):
+        # Until 3.13, the C-API function PySequence_Tuple
+        # would create incomplete tuples which were visible
+        # to the cycle GC
+        TAG = object()
+        tuples = []
+
+        def referrer_tuples():
+            return [x for x in gc.get_referrers(TAG)
+                if isinstance(x, tuple)]
+
+        def my_iter():
+            nonlocal tuples
+            yield TAG    # 'tag' gets stored in the result tuple
+            tuples += referrer_tuples()
+            for x in range(10):
+                tuples += referrer_tuples()
+                # Prior to 3.13 would raise a SystemError when the tuple needs to be resized
+                yield x
+
+        self.assertEqual(tuple(my_iter()), (TAG, *range(10)))
+        self.assertEqual(tuples, [])
+
 # Notes on testing hash codes.  The primary thing is that Python doesn't
 # care about "random" hash codes.  To the contrary, we like them to be
 # very regular when possible, so that the low-order bits are as evenly

Misc/NEWS.d/next/Core and Builtins/2024-04-11-09-54-49.gh-issue-59313.ByDzOy.rst

@@ -0,0 +1,3 @@
+Make sure that all tuple elements are valid and never NULL. Ensuring that
+tuples are always valid reduces the risk of cycle GC bugs and prevents
+``gc.get_referrers`` from returning invalid tuples.

Modules/_testbuffer.c

@@ -333,10 +333,10 @@ pack_from_list(PyObject *obj, PyObject *items, PyObject *format,
 
     offset = NULL;
     for (i = 0; i < nitems; i++) {
-        /* Loop invariant: args[j] are borrowed references or NULL. */
+        /* Loop invariant: args[j] are borrowed references or None. */
         PyTuple_SET_ITEM(args, 0, obj);
         for (j = 1; j < 2+nmemb; j++)
-            PyTuple_SET_ITEM(args, j, NULL);
+            PyTuple_SET_ITEM(args, j, Py_None);
 
         Py_XDECREF(offset);
         offset = PyLong_FromSsize_t(i*itemsize);

Modules/itertoolsmodule.c

@@ -4473,7 +4473,7 @@ zip_longest_next(ziplongestobject *lz)
         Py_INCREF(result);
         for (i=0 ; i < tuplesize ; i++) {
             it = PyTuple_GET_ITEM(lz->ittuple, i);
-            if (it == NULL) {
+            if (it == Py_None) {
                 item = Py_NewRef(lz->fillvalue);
             } else {
                 item = PyIter_Next(it);
@@ -4485,7 +4485,7 @@ zip_longest_next(ziplongestobject *lz)
                         return NULL;
                     } else {
                         item = Py_NewRef(lz->fillvalue);
-                        PyTuple_SET_ITEM(lz->ittuple, i, NULL);
+                        PyTuple_SET_ITEM(lz->ittuple, i, Py_None);
                         Py_DECREF(it);
                     }
                 }
@@ -4505,7 +4505,7 @@ zip_longest_next(ziplongestobject *lz)
             return NULL;
         for (i=0 ; i < tuplesize ; i++) {
             it = PyTuple_GET_ITEM(lz->ittuple, i);
-            if (it == NULL) {
+            if (it == Py_None) {
                 item = Py_NewRef(lz->fillvalue);
             } else {
                 item = PyIter_Next(it);
@@ -4517,7 +4517,7 @@ zip_longest_next(ziplongestobject *lz)
                         return NULL;
                     } else {
                         item = Py_NewRef(lz->fillvalue);
-                        PyTuple_SET_ITEM(lz->ittuple, i, NULL);
+                        PyTuple_SET_ITEM(lz->ittuple, i, Py_None);
                         Py_DECREF(it);
                     }
                 }
@@ -4542,7 +4542,7 @@ zip_longest_reduce(ziplongestobject *lz, PyObject *Py_UNUSED(ignored))
         return NULL;
     for (i=0; i<PyTuple_GET_SIZE(lz->ittuple); i++) {
         PyObject *elem = PyTuple_GET_ITEM(lz->ittuple, i);
-        if (elem == NULL) {
+        if (elem == Py_None) {
             elem = PyTuple_New(0);
             if (elem == NULL) {
                 Py_DECREF(args);

Objects/abstract.c

@@ -2039,9 +2039,7 @@
 PySequence_Tuple(PyObject *v)
 {
     PyObject *it;  /* iter(v) */
     Py_ssize_t n;             /* guess for result tuple size */
-    PyObject *result = NULL;
-    Py_ssize_t j;
 
     if (v == NULL) {
         return null_error();
@@ -2063,57 +2061,31 @@
     if (it == NULL)
         return NULL;
 
-    /* Guess result size and allocate space. */
-    n = PyObject_LengthHint(v, 10);
-    if (n == -1)
-        goto Fail;
-    result = PyTuple_New(n);
-    if (result == NULL)
+    PyObject *temp = PyList_New(0);
+    if (temp == NULL)
         goto Fail;
 
-    /* Fill the tuple. */
-    for (j = 0; ; ++j) {
+    /* Fill the temporary list. */
+    for (;;) {
         PyObject *item = PyIter_Next(it);
         if (item == NULL) {
-            if (PyErr_Occurred())
+            if (PyErr_Occurred()) {
+                Py_DECREF(temp);
                 goto Fail;
+            }
             break;
         }
-        if (j >= n) {
-            size_t newn = (size_t)n;
-            /* The over-allocation strategy can grow a bit faster
-               than for lists because unlike lists the
-               over-allocation isn't permanent -- we reclaim
-               the excess before the end of this routine.
-               So, grow by ten and then add 25%.
-            */
-            newn += 10u;
-            newn += newn >> 2;
-            if (newn > PY_SSIZE_T_MAX) {
-                /* Check for overflow */
-                PyErr_NoMemory();
-                Py_DECREF(item);
-                goto Fail;
-            }
-            n = (Py_ssize_t)newn;
-            if (_PyTuple_Resize(&result, n) != 0) {
-                Py_DECREF(item);
-                goto Fail;
-            }
+        if (_PyList_AppendTakeRef((PyListObject *)temp, item)) {
+            Py_DECREF(temp);
+            goto Fail;
         }
-        PyTuple_SET_ITEM(result, j, item);
     }
-
-    /* Cut tuple back if guess was too large. */
-    if (j < n &&
-        _PyTuple_Resize(&result, j) != 0)
-        goto Fail;
-
     Py_DECREF(it);
+    PyObject *result = _PyList_AsTupleTakeItems(temp);
+    Py_DECREF(temp);
     return result;
 
 Fail:
-    Py_XDECREF(result);
     Py_DECREF(it);
     return NULL;
 }

Objects/listobject.c

@@ -3174,6 +3174,26 @@ PyList_AsTuple(PyObject *v)
     return ret;
 }
 
+PyObject *
+_PyList_AsTupleTakeItems(PyObject *v)
+{
+    if (v == NULL || !PyList_Check(v)) {
+        PyErr_BadInternalCall();
+        return NULL;
+    }
+    PyObject *ret;
+    PyListObject *self = (PyListObject *)v;
+    if (Py_SIZE(v) == 0) {
+        return PyTuple_New(0);
+    }
+    Py_BEGIN_CRITICAL_SECTION(self);
+    Py_ssize_t size = Py_SIZE(v);
+    Py_SET_SIZE(v, 0);
+    ret = _PyTuple_FromArraySteal(self->ob_item, size);
+    Py_END_CRITICAL_SECTION();
+    return ret;
+}
+
 PyObject *
 _PyList_FromArraySteal(PyObject *const *src, Py_ssize_t n)
 {

Objects/tupleobject.c

@@ -34,14 +34,7 @@ static inline int maybe_freelist_push(PyTupleObject *);
 static PyTupleObject *
 tuple_alloc(Py_ssize_t size)
 {
-    if (size < 0) {
-        PyErr_BadInternalCall();
-        return NULL;
-    }
-#ifdef Py_DEBUG
-    assert(size != 0);    // The empty tuple is statically allocated.
-#endif
-
+    assert(size > 0);
     PyTupleObject *op = maybe_freelist_pop(size);
     if (op == NULL) {
         /* Check for overflow */
@@ -78,7 +71,7 @@ PyTuple_New(Py_ssize_t size)
         return NULL;
     }
     for (Py_ssize_t i = 0; i < size; i++) {
-        op->ob_item[i] = NULL;
+        op->ob_item[i] = Py_None;
     }
     _PyObject_GC_TRACK(op);
     return (PyObject *) op;
@@ -205,7 +198,8 @@ tupledealloc(PyTupleObject *op)
 
     Py_ssize_t i = Py_SIZE(op);
     while (--i >= 0) {
-        Py_XDECREF(op->ob_item[i]);
+        assert(op->ob_item[i] != NULL);
+        Py_DECREF(op->ob_item[i]);
     }
     // This will abort on the empty singleton (if there is one).
     if (!maybe_freelist_push(op)) {

Objects/weakrefobject.c

@@ -96,7 +96,7 @@ clear_weakref_lock_held(PyWeakReference *self, PyObject **callback)
         self->wr_prev = NULL;
         self->wr_next = NULL;
     }
-    if (callback != NULL) {
+    if (callback != NULL && self->wr_callback != NULL) {
         *callback = self->wr_callback;
         self->wr_callback = NULL;
     }
@@ -999,7 +999,7 @@ PyObject_ClearWeakRefs(PyObject *object)
     for (int done = 0; !done;) {
         LOCK_WEAKREFS(object);
         if (*list != NULL && is_basic_ref_or_proxy(*list)) {
-            PyObject *callback;
+            PyObject *callback = NULL;
             clear_weakref_lock_held(*list, &callback);
             assert(callback == NULL);
         }
@@ -1022,7 +1022,7 @@ PyObject_ClearWeakRefs(PyObject *object)
 
     Py_ssize_t num_items = 0;
     for (int done = 0; !done;) {
-        PyObject *callback = NULL;
+        PyObject *callback = Py_None;
         LOCK_WEAKREFS(object);
         PyWeakReference *cur = *list;
         if (cur != NULL) {
@@ -1032,18 +1032,18 @@ PyObject_ClearWeakRefs(PyObject *object)
                 PyTuple_SET_ITEM(tuple, num_items, (PyObject *) cur);
                 PyTuple_SET_ITEM(tuple, num_items + 1, callback);
                 num_items += 2;
-                callback = NULL;
+                callback = Py_None;
             }
         }
         done = (*list == NULL);
         UNLOCK_WEAKREFS(object);
 
-        Py_XDECREF(callback);
+        Py_DECREF(callback);
     }
 
     for (Py_ssize_t i = 0; i < num_items; i += 2) {
         PyObject *callback = PyTuple_GET_ITEM(tuple, i + 1);
-        if (callback != NULL) {
+        if (callback != Py_None) {
             PyObject *weakref = PyTuple_GET_ITEM(tuple, i);
             handle_callback((PyWeakReference *)weakref, callback);
         }