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.

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

a SystemError and a crash in PyCData_setstate() when __dict__ is bad #75492

Closed
orenmn mannequin opened this issue Aug 30, 2017 · 10 comments
Closed

a SystemError and a crash in PyCData_setstate() when __dict__ is bad #75492

orenmn mannequin opened this issue Aug 30, 2017 · 10 comments
Labels
3.7 (EOL) end of life topic-ctypes type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@orenmn
Copy link
Mannequin

orenmn mannequin commented Aug 30, 2017

BPO 31311
Nosy @serhiy-storchaka, @orenmn
PRs
  • bpo-31311: fix a SystemError and a crash in PyCData_setstate(), in case of a bad __dict__ #3254
  • bpo-31311: raise a TypeError in case the first argument to PyCData_setstate() isn't a dictionary #3255
  • [3.6] bpo-31311: Fix a SystemError and a crash in ctypes._CData.__setstate__(), in case of a bad __dict__. (GH-3254) #3743
  • [2.7] bpo-31311: Fix a SystemError and a crash in ctypes._CData.__set… #3781
  • 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 2017-09-27.06:40:21.439>
    created_at = <Date 2017-08-30.19:29:41.381>
    labels = ['ctypes', '3.7', 'type-crash']
    title = 'a SystemError and a crash in PyCData_setstate() when __dict__ is bad'
    updated_at = <Date 2017-09-27.06:40:21.407>
    user = 'https://github.com/orenmn'

    bugs.python.org fields:

    activity = <Date 2017-09-27.06:40:21.407>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-09-27.06:40:21.439>
    closer = 'serhiy.storchaka'
    components = ['ctypes']
    creation = <Date 2017-08-30.19:29:41.381>
    creator = 'Oren Milman'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 31311
    keywords = ['patch']
    message_count = 10.0
    messages = ['301034', '301035', '301036', '301807', '302840', '302841', '302914', '302919', '302928', '303099']
    nosy_count = 2.0
    nosy_names = ['serhiy.storchaka', 'Oren Milman']
    pr_nums = ['3254', '3255', '3743', '3781']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue31311'
    versions = ['Python 2.7', 'Python 3.6', 'Python 3.7']

    @orenmn
    Copy link
    Mannequin Author

    orenmn mannequin commented Aug 30, 2017

    The following code causes PyCData_setstate() (in Modules/_ctypes/_ctypes.c) to
    raise a SystemError:

    import ctypes
    class BadStruct(ctypes.Structure):
        def __dict__(self):
            pass

    BadStruct().__setstate__({}, b'foo')

    this is because PyCData_setstate() assumes that the __dict__ attribute is a dict.

    while we are here, I wonder whether we should change the format given to
    PyArg_ParseTuple() to "!Os#", so that the following would raise a TypeError:

    import ctypes
    class MyStruct(ctypes.Structure):
        pass

    MyStruct().__setstate__(42, b'foo')

    AttributeError: 'int' object has no attribute 'keys'

    what do you think?

    @orenmn orenmn mannequin added type-bug An unexpected behavior, bug, or error extension-modules C modules in the Modules dir 3.7 (EOL) end of life labels Aug 30, 2017
    @orenmn
    Copy link
    Mannequin Author

    orenmn mannequin commented Aug 30, 2017

    typo - change the format to "O!s#"

    @serhiy-storchaka
    Copy link
    Member

    And the following example results in segmentation fault.

    import ctypes
    class BadStruct(ctypes.Structure):
        @property
        def __dict__(self):
            raise AttributeError

    BadStruct().__setstate__({}, b'')

    As for change the format given to PyArg_ParseTuple(), I think it can be done only in master. It is better to make a separate PR for this.

    @serhiy-storchaka serhiy-storchaka added type-crash A hard crash of the interpreter, possibly with a core dump and removed type-bug An unexpected behavior, bug, or error labels Aug 30, 2017
    @orenmn orenmn mannequin changed the title SystemError raised by PyCData_setstate() in case __dict__ is not a dict a SystemError and a crash in PyCData_setstate() when __dict__ is bad Aug 31, 2017
    @orenmn orenmn mannequin added topic-ctypes and removed extension-modules C modules in the Modules dir labels Aug 31, 2017
    @orenmn
    Copy link
    Mannequin Author

    orenmn mannequin commented Sep 10, 2017

    just in case it was missed - I have opened two PRs for this issue.

    @serhiy-storchaka
    Copy link
    Member

    New changeset 4facdf5 by Serhiy Storchaka (Oren Milman) in branch 'master':
    bpo-31311: Impove error reporting in case the first argument to PyCData_setstate() isn't a dictionary. (bpo-3255)
    4facdf5

    @serhiy-storchaka
    Copy link
    Member

    I thought about this issue so long because I can't find good cause for __dict__ to be not a dict. Defining the __dict__ method or property doesn't affect instance dictionary and attributes lookup. It just breaks the __setstate__ method. Without having good example of overriding __dict__ I have no good test cases and don't know what a way of handling this error is better.

    There is yet one disadvantage of the current implementation. The instance's dict can be lazy. It can be created only on demand, when instance's attribute is set or the __dict__ attribute is read. PyObject_GetAttrString(myself, "__dict__") creates it if it was not created. It would be more efficient to use _PyObject_GetDictPtr(). But this is a separate issue, 3.7 only.

    @orenmn
    Copy link
    Mannequin Author

    orenmn mannequin commented Sep 25, 2017

    But this is a separate issue, 3.7 only.
    I don't think i understand what this issue would include.

    Anyway, i updated the PR according to your comments.

    @serhiy-storchaka
    Copy link
    Member

    New changeset 57c2561 by Serhiy Storchaka (Oren Milman) in branch 'master':
    bpo-31311: Fix a SystemError and a crash in ctypes._CData.__setstate__(), in case of a bad __dict__. (bpo-3254)
    57c2561

    @serhiy-storchaka
    Copy link
    Member

    New changeset a6bddb8 by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
    [3.6] bpo-31311: Fix a SystemError and a crash in ctypes._CData.__setstate__(), in case of a bad __dict__. (GH-3254) (bpo-3743)
    a6bddb8

    @serhiy-storchaka
    Copy link
    Member

    New changeset 81691b0 by Serhiy Storchaka in branch '2.7':
    [2.7] bpo-31311: Fix a SystemError and a crash in ctypes._CData.__setstate__(), in case of a bad __dict__. (GH-3254). (bpo-3781)
    81691b0

    @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
    Labels
    3.7 (EOL) end of life topic-ctypes type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant