FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[3.15] gh-158585: Fix _PyBytes_Resize() if hash value is already comp… · python/cpython@251278d · GitHub

  1. python
  2. cpython

Repository navigation

Commit 251278d

Browse files
vstinner
authored
[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 6e7738b) * gh-158585: Fix refleak in _testcapi.bytes_resize() (#158806) (cherry picked from commit 587b7a5)
1 parent adc1127 commit 251278d

5 files changed

Lines changed: 225 additions & 83 deletions

File tree

‎Lib/test/test_capi/test_bytes.py‎

Lines changed: 49 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -237,39 +237,60 @@ def test_resize(self):
237237
"""Test _PyBytes_Resize()"""
238238
_resize = _testcapi.bytes_resize
239239

240-
def resize(obj, size, new):
241-
result = _resize(obj, size, new)
242-
if 1 <= len(result):
243-
if new or size != len(obj):
244-
# gh-156995: Make sure that the result is a fresh object.
245-
# Previously, _PyBytes_Resize(&obj, 1) returned a singleton
246-
# if _PyObject_IsUniquelyReferenced() is false.
247-
self.assertEqual(sys.getrefcount(result), 1)
248-
self.assertFalse(sys._is_immortal(result))
249-
else:
240+
def assert_is_fresh_copy(result, refcnt, is_new_obj):
241+
self.assertEqual(refcnt, 1)
242+
self.assertTrue(is_new_obj)
243+
self.assertFalse(sys._is_immortal(result))
244+
245+
def resize(obj, size, new, compute_hash=False):
246+
old_size = len(obj)
247+
result, refcnt, is_new_obj = _resize(obj, size, new,
248+
compute_hash=compute_hash)
249+
250+
if size == old_size:
251+
# Return the same object unchanged
252+
self.assertFalse(is_new_obj)
253+
elif old_size == 0:
254+
assert_is_fresh_copy(result, refcnt, is_new_obj)
255+
elif size == 0:
250256
# check that the result is the empty bytes string singleton
257+
self.assertEqual(result, b'')
251258
self.assertTrue(sys._is_immortal(result))
259+
self.assertTrue(is_new_obj)
260+
elif (not new) or compute_hash:
261+
# gh-156995: Make sure that the result is a fresh object.
262+
# Previously, _PyBytes_Resize(&obj, 1) returned a singleton
263+
# if _PyObject_IsUniquelyReferenced() is false.
264+
assert_is_fresh_copy(result, refcnt, is_new_obj)
265+
else:
266+
# An in-place resize can return the same memory address, or
267+
# not. 'is_new_obj' cannot be tested.
268+
self.assertEqual(refcnt, 1)
269+
self.assertFalse(sys._is_immortal(result))
270+
252271
return result
253272

254273
for new in True, False:
255-
with self.subTest(new=new):
256-
self.assertEqual(resize(b'abc', 0, new), b'')
257-
self.assertEqual(resize(b'abc', 1, new), b'a')
258-
self.assertEqual(resize(b'abc', 2, new), b'ab')
259-
self.assertEqual(resize(b'abc', 3, new), b'abc')
260-
b = resize(b'abc', 4, new)
261-
self.assertEqual(len(b), 4)
262-
self.assertEqual(b[:3], b'abc')
263-
264-
self.assertEqual(resize(b'a', 0, new), b'')
265-
self.assertEqual(resize(b'a', 1, new), b'a')
266-
b = resize(b'a', 2, new)
267-
self.assertEqual(len(b), 2)
268-
self.assertEqual(b[:1], b'a')
269-
270-
self.assertEqual(resize(b'', 0, new), b'')
271-
self.assertEqual(len(resize(b'', 1, new)), 1)
272-
self.assertEqual(len(resize(b'', 2, new)), 2)
274+
for compute_hash in True, False:
275+
with self.subTest(new=new, compute_hash=compute_hash):
276+
self.assertEqual(resize(b'abc', 0, new, compute_hash), b'')
277+
self.assertEqual(resize(b'abc', 1, new, compute_hash), b'a')
278+
self.assertEqual(resize(b'abc', 2, new, compute_hash), b'ab')
279+
self.assertEqual(resize(b'abc', 3, new, compute_hash), b'abc')
280+
281+
b = resize(b'abc', 4, new, compute_hash)
282+
self.assertEqual(len(b), 4)
283+
self.assertEqual(b[:3], b'abc')
284+
285+
self.assertEqual(resize(b'a', 0, new, compute_hash), b'')
286+
self.assertEqual(resize(b'a', 1, new, compute_hash), b'a')
287+
b = resize(b'a', 2, new, compute_hash)
288+
self.assertEqual(len(b), 2)
289+
self.assertEqual(b[:1], b'a')
290+
291+
self.assertEqual(resize(b'', 0, new, compute_hash), b'')
292+
self.assertEqual(len(resize(b'', 1, new, compute_hash)), 1)
293+
self.assertEqual(len(resize(b'', 2, new, compute_hash)), 2)
273294

274295
self.assertRaises(SystemError, resize, b'abc', -1, False)
275296
self.assertRaises(SystemError, resize, bytearray(b'abc'), 3, False)

‎Lib/test/test_capi/test_unicode.py‎

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -168,28 +168,75 @@ def test_write_macro(self):
168168

169169
def test_resize(self):
170170
"""Test PyUnicode_Resize()"""
171-
resize = _testlimitedcapi.unicode_resize
171+
_resize = _testlimitedcapi.unicode_resize
172+
resize_null = _testlimitedcapi.unicode_resize_null
173+
174+
def resize(s, length, new=True, compute_hash=False):
175+
if s is not NULL and isinstance(s, str):
176+
old_length = len(s)
177+
else:
178+
old_length = 0
179+
result, int_result, refcnt, is_new_obj = _resize(s, length,
180+
new, compute_hash)
181+
self.assertEqual(int_result, 0)
182+
183+
if length == old_length:
184+
# Return the same object unchanged
185+
self.assertFalse(is_new_obj)
186+
elif length == 0:
187+
# Get the empty Unicode string
188+
self.assertEqual(result, '')
189+
self.assertTrue(sys._is_immortal(result))
190+
self.assertTrue(is_new_obj)
191+
elif (not new) or compute_hash:
192+
# Get a fresh copy
193+
self.assertEqual(refcnt, 1)
194+
self.assertTrue(is_new_obj)
195+
self.assertFalse(sys._is_immortal(result))
196+
else:
197+
# In-size replace can return the same address, or not.
198+
# So 'is_new_obj' cannot be tested.
199+
self.assertFalse(sys._is_immortal(result))
200+
201+
return result
172202

173203
strings = [
174204
# all strings have exactly 3 characters
175205
'abc', '\xa1\xa2\xa3', '\u4f60\u597d\u4e16',
176206
'\U0001f600\U0001f601\U0001f602'
177207
]
178-
for s in strings:
179-
self.assertEqual(resize(s, 3), (s, 0))
180-
self.assertEqual(resize(s, 2), (s[:2], 0))
181-
self.assertEqual(resize(s, 4), (s + '\0', 0))
182-
self.assertEqual(resize(s, 10), (s + '\0'*7, 0))
183-
self.assertEqual(resize(s, 0), ('', 0))
184-
self.assertRaises(MemoryError, resize, s, PY_SSIZE_T_MAX)
185-
self.assertRaises(SystemError, resize, s, -1)
186-
self.assertRaises(SystemError, resize, s, PY_SSIZE_T_MIN)
208+
for new in (True, False):
209+
for compute_hash in (True, False):
210+
for s in strings:
211+
with self.subTest(new=new, compute_hash=compute_hash, s=s):
212+
self.assertEqual(resize(s, 3, new, compute_hash),
213+
s)
214+
self.assertEqual(resize(s, 2, new, compute_hash),
215+
s[:2])
216+
self.assertEqual(resize(s, 4, new, compute_hash),
217+
s + '\0')
218+
self.assertEqual(resize(s, 10, new, compute_hash),
219+
s + '\0'*7)
220+
self.assertEqual(resize(s, 0, new, compute_hash),
221+
'')
222+
223+
with self.assertRaises(MemoryError):
224+
resize(s, PY_SSIZE_T_MAX, new, compute_hash)
225+
with self.assertRaises(SystemError):
226+
resize(s, -1, new, compute_hash)
227+
with self.assertRaises(SystemError):
228+
resize(s, PY_SSIZE_T_MIN, new, compute_hash)
229+
187230
self.assertRaises(SystemError, resize, b'abc', 0)
188231
self.assertRaises(SystemError, resize, [], 0)
189232
self.assertRaises(SystemError, resize, NULL, 0)
190233
# TODO: Test PyUnicode_Resize() with non-modifiable and legacy unicode
191234
# and with NULL as the address.
192235

236+
# Test PyUnicode_Resize(NULL, length)
237+
self.assertRaises(SystemError, resize_null, 0)
238+
self.assertRaises(SystemError, resize_null, 123)
239+
193240
def test_append(self):
194241
"""Test PyUnicode_Append()"""
195242
append = _testlimitedcapi.unicode_append

‎Modules/_testcapi/bytes.c‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,19 @@
1111

1212
/* Test _PyBytes_Resize() */
1313
static PyObject *
14-
bytes_resize(PyObject *Py_UNUSED(module), PyObject *args)
14+
bytes_resize(PyObject *Py_UNUSED(module), PyObject *args, PyObject *kwargs)
1515
{
16+
static char *kwlist[] = {"obj", "newsize", "new", "compute_hash", NULL};
1617
PyObject *obj;
1718
Py_ssize_t newsize;
1819
int new;
20+
int compute_hash = 0;
1921

20-
if (!PyArg_ParseTuple(args, "Onp", &obj, &newsize, &new))
22+
if (!PyArg_ParseTupleAndKeywords(args, kwargs,
23+
"Onp|p", kwlist,
24+
&obj, &newsize, &new, &compute_hash)) {
2125
return NULL;
26+
}
2227

2328
NULLABLE(obj);
2429
if (new) {
@@ -34,13 +39,25 @@ bytes_resize(PyObject *Py_UNUSED(module), PyObject *args)
3439
else {
3540
Py_XINCREF(obj);
3641
}
42+
43+
if (compute_hash) {
44+
if (PyObject_Hash(obj) == -1) {
45+
Py_DECREF(obj);
46+
return NULL;
47+
}
48+
}
49+
50+
PyObject *old_obj = obj;
3751
if (_PyBytes_Resize(&obj, newsize) < 0) {
3852
assert(obj == NULL);
53+
return NULL;
3954
}
4055
else {
4156
assert(obj != NULL);
4257
}
43-
return obj;
58+
59+
Py_ssize_t refcnt = Py_REFCNT(obj);
60+
return Py_BuildValue("Nnp", obj, refcnt, obj != old_obj);
4461
}
4562

4663

@@ -526,7 +543,7 @@ test_byteswriter_ptr(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args))
526543

527544

528545
static PyMethodDef test_methods[] = {
529-
{"bytes_resize", bytes_resize, METH_VARARGS},
546+
{"bytes_resize", _PyCFunction_CAST(bytes_resize), METH_VARARGS | METH_KEYWORDS},
530547
{"bytes_join", bytes_join, METH_VARARGS},
531548
{"byteswriter_abc", byteswriter_abc, METH_NOARGS},
532549
{"byteswriter_resize", byteswriter_resize, METH_NOARGS},

‎Modules/_testlimitedcapi/unicode.c‎

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -183,24 +183,59 @@ unicode_resize(PyObject *self, PyObject *args)
183183
PyObject *obj, *copy;
184184
Py_ssize_t length;
185185
int result;
186+
int new = 1;
187+
int compute_hash = 0;
186188

187-
if (!PyArg_ParseTuple(args, "On", &obj, &length)) {
189+
if (!PyArg_ParseTuple(args, "On|pp", &obj, &length, &new, &compute_hash)) {
188190
return NULL;
189191
}
190192

191193
NULLABLE(obj);
192-
if (!(copy = unicode_copy(obj)) && obj) {
193-
return NULL;
194+
Py_ssize_t old_len = obj ? PyUnicode_GetLength(obj) : 0;
195+
if (obj != NULL && new) {
196+
copy = unicode_copy(obj);
197+
if (!copy) {
198+
return NULL;
199+
}
200+
obj = copy;
194201
}
195-
result = PyUnicode_Resize(&copy, length);
202+
else {
203+
Py_XINCREF(obj);
204+
}
205+
206+
if (compute_hash && obj != NULL) {
207+
if (PyObject_Hash(obj) == -1) {
208+
Py_DECREF(obj);
209+
return NULL;
210+
}
211+
}
212+
213+
PyObject *old_obj = obj;
214+
result = PyUnicode_Resize(&obj, length);
196215
if (result == -1 && PyErr_Occurred()) {
197-
Py_XDECREF(copy);
216+
Py_XDECREF(obj);
198217
return NULL;
199218
}
200-
if (obj && PyUnicode_Check(obj) && length > PyUnicode_GetLength(obj)) {
201-
unicode_fill(copy, PyUnicode_GetLength(obj), length, 0U);
219+
if (obj && PyUnicode_Check(obj) && length > old_len) {
220+
unicode_fill(obj, old_len, length, 0U);
202221
}
203-
return Py_BuildValue("(Ni)", copy, result);
222+
Py_ssize_t refcnt = Py_REFCNT(obj);
223+
return Py_BuildValue("(Ninp)", obj, result, refcnt, obj != old_obj);
224+
}
225+
226+
// Test PyUnicode_Resize(NULL, length)
227+
static PyObject *
228+
unicode_resize_null(PyObject *self, PyObject *args)
229+
{
230+
Py_ssize_t length;
231+
if (!PyArg_ParseTuple(args, "n", &length)) {
232+
return NULL;
233+
}
234+
235+
if (PyUnicode_Resize(NULL, length)) {
236+
return NULL;
237+
}
238+
Py_RETURN_NONE;
204239
}
205240

206241
/* Test PyUnicode_Append() */
@@ -1882,6 +1917,7 @@ static PyMethodDef TestMethods[] = {
18821917
{"test_widechar", test_widechar, METH_NOARGS},
18831918
{"unicode_writechar", unicode_writechar, METH_VARARGS},
18841919
{"unicode_resize", unicode_resize, METH_VARARGS},
1920+
{"unicode_resize_null", unicode_resize_null, METH_VARARGS},
18851921
{"unicode_append", unicode_append, METH_VARARGS},
18861922
{"unicode_appendanddel", unicode_appendanddel, METH_VARARGS},
18871923
{"unicode_fromstringandsize",unicode_fromstringandsize, METH_VARARGS},

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL