diff --git a/Lib/test/test_capi/test_bytes.py b/Lib/test/test_capi/test_bytes.py index f8487a2b8fb085..367ded49fd9125 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 c5926dce397409..ffdaeaefc7c476 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 f5a5fb0481d263..e9ed765850a907 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 a8511a8242fe7a..53fff1916ac1ab 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 cb2c2f7bfae634..8d4becd658316d 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -3395,7 +3395,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; } @@ -3427,7 +3426,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; } @@ -3448,7 +3449,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);