https://github.com/python/cpython/commit/6e7738bb5e6a294d888f8af6f409f52508194249
commit: 6e7738bb5e6a294d888f8af6f409f52508194249
branch: main
author: Victor Stinner <[email protected]>
committer: vstinner <[email protected]>
date: 2026-10-03T23:35:46+02:00
summary:
gh-158585: Fix _PyBytes_Resize() if hash value is already computed (#158589)
_PyBytes_Resize() now creates a new bytes object if the hash value
was already computed.
bytes_resize_inplace() no longer sets the hash value to -1, since
this function must not be called if the hash value was already
computed.
Add tests on PyUnicode_Resize().
files:
M Lib/test/test_capi/test_bytes.py
M Lib/test/test_capi/test_unicode.py
M Modules/_testcapi/bytes.c
M Modules/_testlimitedcapi/unicode.c
M Objects/bytesobject.c
diff --git a/Lib/test/test_capi/test_bytes.py b/Lib/test/test_capi/test_bytes.py
index 910f2bedac0bcb4..ea5d3a3d59b6fd4 100644
--- a/Lib/test/test_capi/test_bytes.py
+++ b/Lib/test/test_capi/test_bytes.py
@@ -239,39 +239,60 @@ def test_resize(self):
"""Test _PyBytes_Resize()"""
_resize = _testcapi.bytes_resize
- def resize(obj, size, new):
- result = _resize(obj, size, new)
- if 1 <= len(result):
- if new or size != len(obj):
- # gh-156995: Make sure that the result is a fresh object.
- # Previously, _PyBytes_Resize(&obj, 1) returned a singleton
- # if _PyObject_IsUniquelyReferenced() is false.
- self.assertEqual(sys.getrefcount(result), 1)
- self.assertFalse(sys._is_immortal(result))
- else:
+ def assert_is_fresh_copy(result, refcnt, is_new_obj):
+ self.assertEqual(refcnt, 1)
+ self.assertTrue(is_new_obj)
+ self.assertFalse(sys._is_immortal(result))
+
+ def resize(obj, size, new, compute_hash=False):
+ old_size = len(obj)
+ result, refcnt, is_new_obj = _resize(obj, size, new,
+ compute_hash=compute_hash)
+
+ if size == old_size:
+ # Return the same object unchanged
+ self.assertFalse(is_new_obj)
+ elif old_size == 0:
+ assert_is_fresh_copy(result, refcnt, is_new_obj)
+ elif size == 0:
# check that the result is the empty bytes string singleton
+ self.assertEqual(result, b'')
self.assertTrue(sys._is_immortal(result))
+ self.assertTrue(is_new_obj)
+ elif (not new) or compute_hash:
+ # gh-156995: Make sure that the result is a fresh object.
+ # Previously, _PyBytes_Resize(&obj, 1) returned a singleton
+ # if _PyObject_IsUniquelyReferenced() is false.
+ assert_is_fresh_copy(result, refcnt, is_new_obj)
+ else:
+ # An in-place resize can return the same memory address, or
+ # not. 'is_new_obj' cannot be tested.
+ self.assertEqual(refcnt, 1)
+ self.assertFalse(sys._is_immortal(result))
+
return result
for new in True, False:
- with self.subTest(new=new):
- self.assertEqual(resize(b'abc', 0, new), b'')
- self.assertEqual(resize(b'abc', 1, new), b'a')
- self.assertEqual(resize(b'abc', 2, new), b'ab')
- self.assertEqual(resize(b'abc', 3, new), b'abc')
- b = resize(b'abc', 4, new)
- self.assertEqual(len(b), 4)
- self.assertEqual(b[:3], b'abc')
-
- self.assertEqual(resize(b'a', 0, new), b'')
- self.assertEqual(resize(b'a', 1, new), b'a')
- b = resize(b'a', 2, new)
- self.assertEqual(len(b), 2)
- self.assertEqual(b[:1], b'a')
-
- self.assertEqual(resize(b'', 0, new), b'')
- self.assertEqual(len(resize(b'', 1, new)), 1)
- self.assertEqual(len(resize(b'', 2, new)), 2)
+ for compute_hash in True, False:
+ with self.subTest(new=new, compute_hash=compute_hash):
+ self.assertEqual(resize(b'abc', 0, new, compute_hash), b'')
+ self.assertEqual(resize(b'abc', 1, new, compute_hash),
b'a')
+ self.assertEqual(resize(b'abc', 2, new, compute_hash),
b'ab')
+ self.assertEqual(resize(b'abc', 3, new, compute_hash),
b'abc')
+
+ b = resize(b'abc', 4, new, compute_hash)
+ self.assertEqual(len(b), 4)
+ self.assertEqual(b[:3], b'abc')
+
+ self.assertEqual(resize(b'a', 0, new, compute_hash), b'')
+ self.assertEqual(resize(b'a', 1, new, compute_hash), b'a')
+ b = resize(b'a', 2, new, compute_hash)
+ self.assertEqual(len(b), 2)
+ self.assertEqual(b[:1], b'a')
+
+ self.assertEqual(resize(b'', 0, new, compute_hash), b'')
+ self.assertEqual(len(resize(b'', 1, new, compute_hash)), 1)
+ self.assertEqual(len(resize(b'', 2, new, compute_hash)), 2)
self.assertRaises(SystemError, resize, b'abc', -1, False)
self.assertRaises(SystemError, resize, bytearray(b'abc'), 3, False)
diff --git a/Lib/test/test_capi/test_unicode.py
b/Lib/test/test_capi/test_unicode.py
index 6e2fd13567f5fbe..d43318770a4e62c 100644
--- a/Lib/test/test_capi/test_unicode.py
+++ b/Lib/test/test_capi/test_unicode.py
@@ -170,28 +170,75 @@ def test_write_macro(self):
def test_resize(self):
"""Test PyUnicode_Resize()"""
- resize = _testlimitedcapi.unicode_resize
+ _resize = _testlimitedcapi.unicode_resize
+ resize_null = _testlimitedcapi.unicode_resize_null
+
+ def resize(s, length, new=True, compute_hash=False):
+ if s is not NULL and isinstance(s, str):
+ old_length = len(s)
+ else:
+ old_length = 0
+ result, int_result, refcnt, is_new_obj = _resize(s, length,
+ new, compute_hash)
+ self.assertEqual(int_result, 0)
+
+ if length == old_length:
+ # Return the same object unchanged
+ self.assertFalse(is_new_obj)
+ elif length == 0:
+ # Get the empty Unicode string
+ self.assertEqual(result, '')
+ self.assertTrue(sys._is_immortal(result))
+ self.assertTrue(is_new_obj)
+ elif (not new) or compute_hash:
+ # Get a fresh copy
+ self.assertEqual(refcnt, 1)
+ self.assertTrue(is_new_obj)
+ self.assertFalse(sys._is_immortal(result))
+ else:
+ # In-size replace can return the same address, or not.
+ # So 'is_new_obj' cannot be tested.
+ self.assertFalse(sys._is_immortal(result))
+
+ return result
strings = [
# all strings have exactly 3 characters
'abc', '\xa1\xa2\xa3', '\u4f60\u597d\u4e16',
'\U0001f600\U0001f601\U0001f602'
]
- for s in strings:
- self.assertEqual(resize(s, 3), (s, 0))
- self.assertEqual(resize(s, 2), (s[:2], 0))
- self.assertEqual(resize(s, 4), (s + '\0', 0))
- self.assertEqual(resize(s, 10), (s + '\0'*7, 0))
- self.assertEqual(resize(s, 0), ('', 0))
- self.assertRaises(MemoryError, resize, s, PY_SSIZE_T_MAX)
- self.assertRaises(SystemError, resize, s, -1)
- self.assertRaises(SystemError, resize, s, PY_SSIZE_T_MIN)
+ for new in (True, False):
+ for compute_hash in (True, False):
+ for s in strings:
+ with self.subTest(new=new, compute_hash=compute_hash, s=s):
+ self.assertEqual(resize(s, 3, new, compute_hash),
+ s)
+ self.assertEqual(resize(s, 2, new, compute_hash),
+ s[:2])
+ self.assertEqual(resize(s, 4, new, compute_hash),
+ s + '\0')
+ self.assertEqual(resize(s, 10, new, compute_hash),
+ s + '\0'*7)
+ self.assertEqual(resize(s, 0, new, compute_hash),
+ '')
+
+ with self.assertRaises(MemoryError):
+ resize(s, PY_SSIZE_T_MAX, new, compute_hash)
+ with self.assertRaises(SystemError):
+ resize(s, -1, new, compute_hash)
+ with self.assertRaises(SystemError):
+ resize(s, PY_SSIZE_T_MIN, new, compute_hash)
+
self.assertRaises(SystemError, resize, b'abc', 0)
self.assertRaises(SystemError, resize, [], 0)
self.assertRaises(SystemError, resize, NULL, 0)
# TODO: Test PyUnicode_Resize() with non-modifiable and legacy unicode
# and with NULL as the address.
+ # Test PyUnicode_Resize(NULL, length)
+ self.assertRaises(SystemError, resize_null, 0)
+ self.assertRaises(SystemError, resize_null, 123)
+
def test_append(self):
"""Test PyUnicode_Append()"""
append = _testlimitedcapi.unicode_append
diff --git a/Modules/_testcapi/bytes.c b/Modules/_testcapi/bytes.c
index f5a5fb0481d263a..e9ed765850a907a 100644
--- a/Modules/_testcapi/bytes.c
+++ b/Modules/_testcapi/bytes.c
@@ -11,14 +11,19 @@
/* Test _PyBytes_Resize() */
static PyObject *
-bytes_resize(PyObject *Py_UNUSED(module), PyObject *args)
+bytes_resize(PyObject *Py_UNUSED(module), PyObject *args, PyObject *kwargs)
{
+ static char *kwlist[] = {"obj", "newsize", "new", "compute_hash", NULL};
PyObject *obj;
Py_ssize_t newsize;
int new;
+ int compute_hash = 0;
- if (!PyArg_ParseTuple(args, "Onp", &obj, &newsize, &new))
+ if (!PyArg_ParseTupleAndKeywords(args, kwargs,
+ "Onp|p", kwlist,
+ &obj, &newsize, &new, &compute_hash)) {
return NULL;
+ }
NULLABLE(obj);
if (new) {
@@ -34,13 +39,24 @@ bytes_resize(PyObject *Py_UNUSED(module), PyObject *args)
else {
Py_XINCREF(obj);
}
+
+ if (compute_hash) {
+ if (PyObject_Hash(obj) == -1) {
+ return NULL;
+ }
+ }
+
+ PyObject *old_obj = obj;
if (_PyBytes_Resize(&obj, newsize) < 0) {
assert(obj == NULL);
+ return NULL;
}
else {
assert(obj != NULL);
}
- return obj;
+
+ Py_ssize_t refcnt = Py_REFCNT(obj);
+ return Py_BuildValue("Onp", obj, refcnt, obj != old_obj);
}
@@ -604,7 +620,7 @@ corrupt_bytes(PyObject *Py_UNUSED(module), PyObject *args)
static PyMethodDef test_methods[] = {
- {"bytes_resize", bytes_resize, METH_VARARGS},
+ {"bytes_resize", _PyCFunction_CAST(bytes_resize), METH_VARARGS |
METH_KEYWORDS},
{"bytes_join", bytes_join, METH_VARARGS},
{"byteswriter_abc", byteswriter_abc, METH_NOARGS},
{"byteswriter_resize", byteswriter_resize, METH_NOARGS},
diff --git a/Modules/_testlimitedcapi/unicode.c
b/Modules/_testlimitedcapi/unicode.c
index a8511a8242fe7a1..53fff1916ac1ab6 100644
--- a/Modules/_testlimitedcapi/unicode.c
+++ b/Modules/_testlimitedcapi/unicode.c
@@ -183,24 +183,59 @@ unicode_resize(PyObject *self, PyObject *args)
PyObject *obj, *copy;
Py_ssize_t length;
int result;
+ int new = 1;
+ int compute_hash = 0;
- if (!PyArg_ParseTuple(args, "On", &obj, &length)) {
+ if (!PyArg_ParseTuple(args, "On|pp", &obj, &length, &new, &compute_hash)) {
return NULL;
}
NULLABLE(obj);
- if (!(copy = unicode_copy(obj)) && obj) {
- return NULL;
+ Py_ssize_t old_len = obj ? PyUnicode_GetLength(obj) : 0;
+ if (obj != NULL && new) {
+ copy = unicode_copy(obj);
+ if (!copy) {
+ return NULL;
+ }
+ obj = copy;
}
- result = PyUnicode_Resize(©, length);
+ else {
+ Py_XINCREF(obj);
+ }
+
+ if (compute_hash && obj != NULL) {
+ if (PyObject_Hash(obj) == -1) {
+ Py_DECREF(obj);
+ return NULL;
+ }
+ }
+
+ PyObject *old_obj = obj;
+ result = PyUnicode_Resize(&obj, length);
if (result == -1 && PyErr_Occurred()) {
- Py_XDECREF(copy);
+ Py_XDECREF(obj);
return NULL;
}
- if (obj && PyUnicode_Check(obj) && length > PyUnicode_GetLength(obj)) {
- unicode_fill(copy, PyUnicode_GetLength(obj), length, 0U);
+ if (obj && PyUnicode_Check(obj) && length > old_len) {
+ unicode_fill(obj, old_len, length, 0U);
}
- return Py_BuildValue("(Ni)", copy, result);
+ Py_ssize_t refcnt = Py_REFCNT(obj);
+ return Py_BuildValue("(Ninp)", obj, result, refcnt, obj != old_obj);
+}
+
+// Test PyUnicode_Resize(NULL, length)
+static PyObject *
+unicode_resize_null(PyObject *self, PyObject *args)
+{
+ Py_ssize_t length;
+ if (!PyArg_ParseTuple(args, "n", &length)) {
+ return NULL;
+ }
+
+ if (PyUnicode_Resize(NULL, length)) {
+ return NULL;
+ }
+ Py_RETURN_NONE;
}
/* Test PyUnicode_Append() */
@@ -1882,6 +1917,7 @@ static PyMethodDef TestMethods[] = {
{"test_widechar", test_widechar, METH_NOARGS},
{"unicode_writechar", unicode_writechar, METH_VARARGS},
{"unicode_resize", unicode_resize, METH_VARARGS},
+ {"unicode_resize_null", unicode_resize_null, METH_VARARGS},
{"unicode_append", unicode_append, METH_VARARGS},
{"unicode_appendanddel", unicode_appendanddel, METH_VARARGS},
{"unicode_fromstringandsize",unicode_fromstringandsize, METH_VARARGS},
diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c
index 2339e8bc1d2819c..ac3a3340ec32019 100644
--- a/Objects/bytesobject.c
+++ b/Objects/bytesobject.c
@@ -3405,7 +3405,6 @@ bytes_resize_inplace(PyObject **pv, Py_ssize_t newsize)
PyBytesObject *sv = (PyBytesObject *)v;
Py_SET_SIZE(sv, newsize);
sv->ob_sval[newsize] = '\0';
- set_ob_shash(sv, -1); /* invalidate cached hash value */
assert(_PyBytes_IsMutable(*pv));
return 0;
}
@@ -3437,7 +3436,9 @@ _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t
newsize)
Py_ssize_t oldsize = PyBytes_GET_SIZE(v);
if (oldsize == newsize) {
- /* return early if newsize equals to v->ob_size */
+ // Leave the object unchanged if the new size is the same as the old
+ // size, even if the object is not uniquely referenced or if the hash
+ // value was already computed.
return 0;
}
@@ -3458,7 +3459,10 @@ _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t
newsize)
return 0;
}
- if (!_PyObject_IsUniquelyReferenced(v)) {
+ if (!_PyObject_IsUniquelyReferenced(v)
+ // Return a copy if the hash value was already computed
+ || get_ob_shash((PyBytesObject *)v) != -1)
+ {
// Allocate and then copy so we don't get a shared immortal
// one-character singleton!
result = _PyBytes_FromSize(newsize, 0);
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3//lists/python-checkins.python.org
Member address: [email protected]
