Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Memory errors in array.array #51312

Closed
chuck mannequin opened this issue Oct 5, 2009 · 9 comments
Closed

Memory errors in array.array #51312

chuck mannequin opened this issue Oct 5, 2009 · 9 comments
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@chuck
Copy link
Mannequin

chuck mannequin commented Oct 5, 2009

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
    Copy link
    Mannequin Author

    chuck mannequin commented Oct 5, 2009

    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?

    @chuck chuck mannequin added stdlib Python modules in the Lib dir tests Tests in the Lib/test dir labels Oct 5, 2009
    @amauryfa
    Copy link
    Member

    amauryfa commented Oct 5, 2009

    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
    Copy link
    Mannequin Author

    chuck mannequin commented Oct 5, 2009

    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
    Copy link
    Member

    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
    Copy link
    Mannequin

    skrah mannequin commented Feb 19, 2013

    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.

    @skrah skrah mannequin added the type-feature A feature request or enhancement label Feb 19, 2013
    @chuck
    Copy link
    Mannequin Author

    chuck mannequin commented Feb 19, 2013

    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
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jul 11, 2014

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

    @vadmium
    Copy link
    Member

    vadmium commented Jul 24, 2016

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

    @vadmium vadmium added extension-modules C modules in the Modules dir and removed stdlib Python modules in the Lib dir tests Tests in the Lib/test dir labels Jul 24, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 25, 2016

    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

    @vadmium vadmium closed this as completed Jul 25, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @bitdancer @amauryfa @vadmium