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

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>`.

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
 ----------

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

@@ -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

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)

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-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.

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);

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){

Modules/itertoolsmodule.c

@@ -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);
  }
  }

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;
 }

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)
 {