| 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) *  |
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) * |
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) * |
Date: 2013-02-19 13:27 |
| I think 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) * |
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)  |
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 |
|
|