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

[orenmn]

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]

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]

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

[serhiy-storchaka]

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.

[orenmn]

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

[serhiy-storchaka]

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]

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]

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.