https://github.com/python/cpython/commit/8356e9c72170f2ef8df7edd0d69cc9b918a133d9
commit: 8356e9c72170f2ef8df7edd0d69cc9b918a133d9
branch: 3.14
author: Victor Stinner <[email protected]>
committer: vstinner <[email protected]>
date: 2026-10-16T22:09:04Z
summary:
[3.14] gh-158585: Fix _PyBytes_Resize() if hash value is already computed
(#159585)
* [3.15] gh-158585: Fix _PyBytes_Resize() if hash value is already computed
(#159555)
* 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().
(cherry picked from commit 6e7738bb5e6a294d888f8af6f409f52508194249)
* gh-158585: Fix refleak in _testcapi.bytes_resize() (#158806)
(cherry picked from commit 587b7a5b315a971ecd376184a1f1cbecaa9e6f21)
(cherry picked from commit 251278d5ee94bbff07a6d016aa8ff46151ef5723)
* Adjust code for 3.14
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 a3e10771b19da7..e2a39cb4252dcf 100644
--- a/Lib/test/test_capi/test_bytes.py
+++ b/Lib/test/test_capi/test_bytes.py
@@ -234,39 +234,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 27b281f20f4559..bef0749eab45f3 100644
--- a/Lib/test/test_capi/test_unicode.py
+++ b/Lib/test/test_capi/test_unicode.py
@@ -168,28 +168,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 33903de14ba68d..c30a041ade13f3 100644
--- a/Modules/_testcapi/bytes.c
+++ b/Modules/_testcapi/bytes.c
@@ -4,14 +4,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) {
@@ -27,13 +32,25 @@ bytes_resize(PyObject *Py_UNUSED(module), PyObject *args)
else {
Py_XINCREF(obj);
}
+
+ if (compute_hash) {
+ if (PyObject_Hash(obj) == -1) {
+ Py_DECREF(obj);
+ 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("Nnp", obj, refcnt, obj != old_obj);
}
@@ -52,7 +69,7 @@ bytes_join(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},
{NULL},
};
diff --git a/Modules/_testlimitedcapi/unicode.c
b/Modules/_testlimitedcapi/unicode.c
index b3dbc8cc0daec0..31e8b4c5cc9980 100644
--- a/Modules/_testlimitedcapi/unicode.c
+++ b/Modules/_testlimitedcapi/unicode.c
@@ -181,24 +181,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() */
@@ -1880,6 +1915,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 0b7ddfdf6b0020..63730630e68497 100644
--- a/Objects/bytesobject.c
+++ b/Objects/bytesobject.c
@@ -41,7 +41,7 @@ Py_LOCAL_INLINE(Py_ssize_t)
_PyBytesWriter_GetSize(_PyBytesWriter *writer,
#define CHARACTERS _Py_SINGLETON(bytes_characters)
#define CHARACTER(ch) \
- ((PyBytesObject *)&(CHARACTERS[ch]));
+ ((PyBytesObject *)&(CHARACTERS[ch]))
#define EMPTY (&_Py_SINGLETON(bytes_empty))
@@ -3209,6 +3209,75 @@ PyBytes_ConcatAndDel(PyObject **pv, PyObject *w)
}
+#ifndef NDEBUG
+// Make sure that a bytes object can still be mutated.
+//
+// Usage: assert(_PyBytes_IsMutable(obj)).
+int
+_PyBytes_IsMutable(PyObject *self)
+{
+ assert(PyBytes_Check(self));
+ assert(_PyObject_IsUniquelyReferenced(self));
+ assert(!_Py_IsImmortal(self));
+
+ // Check that the object is not a singleton
+ Py_ssize_t size = PyBytes_GET_SIZE(self);
+ if (size == 0) {
+ assert(self != bytes_get_empty());
+ }
+ else if (size == 1) {
+ unsigned char ch = PyBytes_AS_STRING(self)[0];
+ assert(self != (PyObject*)CHARACTER(ch));
+ }
+ return 1;
+}
+#endif
+
+
+static inline int
+bytes_resize_inplace(PyObject **pv, Py_ssize_t newsize)
+{
+ PyObject *v = *pv;
+ assert(_PyObject_IsUniquelyReferenced(v));
+ assert(PyBytes_GET_SIZE(v) >= 1);
+ assert(newsize >= 1);
+
+ // Only mutable bytes can be resized in-place
+ assert(_PyBytes_IsMutable(v));
+
+ if ((size_t)newsize > (size_t)PY_SSIZE_T_MAX - PyBytesObject_SIZE) {
+ PyErr_SetString(PyExc_OverflowError,
+ "byte string is too large");
+ return -1;
+ }
+
+#ifdef Py_TRACE_REFS
+ _Py_ForgetReference(v);
+#endif
+ _PyReftracerTrack(v, PyRefTracer_DESTROY);
+
+ PyObject *result = PyObject_Realloc(v, PyBytesObject_SIZE + newsize);
+ if (result == NULL) {
+#ifdef Py_TRACE_REFS
+ _Py_AddToAllObjects(v);
+#endif
+ _PyReftracerTrack(v, PyRefTracer_CREATE);
+
+ PyErr_NoMemory();
+ return -1;
+ }
+
+ *pv = result;
+ v = result;
+ _Py_NewReferenceNoTotal(v);
+ PyBytesObject *sv = (PyBytesObject *)v;
+ Py_SET_SIZE(sv, newsize);
+ sv->ob_sval[newsize] = '\0';
+ assert(_PyBytes_IsMutable(*pv));
+ return 0;
+}
+
+
/* The following function breaks the notion that bytes are immutable:
it changes the size of a bytes object. You can think of it
as creating a new bytes object and destroying the old one, only
@@ -3235,7 +3304,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;
}
@@ -3246,6 +3317,7 @@ _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t
newsize)
}
*pv = result;
Py_DECREF(v);
+ assert(_PyBytes_IsMutable(*pv));
return 0;
}
@@ -3255,7 +3327,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);
@@ -3267,39 +3342,11 @@ _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t
newsize)
Py_MIN(oldsize, newsize));
*pv = result;
Py_DECREF(v);
+ assert(_PyBytes_IsMutable(*pv));
return 0;
}
- if ((size_t)newsize > (size_t)PY_SSIZE_T_MAX - PyBytesObject_SIZE) {
- PyErr_SetString(PyExc_OverflowError,
- "byte string is too large");
- return -1;
- }
-
-#ifdef Py_TRACE_REFS
- _Py_ForgetReference(v);
-#endif
- _PyReftracerTrack(v, PyRefTracer_DESTROY);
-
- result = (PyObject *)PyObject_Realloc(v, PyBytesObject_SIZE + newsize);
- if (result == NULL) {
-#ifdef Py_TRACE_REFS
- _Py_AddToAllObjects(v);
-#endif
- _PyReftracerTrack(v, PyRefTracer_CREATE);
-
- PyErr_NoMemory();
- return -1;
- }
-
- *pv = result;
- v = result;
- _Py_NewReferenceNoTotal(v);
- PyBytesObject *sv = (PyBytesObject *)v;
- Py_SET_SIZE(sv, newsize);
- sv->ob_sval[newsize] = '\0';
- set_ob_shash(sv, -1); /* invalidate cached hash value */
- return 0;
+ return bytes_resize_inplace(pv, newsize);
}
_______________________________________________
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]
