This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Memory errors in array.array
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Chuck, amaury.forgeotdarc, berker.peksag, chuck, martin.panter, python-dev, r.david.murray
Priority: low Keywords: patch

Created on 2009-10-05 08:12 by chuck, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
array_del_slice.patch Chuck, 2013-02-19 14:46 review
Issue7063.diff BreamoreBoy, 2014-07-11 22:16 Original patch reworked as it no longer applied. review
Messages (9)
msg93581 - (view) Author: Jan (chuck) * Date: 2009-10-05 08:12
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?
msg93584 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2009-10-05 09:01
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).
msg93598 - (view) Author: Jan (chuck) * Date: 2009-10-05 12:52
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()?
msg182352 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-02-19 00:30
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?
msg182378 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2013-02-19 13:27
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.
msg182384 - (view) Author: Jan Hosang (Chuck) Date: 2013-02-19 14:46
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.
msg222803 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-07-11 22:16
I've tested the reworked patch on Windows 7, ran 718 tests with 1 skipped both before and after applying the patch.
msg271134 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-07-24 07:06
The patch looks good to me (assuming it still applies). Should also remove the “b” macro.
msg271209 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-07-25 02:22
New changeset ab28676df655 by Martin Panter in branch 'default':
Issue #7063: Remove dead code from array slice handling
https://hg.python.org/cpython/rev/ab28676df655
History
Date User Action Args
2022-04-11 14:56:53adminsetgithub: 51312
2016-07-25 02:45:10martin.pantersetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2016-07-25 02:22:33python-devsetnosy: + python-dev
messages: + msg271209
2016-07-24 07:06:24martin.pantersetnosy: + martin.panter
messages: + msg271134

components: + Extension Modules, - Library (Lib), Tests
stage: patch review -> commit review
2016-06-08 20:09:57BreamoreBoysetnosy: - BreamoreBoy
2016-06-08 15:22:20berker.peksagsetnosy: + berker.peksag
stage: needs patch -> patch review

versions: + Python 3.6, - Python 3.3, Python 3.4
2014-07-18 11:49:00skrahsetnosy: - skrah
2014-07-11 22:16:43BreamoreBoysetfiles: + Issue7063.diff
nosy: + BreamoreBoy
messages: + msg222803

2013-02-19 14:46:23Chucksetfiles: + array_del_slice.patch

nosy: + Chuck
messages: + msg182384

keywords: + patch
2013-02-19 13:27:57skrahsetpriority: normal -> low
versions: + Python 3.3, Python 3.4, - Python 3.2
messages: + msg182378

type: enhancement
stage: needs patch
2013-02-19 00:30:34r.david.murraysetnosy: + skrah, r.david.murray
messages: + msg182352
2009-10-05 12:52:16chucksetmessages: + msg93598
2009-10-05 09:01:32amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg93584
2009-10-05 08:12:28chuckcreate