Skip to content

gh-160303: Fix bytearray state after a failed del b[:n] - #160304

Open
gpshead wants to merge 1 commit into
python:mainfrom
gpshead:scanfix-bytearrayobjec-qrz6idtc
Open

gpshead wants to merge 1 commit into
python:mainfrom
gpshead:scanfix-bytearrayobjec-qrz6idtc

Conversation

@gpshead

@gpshead gpshead commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

bytearray_setslice_linear() advances ob_start before resizing and moves it back if the resize fails, but bytearray_resize_storage() realigned the data and reset ob_start before calling the allocator, so after a failed del b[:n] the bytearray started n bytes before its buffer. When shrinking a bytearray that has a logical offset, the new storage is now allocated first and the kept bytes are copied into it, so a failure leaves the object untouched (as 3.14 did); the recovery memmove() that read past the buffer is gone. The failure path for lo != 0 also writes the trailing null byte it was missing.

Caveat: that path is now malloc + memcpy instead of memmove + shrinking realloc. In a FIFO microbenchmark (append 8 chunks, then del b[:2/3] repeatedly) on a debug build, 64-byte chunks were about 8% faster, 4 KiB chunks about 12% slower and 1 MiB chunks about 35-40% slower.

Fixes #160303

ANT-2026-QRZ6IDTC

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.

@cmaloney cmaloney left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vstinner recently made it so that failed allocations keep the pre-existing bytes (#157242). That means we should always have a valid bytes. Backported to 3.15 (3.14 has a different internal bytearray structure).

bytearray implements a fast delete at front by having an ob_start member which can be incremented. Also fast delete at end: Leave ob_alloc as is and call Py_SET_SIZE to set the new size. The "effective" bytes being smaller than the allocation is okay.

The bug report sounds like we're getting that pointer manipulation off? I would strongly prefer not allocating a new bytes here with PyBytes_FromStringAndSize

Comment thread Lib/test/test_bytes.py
self.assertEqual(ba, expected)

@support.nomemtest
def test_remove_prefix_error(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these cases be moved to test_resize_error? That already has some shrink errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

bytearray reads and writes outside its buffer after del b[:n] fails with MemoryError

2 participants