Memory errors in array.array

[chuck]

BPO	7063
Nosy	@amauryfa, @bitdancer, @berkerpeksag, @vadmium
Files	array_del_slice.patch Issue7063.diff: Original patch reworked as it no longer applied.

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2016-07-25.02:45:10.821>
created_at = <Date 2009-10-05.08:12:28.045>
labels = ['extension-modules', 'type-feature']
title = 'Memory errors in array.array'
updated_at = <Date 2016-07-25.02:45:10.821>
user = 'https://bugs.python.org/chuck'

bugs.python.org fields:

activity = <Date 2016-07-25.02:45:10.821>
actor = 'martin.panter'
assignee = 'none'
closed = True
closed_date = <Date 2016-07-25.02:45:10.821>
closer = 'martin.panter'
components = ['Extension Modules']
creation = <Date 2009-10-05.08:12:28.045>
creator = 'chuck'
dependencies = []
files = ['29121', '35926']
hgrepos = []
issue_num = 7063
keywords = ['patch']
message_count = 9.0
messages = ['93581', '93584', '93598', '182352', '182378', '182384', '222803', '271134', '271209']
nosy_count = 7.0
nosy_names = ['amaury.forgeotdarc', 'r.david.murray', 'chuck', 'python-dev', 'berker.peksag', 'martin.panter', 'Chuck']
pr_nums = []
priority = 'low'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue7063'
versions = ['Python 3.6']

[chuck]

While I was backporting the new buffer API to 2.7 I noticed some issues
in array_ass_slice() in Modules/arraymodule.c in the python 3k branch.

1. Manual memory reallocation had been replaced by calls to
   array_resize. But I think when PyMem_RESIZE is called the pointer to the
   memory might change. So this now happens in array_resize, and the
   array->ob_item pointer changes but not it's local copy (item) in
   array_ass_slice().

2. The function moves too much memory if the array size is increased:
   (Py_SIZE(a)-d-ihigh) items should be moved, because Py_SIZE(a) was
   already modified by array_resize, but the function moves (Py_SIZE(a)-
   ihigh) items.

While 1) might go unnoticed, 2) definitely broke slice tests in a
"segmentation fault"-way (in debug mode forbidden bits show the error).
I tried to write a test, but I don't know how to trigger
array_ass_slice() with a write access, as it is not in array_as_sequence
anymore (like in 2.7). How is slicing handled now?

[amauryfa]

The array type also defines tp_as_mapping->mp_ass_subscript, which has
priority in PyObject_SetItem().
A way to call array_ass_slice() is to use PySequence_SetItem(), but this
is hard to trigger from python code (it should be possible with ctypes).

[chuck]

The mp_ass_subscript function looks fine in contrast to array_ass_slice().
So if array_ass_slice() is not accessible from the outside and is only
called with NULL as replacement parameter from the inside, I won't be able
to cause trouble with those two issues.

Still I think it's bad to keep buggy code around, even it is not used.
Maybe array_ass_slice() should be changed to what it's used for:
array_del_slice()?

[bitdancer]

Stefan, IIRC you reworked some of the buffer/memoryview code. Do you know if what is reported here is still an issue, and if so if it is worth fixing?

[skrah]

I think msg93598 sums it up: array_ass_slice() is only called with
v==NULL, so the issue can't be triggered.

However, it's pretty dirty to leave the code as is (IIRC Coverity
also had some complaints), so Chuck's suggestion to rewrite the
function as array_del_slice() seems good to me.

[chuck]

I attached a patch, in which I removed v and all code having to do with inserting elements. In particular, I changed the value of b to being positive, since there is no distinction between increasing and decreasing size anymore.

[BreamoreBoy]

I've tested the reworked patch on Windows 7, ran 718 tests with 1 skipped both before and after applying the patch.

[vadmium]

The patch looks good to me (assuming it still applies). Should also remove the “b” macro.

[python-dev]

New changeset ab28676df655 by Martin Panter in branch 'default':
Issue bpo-7063: Remove dead code from array slice handling
https://hg.python.org/cpython/rev/ab28676df655