Skip to content

Commit 1405607

Browse files
authored
gh-161226: Fail if the hash is cached in _PyTuple_Resize() (#161293)
_PyTuple_Resize() now fails with SystemError if the hash value was already computed. Add compute_hash optional parameter to _testcapi._tuple_resize().
1 parent 356e6e2 commit 1405607

5 files changed

Lines changed: 118 additions & 35 deletions

File tree

‎Doc/c-api/tuple.rst‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -133,16 +133,36 @@ Tuple Objects
133133
134134
.. c:function:: int _PyTuple_Resize(PyObject **p, Py_ssize_t newsize)
135135
136-
Can be used to resize a tuple. *newsize* will be the new length of the tuple.
137-
Because tuples are *supposed* to be immutable, this should only be used if there
138-
is only one reference to the object. Do *not* use this if the tuple may already
139-
be known to some other part of the code. The tuple will always grow or shrink
140-
at the end. Think of this as destroying the old tuple and creating a new one,
141-
only more efficiently. Returns ``0`` on success. Client code should never
142-
assume that the resulting value of ``*p`` will be the same as before calling
143-
this function. If the object referenced by ``*p`` is replaced, the original
144-
``*p`` is destroyed. On failure, returns ``-1`` and sets ``*p`` to ``NULL``, and
145-
raises :exc:`MemoryError` or :exc:`SystemError`.
136+
Resize a tuple.
137+
*newsize* will be the new length of the tuple.
138+
139+
Because tuples are *supposed* to be immutable, this should only be used if
140+
there is only one reference to the object.
141+
Do *not* use this if the tuple may already be known to some other part of
142+
the code.
143+
The tuple hash value must not be computed yet.
144+
145+
The tuple will always grow or shrink at the end.
146+
Think of this as destroying the old tuple and creating a new one, only more
147+
efficiently.
148+
149+
Returns ``0`` on success.
150+
Client code should never assume that the resulting value of ``*p`` will be
151+
the same as before calling this function.
152+
If the object referenced by ``*p`` is replaced, the original ``*p`` is
153+
destroyed.
154+
155+
On failure, returns ``-1`` and sets ``*p`` to ``NULL``, and raises
156+
:exc:`MemoryError` or :exc:`SystemError`.
157+
158+
While tuple objects are usually immutable in Python, this special C API
159+
allows mutating a tuple object in-place.
160+
The returned tuple object can still be mutated using
161+
:c:func:`PyTuple_SetItem`; except if *newsize* is zero in which case it
162+
returns the immutable empty tuple.
163+
164+
.. versionchanged:: next
165+
The function now fails if the tuple hash value was already computed.
146166
147167
148168
.. _struct-sequence-objects:

‎Lib/test/test_capi/test_tuple.py‎

Lines changed: 60 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import unittest
21
import gc
3-
from sys import getrefcount
2+
import sys
3+
import unittest
44
from test.support import import_helper
55

66
_testcapi = import_helper.import_module('_testcapi')
@@ -131,9 +131,9 @@ def check_tuple_from_pair(self, from_pair):
131131
class Temp:
132132
pass
133133
temp = Temp()
134-
temp_rc = getrefcount(temp)
134+
temp_rc = sys.getrefcount(temp)
135135
self.assertEqual(from_pair(temp, temp), (temp, temp))
136-
self.assertEqual(getrefcount(temp), temp_rc)
136+
self.assertEqual(sys.getrefcount(temp), temp_rc)
137137

138138
self._not_tracked(from_pair(1, 2))
139139
self._not_tracked(from_pair(None, None))
@@ -299,30 +299,71 @@ def test_tuple_set_item(self):
299299

300300
def test__tuple_resize(self):
301301
# Test _PyTuple_Resize()
302-
resize = _testcapi._tuple_resize
302+
_resize = _testcapi._tuple_resize
303303
checknull = _testcapi._check_tuple_item_is_NULL
304304

305-
a = ()
306-
b = resize(a, 0, False)
307-
self.assertEqual(len(a), 0)
308-
self.assertEqual(len(b), 0)
309-
b = resize(a, 2, False)
310-
self.assertEqual(len(a), 0)
311-
self.assertEqual(len(b), 2)
312-
self.assertTrue(checknull(b, 0))
313-
self.assertTrue(checknull(b, 1))
314-
315-
a = ([1], [2], [3])
316-
b = resize(a, 3)
305+
def assert_is_fresh_copy(result, refcnt, is_new_obj):
306+
self.assertEqual(refcnt, 1)
307+
self.assertTrue(is_new_obj)
308+
self.assertFalse(sys._is_immortal(result))
309+
310+
def resize(tup, new_size, new=True, compute_hash=False):
311+
if isinstance(tup, tuple):
312+
old_size = len(tup)
313+
else:
314+
old_size = 0
315+
result, refcnt, is_new_obj = _resize(tup, new_size, new,
316+
compute_hash=compute_hash)
317+
if new_size == old_size:
318+
# Return the same object unchanged
319+
self.assertFalse(is_new_obj)
320+
elif old_size == 0:
321+
assert_is_fresh_copy(result, refcnt, is_new_obj)
322+
elif new_size == 0:
323+
# check that the result is the empty bytes string singleton
324+
self.assertEqual(result, ())
325+
self.assertTrue(sys._is_immortal(result))
326+
self.assertTrue(is_new_obj)
327+
elif not new:
328+
# Make sure that the result is a fresh object.
329+
assert_is_fresh_copy(result, refcnt, is_new_obj)
330+
else:
331+
# An in-place resize can return the same memory address, or
332+
# not. 'is_new_obj' cannot be tested.
333+
self.assertEqual(refcnt, 1)
334+
self.assertFalse(sys._is_immortal(result))
335+
336+
return result
337+
338+
# Resizing the empty tuple singleton always create a new tuple
339+
for compute_hash in (False, True):
340+
with self.subTest(compute_hash=compute_hash):
341+
a = ()
342+
b = resize(a, 0, False, compute_hash)
343+
self.assertEqual(len(a), 0)
344+
self.assertEqual(len(b), 0)
345+
b = resize(a, 2, False, compute_hash)
346+
self.assertEqual(len(a), 0)
347+
self.assertEqual(len(b), 2)
348+
self.assertTrue(checknull(b, 0))
349+
self.assertTrue(checknull(b, 1))
350+
351+
a = tuple(object() for _ in range(3))
352+
b = resize(a, 3, True)
317353
self.assertEqual(b, a)
318-
b = resize(a, 2)
354+
b = resize(a, 2, True)
319355
self.assertEqual(b, a[:2])
320-
b = resize(a, 5)
356+
b = resize(a, 5, True)
321357
self.assertEqual(len(b), 5)
322358
self.assertEqual(b[:3], a)
323359
self.assertTrue(checknull(b, 3))
324360
self.assertTrue(checknull(b, 4))
325361

362+
# gh-161226: Disallow resizing a tuple if the hash value is cached
363+
a = tuple(object() for _ in range(3))
364+
with self.assertRaises(SystemError):
365+
resize(a, 3, True, compute_hash=True)
366+
326367
a = ()
327368
self.assertRaises(MemoryError, resize, a, PY_SSIZE_T_MAX)
328369
self.assertRaises(SystemError, resize, a, -1)
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:c:func:`_PyTuple_Resize` now fails with :exc:`SystemError` if the hash value
2+
was already computed. Patch by Victor Stinner.

‎Modules/_testcapi/tuple.c‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,17 @@ tuple_set_item(PyObject *Py_UNUSED(module), PyObject *args)
6666
}
6767

