python · markshannon · Apr 10, 2024 · Apr 10, 2024 · Apr 11, 2024 · Apr 11, 2024
diff --git a/Doc/c-api/tuple.rst b/Doc/c-api/tuple.rst
@@ -86,7 +86,7 @@ Tuple Objects
 
    Insert a reference to object *o* at position *pos* of the tuple pointed to by
    *p*.  Return ``0`` on success.  If *pos* is out of bounds, return ``-1``
-   and set an :exc:`IndexError` exception.
+   and set an :exc:`IndexError` exception. Both ``p`` and ``o`` must be non-``NULL``.
 
    .. note::
 
@@ -97,7 +97,7 @@ Tuple Objects
 .. c:function:: void PyTuple_SET_ITEM(PyObject *p, Py_ssize_t pos, PyObject *o)
 
    Like :c:func:`PyTuple_SetItem`, but does no error checking, and should *only* be
-   used to fill in brand new tuples.
+   used to fill in brand new tuples. Both ``p`` and ``o`` must be non-``NULL``.
 
    Bounds checking is performed as an assertion if Python is built in
    :ref:`debug mode <debug-build>` or :option:`with assertions <--with-assertions>`.

diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst
@@ -827,6 +827,10 @@ Porting to Python 3.14
   implementation details.
   (Contributed by Victor Stinner in :gh:`120600` and :gh:`124127`.)
 
+* 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
 ----------

diff --git a/Include/cpython/tupleobject.h b/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;

diff --git a/Include/internal/pycore_list.h b/Include/internal/pycore_list.h
@@ -61,6 +61,9 @@ union _PyStackRef;
 PyAPI_FUNC(PyObject *)_PyList_FromStackRefSteal(const union _PyStackRef *src, Py_ssize_t n);
 
 
+/* Creates tuple from the list, leaving the list empty */
+PyObject *_PyList_AsTupleTakeItems(PyObject *);
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/Lib/test/test_capi/test_tuple.py b/Lib/test/test_capi/test_tuple.py
@@ -57,7 +57,7 @@ def test_tuple_new(self):
         self.assertIs(type(tup2), tuple)
         self.assertEqual(size(tup2), 1)
         self.assertIsNot(tup2, tup1)
-        self.assertTrue(checknull(tup2, 0))
+        self.assertIs(tup2[0], None)
 
         self.assertRaises(SystemError, tuple_new, -1)
         self.assertRaises(SystemError, tuple_new, PY_SSIZE_T_MIN)
@@ -71,8 +71,8 @@ def test_tuple_pack(self):
         self.assertEqual(pack(1, [1]), ([1],))
         self.assertEqual(pack(2, [1], [2]), ([1], [2]))
 
-        self.assertRaises(SystemError, pack, PY_SSIZE_T_MIN)
-        self.assertRaises(SystemError, pack, -1)
+        self.assertRaises(ValueError, pack, PY_SSIZE_T_MIN)
+        self.assertRaises(ValueError, pack, -1)
         self.assertRaises(MemoryError, pack, PY_SSIZE_T_MAX)
 
         # CRASHES pack(1, NULL)
@@ -183,9 +183,6 @@ def test_tuple_setitem(self):
         self.assertEqual(setitem(tup, 0, []), ([], [2]))
         self.assertEqual(setitem(tup, 1, []), ([1], []))
 
-        tup2 = setitem(tup, 1, NULL)
-        self.assertTrue(checknull(tup2, 1))
-
         tup2 = TupleSubclass(([1], [2]))
         self.assertRaises(SystemError, setitem, tup2, 0, [])
 
@@ -196,8 +193,6 @@ def test_tuple_setitem(self):
         self.assertRaises(SystemError, setitem, [1], 0, [])
         self.assertRaises(SystemError, setitem, 42, 0, [])
 
-        # CRASHES setitem(NULL, 0, [])
-
     def test_tuple_set_item(self):
         # Test PyTuple_SET_ITEM()
         set_item = _testcapi.tuple_set_item
@@ -207,9 +202,6 @@ def test_tuple_set_item(self):
         self.assertEqual(set_item(tup, 0, []), ([], [2]))
         self.assertEqual(set_item(tup, 1, []), ([1], []))
 
