From 003e4510690801166409744d048a55763d5f8d9d Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 22 Oct 2026 08:32:53 +0000 Subject: [PATCH 1/2] gh-160303: Fix bytearray state after a failed del b[:n] When shrinking the storage of a bytearray which has a logical offset, allocate the new buffer before modifying the object, so that bytearray_setslice_linear() can restore ob_start if the allocation fails. --- Lib/test/test_bytes.py | 20 +++++++++ ...-10-22-15-40-00.gh-issue-160303.q7Zb2K.rst | 3 ++ Objects/bytearrayobject.c | 41 +++++++++++-------- 3 files changed, 46 insertions(+), 18 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-10-22-15-40-00.gh-issue-160303.q7Zb2K.rst diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index 9c36fbed9eb19a..3a5f7511bf6815 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -1687,6 +1687,26 @@ def test_resize_error(self): ba.resize(1) self.assertEqual(ba, expected) + @support.nomemtest + def test_remove_prefix_error(self): + # If the storage cannot be shrunk after removing a prefix + # (MemoryError), the bytearray must be left unchanged. + for logical_offset in (False, True): + with self.subTest(logical_offset=logical_offset): + ba = bytearray(b'0123456789' * 4) + if logical_offset: + del ba[:3] + expected = ba.copy() + with inject_memory_error(self): + del ba[:30] + self.assertEqual(ba, expected) + with inject_memory_error(self): + ba[:30] = b'xy' + self.assertEqual(ba, expected) + # The bytearray is still usable + del ba[:30] + self.assertEqual(ba, expected[30:]) + def test_take_bytes(self): ba = bytearray(b'ab') self.assertEqual(ba.take_bytes(), b'ab') diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-22-15-40-00.gh-issue-160303.q7Zb2K.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-22-15-40-00.gh-issue-160303.q7Zb2K.rst new file mode 100644 index 00000000000000..d0766b48ad0275 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-22-15-40-00.gh-issue-160303.q7Zb2K.rst @@ -0,0 +1,3 @@ +Fix :class:`bytearray` reading and writing memory outside of its buffer +after ``del b[:n]`` or ``b[:n] = shorter`` failed with :exc:`MemoryError`. +The bytearray is now left unchanged. diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index dfd041b0152bdd..716342d29c96aa 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -238,28 +238,32 @@ bytearray_resize_storage(PyByteArrayObject *self, Py_ssize_t size = Py_SIZE(self); - /* Re-align data to the start of the allocation. */ - char *old_start = self->ob_start; - if (self->ob_start != self->ob_bytes) { - /* optimization tradeoff: This is faster than a new allocation when - the number of bytes being removed in a resize is small; for - large size changes it may be better to just make a new bytes - object as _PyBytes_Resize will do a malloc + memcpy internally. + if (self->ob_start != self->ob_bytes && new_size < size) { + /* Shrink with a logical offset: copy the bytes to keep into a new + allocation. Allocate first, so that the bytearray (including + ob_start) is left unchanged on error: bytearray_setslice_linear() + relies on it to restore the bytearray after a failed "del b[:n]". */ - Py_ssize_t move = Py_MIN(new_size, size); - memmove(self->ob_bytes, self->ob_start, move); - self->ob_start = self->ob_bytes; + PyObject *bytes = PyBytes_FromStringAndSize(NULL, alloc); + if (bytes == NULL) { + return -1; + } + memcpy(PyBytes_AS_STRING(bytes), self->ob_start, new_size); + Py_SETREF(self->ob_bytes_object, bytes); } + else { + /* Re-align data to the start of the allocation. */ + if (self->ob_start != self->ob_bytes) { + /* optimization tradeoff: This is faster than a new allocation + when the number of bytes being added in a resize is small. */ + memmove(self->ob_bytes, self->ob_start, size); + self->ob_start = self->ob_bytes; + } - if (_PyBytes_ResizeKeepOnError(&self->ob_bytes_object, alloc) < 0) { - if (old_start != self->ob_bytes && new_size < size) { - // Move remaining bytes - Py_ssize_t moved = new_size; - Py_ssize_t remaining = size - moved; - memmove(self->ob_bytes + moved, old_start + moved, remaining); + if (_PyBytes_ResizeKeepOnError(&self->ob_bytes_object, alloc) < 0) { + bytearray_write_trailing_null_byte(self); + return -1; } - bytearray_write_trailing_null_byte(self); - return -1; } assert(_PyBytes_IsMutable(self->ob_bytes_object)); return 0; @@ -649,6 +653,7 @@ bytearray_setslice_linear(PyByteArrayObject *self, /* memmove() removed bytes, the bytearray object cannot be restored in its previous state. */ Py_SET_SIZE(self, Py_SIZE(self) + growth); + bytearray_write_trailing_null_byte(self); res = -1; } buf = PyByteArray_AS_STRING(self); From 51dd1cab5184ed828148fd452d7698d345dc2885 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 31 Oct 2026 23:33:04 +0000 Subject: [PATCH 2/2] gh-160303: Keep ob_start instead of allocating in del b[:n] Move ob_start and the size before the shrinking resize, so a failed shrink leaves the bytes removed and the larger block in place, as the non-front path already does. Split the checks out of bytearray_resize_lock_held() so the slice code can resize directly. Move the tests into test_resize_error. --- Lib/test/test_bytes.py | 36 +++--- ...-10-22-15-40-00.gh-issue-160303.q7Zb2K.rst | 3 +- Objects/bytearrayobject.c | 117 +++++++++--------- 3 files changed, 83 insertions(+), 73 deletions(-) diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index 3a5f7511bf6815..411ea231c6268b 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -1663,6 +1663,7 @@ def test_resize_error(self): # the bytearray must be left unchanged. offset = 3 + front = slice(None, -2) for logical_offset in (False, True): with self.subTest(logical_offset=logical_offset): # grow bytearray @@ -1687,25 +1688,28 @@ def test_resize_error(self): ba.resize(1) self.assertEqual(ba, expected) - @support.nomemtest - def test_remove_prefix_error(self): - # If the storage cannot be shrunk after removing a prefix - # (MemoryError), the bytearray must be left unchanged. - for logical_offset in (False, True): - with self.subTest(logical_offset=logical_offset): - ba = bytearray(b'0123456789' * 4) + # gh-160303: When bytes are removed at the front, they are + # removed even if shrinking the memory block fails. + ba = bytearray(b'0123456789') if logical_offset: - del ba[:3] - expected = ba.copy() + del ba[:offset] with inject_memory_error(self): - del ba[:30] - self.assertEqual(ba, expected) + del ba[front] + self.assertEqual(ba, b'89') + ba.append(ord('a')) + del ba[:1] + self.assertEqual(ba, b'9a') + + ba = bytearray(b'0123456789') + if logical_offset: + del ba[:offset] + replacement = bytearray(b'xy') with inject_memory_error(self): - ba[:30] = b'xy' - self.assertEqual(ba, expected) - # The bytearray is still usable - del ba[:30] - self.assertEqual(ba, expected[30:]) + ba[front] = replacement + self.assertEqual(ba, b'xy89') + ba.append(ord('a')) + del ba[:1] + self.assertEqual(ba, b'y89a') def test_take_bytes(self): ba = bytearray(b'ab') diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-22-15-40-00.gh-issue-160303.q7Zb2K.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-22-15-40-00.gh-issue-160303.q7Zb2K.rst index d0766b48ad0275..c6f6b74f0aa0d7 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-22-15-40-00.gh-issue-160303.q7Zb2K.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-22-15-40-00.gh-issue-160303.q7Zb2K.rst @@ -1,3 +1,4 @@ Fix :class:`bytearray` reading and writing memory outside of its buffer after ``del b[:n]`` or ``b[:n] = shorter`` failed with :exc:`MemoryError`. -The bytearray is now left unchanged. +The bytes are now removed even if shrinking the memory block fails; +:exc:`MemoryError` is still raised. diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index 716342d29c96aa..12c95f7acde4ea 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -238,45 +238,46 @@ bytearray_resize_storage(PyByteArrayObject *self, Py_ssize_t size = Py_SIZE(self); - if (self->ob_start != self->ob_bytes && new_size < size) { - /* Shrink with a logical offset: copy the bytes to keep into a new - allocation. Allocate first, so that the bytearray (including - ob_start) is left unchanged on error: bytearray_setslice_linear() - relies on it to restore the bytearray after a failed "del b[:n]". + /* Re-align data to the start of the allocation. */ + char *old_start = self->ob_start; + if (self->ob_start != self->ob_bytes) { + /* optimization tradeoff: This is faster than a new allocation when + the number of bytes being removed in a resize is small; for + large size changes it may be better to just make a new bytes + object as _PyBytes_Resize will do a malloc + memcpy internally. */ - PyObject *bytes = PyBytes_FromStringAndSize(NULL, alloc); - if (bytes == NULL) { - return -1; - } - memcpy(PyBytes_AS_STRING(bytes), self->ob_start, new_size); - Py_SETREF(self->ob_bytes_object, bytes); + Py_ssize_t move = Py_MIN(new_size, size); + memmove(self->ob_bytes, self->ob_start, move); + self->ob_start = self->ob_bytes; } - else { - /* Re-align data to the start of the allocation. */ - if (self->ob_start != self->ob_bytes) { - /* optimization tradeoff: This is faster than a new allocation - when the number of bytes being added in a resize is small. */ - memmove(self->ob_bytes, self->ob_start, size); - self->ob_start = self->ob_bytes; - } - if (_PyBytes_ResizeKeepOnError(&self->ob_bytes_object, alloc) < 0) { - bytearray_write_trailing_null_byte(self); - return -1; + if (_PyBytes_ResizeKeepOnError(&self->ob_bytes_object, alloc) < 0) { + if (old_start != self->ob_bytes && new_size < size) { + // Move remaining bytes + Py_ssize_t moved = new_size; + Py_ssize_t remaining = size - moved; + memmove(self->ob_bytes + moved, old_start + moved, remaining); } + bytearray_write_trailing_null_byte(self); + return -1; } assert(_PyBytes_IsMutable(self->ob_bytes_object)); return 0; } +/* Resize the storage for requested_size bytes, even if it is already the + size of the bytearray. The caller must have checked that the bytearray + can be resized. */ static int -bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size) +bytearray_resize_unchecked(PyByteArrayObject *obj, Py_ssize_t requested_size) { - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); - PyByteArrayObject *obj = ((PyByteArrayObject *)self); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(obj); + PyObject *self = (PyObject *)obj; assert(obj->ob_bytes_object != NULL); + assert(requested_size >= 0); + assert(obj->ob_exports == 0); /* All computations are done unsigned to avoid integer overflows (see issue #22335). */ @@ -284,23 +285,8 @@ bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size) size_t logical_offset = (size_t) (obj->ob_start - obj->ob_bytes); size_t size = (size_t) requested_size; - assert(self != NULL); - assert(PyByteArray_Check(self)); assert(logical_offset <= alloc); - if (requested_size < 0) { - PyErr_Format(PyExc_ValueError, - "Can only resize to positive sizes, got %zd", requested_size); - return -1; - } - - if (requested_size == Py_SIZE(self)) { - return 0; - } - if (!_canresize(obj)) { - return -1; - } - /* Resize to 0 resets to empty bytes (see issue #153419). */ if (requested_size == 0) { Py_SETREF(obj->ob_bytes_object, @@ -352,6 +338,30 @@ bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size) return 0; } +static int +bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size) +{ + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); + PyByteArrayObject *obj = ((PyByteArrayObject *)self); + + assert(self != NULL); + assert(PyByteArray_Check(self)); + + if (requested_size < 0) { + PyErr_Format(PyExc_ValueError, + "Can only resize to positive sizes, got %zd", requested_size); + return -1; + } + + if (requested_size == Py_SIZE(self)) { + return 0; + } + if (!_canresize(obj)) { + return -1; + } + return bytearray_resize_unchecked(obj, requested_size); +} + int PyByteArray_Resize(PyObject *self, Py_ssize_t requested_size) { @@ -616,8 +626,12 @@ bytearray_setslice_linear(PyByteArrayObject *self, return -1; if (lo == 0) { - /* Shrink the buffer by advancing its logical start */ + /* Shrink the buffer by advancing its logical start. The bytes + are removed at this point; the resize below only releases + memory. */ self->ob_start -= growth; + Py_SET_SIZE(self, Py_SIZE(self) + growth); + growth = 0; /* 0 lo hi old_size | |<----avail----->|<-----tail------>| @@ -635,23 +649,14 @@ bytearray_setslice_linear(PyByteArrayObject *self, memmove(buf + lo + bytes_len, buf + hi, Py_SIZE(self) - hi); } - if (bytearray_resize_lock_held((PyObject *)self, - Py_SIZE(self) + growth) < 0) { + if (bytearray_resize_unchecked(self, Py_SIZE(self) + growth) < 0) { /* Issue #19578: Handling the memory allocation failure here is tricky here because the bytearray object has already been - modified. Depending on growth and lo, the behaviour is - different. - - If growth < 0 and lo != 0, the operation is completed, but a - MemoryError is still raised and the memory block is not - shrunk. Otherwise, the bytearray is restored in its previous - state and a MemoryError is raised. */ - if (lo == 0) { - self->ob_start += growth; - return -1; - } - /* memmove() removed bytes, the bytearray object cannot be - restored in its previous state. */ + modified: the bytes are removed and the bytearray object + cannot be restored in its previous state. + + The operation is completed, but a MemoryError is still + raised and the memory block is not shrunk. */ Py_SET_SIZE(self, Py_SIZE(self) + growth); bytearray_write_trailing_null_byte(self); res = -1;