6868
static PyObject *
69-
_tuple_resize(PyObject *Py_UNUSED(module), PyObject *args)
69+
_tuple_resize(PyObject *Py_UNUSED(module), PyObject *args, PyObject *kwargs)
7070
{
71+
static char *kwlist[] = {"obj", "newsize", "new", "compute_hash", NULL};
7172
PyObject *tup;
7273
Py_ssize_t newsize;
7374
int new = 1;
74-
if (!PyArg_ParseTuple(args, "On|p", &tup, &newsize, &new)) {
75+
int compute_hash = 0;
76+
77+
if (!PyArg_ParseTupleAndKeywords(args, kwargs,
78+
"On|pi", kwlist,
79+
&tup, &newsize, &new, &compute_hash)) {
7580
return NULL;
7681
}
7782
if (new) {
@@ -84,12 +89,24 @@ _tuple_resize(PyObject *Py_UNUSED(module), PyObject *args)
8489
NULLABLE(tup);
8590
Py_XINCREF(tup);
8691
}
92+
93+
if (compute_hash) {
94+
if (PyObject_Hash(tup) == -1) {
95+
Py_DECREF(tup);
96+
return NULL;
97+
}
98+
}
99+
100+
PyObject *old_tup = tup;
87101
int r = _PyTuple_Resize(&tup, newsize);
88102
if (r == -1) {
89103
assert(tup == NULL);
90104
return NULL;
91105
}
92-
return tup;
106+
assert(tup != NULL);
107+
108+
Py_ssize_t refcnt = Py_REFCNT(tup);
109+
return Py_BuildValue("Nnp", tup, refcnt, tup != old_tup);
93110
}
94111

95112
static PyObject *
@@ -135,7 +152,7 @@ static PyMethodDef test_methods[] = {
135152
{"tuple_get_size", tuple_get_size, METH_O},
136153
{"tuple_get_item", tuple_get_item, METH_VARARGS},
137154
{"tuple_set_item", tuple_set_item, METH_VARARGS},
138-
{"_tuple_resize", _tuple_resize, METH_VARARGS},
155+
{"_tuple_resize", _PyCFunction_CAST(_tuple_resize), METH_VARARGS | METH_KEYWORDS},
139156
{"_check_tuple_item_is_NULL", _check_tuple_item_is_NULL, METH_VARARGS},
140157
{"tuple_fromarray", tuple_fromarray, METH_VARARGS},
141158
{NULL},

‎Objects/tupleobject.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,8 +1031,11 @@ _PyTuple_Resize(PyObject **pv, Py_ssize_t newsize)
10311031
Py_ssize_t oldsize;
10321032

10331033
v = (PyTupleObject *) *pv;
1034-
if (v == NULL || !Py_IS_TYPE(v, &PyTuple_Type) || newsize < 0 ||
1035-
(Py_SIZE(v) != 0 && !_PyObject_IsUniquelyReferenced(*pv))) {
1034+
if (v == NULL || !Py_IS_TYPE(v, &PyTuple_Type) || newsize < 0
1035+
|| (Py_SIZE(v) != 0 && !_PyObject_IsUniquelyReferenced(*pv))
1036+
// gh-161226: Disallow resizing a tuple if the hash value is cached
1037+
|| (Py_SIZE(v) != 0 && v->ob_hash != -1))
1038+
{
10361039
*pv = 0;
10371040
Py_XDECREF(v);
10381041
PyErr_BadInternalCall();

0 commit comments

Comments
 (0)