-        tup2 = set_item(tup, 1, NULL)
-        self.assertTrue(checknull(tup2, 1))
-
         tup2 = TupleSubclass(([1], [2]))
         self.assertIs(set_item(tup2, 0, []), tup2)
         self.assertEqual(tup2, ([], [2]))
@@ -231,8 +223,8 @@ def test__tuple_resize(self):
         b = resize(a, 2, False)
         self.assertEqual(len(a), 0)
         self.assertEqual(len(b), 2)
-        self.assertTrue(checknull(b, 0))
-        self.assertTrue(checknull(b, 1))
+        self.assertIs(b[0], None)
+        self.assertIs(b[1], None)
 
         a = ([1], [2], [3])
         b = resize(a, 3)
@@ -242,8 +234,8 @@ def test__tuple_resize(self):
         b = resize(a, 5)
         self.assertEqual(len(b), 5)
         self.assertEqual(b[:3], a)
-        self.assertTrue(checknull(b, 3))
-        self.assertTrue(checknull(b, 4))
+        self.assertIs(b[3], None)
+        self.assertIs(b[4], None)
 
         a = ()
         self.assertRaises(MemoryError, resize, a, PY_SSIZE_T_MAX)

diff --git a/Lib/test/test_tuple.py b/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

diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-11-08-14-24-51.gh-issue-59313.ByDzOy.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-08-14-24-51.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.
diff --git a/Modules/_testbuffer.c b/Modules/_testbuffer.c
@@ -335,10 +335,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. */
         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);

diff --git a/Modules/_testcapi/tuple.c b/Modules/_testcapi/tuple.c
@@ -43,7 +43,6 @@ tuple_set_item(PyObject *Py_UNUSED(module), PyObject *args)
     if (!PyArg_ParseTuple(args, "OnO", &obj, &i, &value)) {
         return NULL;
     }
-    NULLABLE(value);
     if (PyTuple_CheckExact(obj)) {
         newtuple = tuple_copy(obj);
         if (!newtuple) {

@@ -3696,7 +3696,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);
@@ -3708,7 +3708,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);
                     }
                 }
@@ -3728,7 +3728,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);
@@ -3740,7 +3740,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);
                     }
                 }

diff --git a/Objects/abstract.c b/Objects/abstract.c
@@ -1992,11 +1992,6 @@ PySequence_DelSlice(PyObject *s, Py_ssize_t i1, Py_ssize_t i2)
 PyObject *
 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();
     }
@@ -2013,61 +2008,36 @@ PySequence_Tuple(PyObject *v)
         return PyList_AsTuple(v);
 
     /* Get iterator. */
-    it = PyObject_GetIter(v);
+    PyObject *it = PyObject_GetIter(v);
     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)
-        goto Fail;
+    PyObject *temp = PyList_New(0);
+    if (temp == NULL) {
+        Py_DECREF(it);
+        return NULL;
+    }
 
-    /* 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()) {
                 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)) {
+            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(temp);
     Py_DECREF(it);
     return NULL;
 }

diff --git a/Objects/listobject.c b/Objects/listobject.c
@@ -3138,6 +3138,30 @@ PyList_AsTuple(PyObject *v)
     return ret;
 }
 
+PyObject *
+_PyList_AsTupleTakeItems(PyObject *v)
+{
+    assert(PyList_Check(v));
+    PyObject *ret;
+    PyListObject *self = (PyListObject *)v;
+    if (Py_SIZE(v) == 0) {
+        return PyTuple_New(0);
+    }
+    Py_ssize_t size;
+    PyObject **items;
+    Py_BEGIN_CRITICAL_SECTION(self);
+    size = PyList_GET_SIZE(v);
+    items = self->ob_item;
+    Py_SET_SIZE(v, 0);
+    self->ob_item = NULL;
+    Py_END_CRITICAL_SECTION();
+    ret = _PyTuple_FromArraySteal(items, size);
+    if (size) {
+        free_list_items(items, false);
+    }
+    return ret;
+}
+
 PyObject *
 _PyList_FromStackRefSteal(const _PyStackRef *src, Py_ssize_t n)
 {