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

ctypes: fail to create a _ctypes._SimpleCData subclass using a closure like calling super() without arguments #73456

Open
waveform80 mannequin opened this issue Jan 13, 2017 · 15 comments
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 new features, bugs and security fixes topic-ctypes type-bug An unexpected behavior, bug, or error

Comments

@waveform80
Copy link
Mannequin

waveform80 mannequin commented Jan 13, 2017

BPO 29270
Nosy @ncoghlan, @vstinner, @serhiy-storchaka, @eryksun, @https://github.com/hakril, @waveform80
Files
  • issue_29270_01.patch
  • 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 = None
    created_at = <Date 2017-01-13.22:10:22.119>
    labels = ['ctypes', 'type-bug', '3.9', '3.10', '3.11']
    title = 'ctypes: fail to create a _ctypes._SimpleCData subclass using a closure like calling super() without arguments'
    updated_at = <Date 2022-03-28.22:12:40.498>
    user = 'https://github.com/waveform80'

    bugs.python.org fields:

    activity = <Date 2022-03-28.22:12:40.498>
    actor = 'vstinner'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['ctypes']
    creation = <Date 2017-01-13.22:10:22.119>
    creator = 'Dave Jones'
    dependencies = []
    files = ['46285']
    hgrepos = []
    issue_num = 29270
    keywords = ['patch']
    message_count = 14.0
    messages = ['285444', '285450', '285454', '285469', '285473', '285474', '285475', '285480', '286287', '286310', '313985', '355368', '360252', '416219']
    nosy_count = 7.0
    nosy_names = ['ncoghlan', 'vstinner', 'serhiy.storchaka', 'eryksun', 'hakril', 'Dave Jones', 'Asdger Gdsfe']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'needs patch'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue29270'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    Linked PRs

    @waveform80
    Copy link
    Mannequin Author

    waveform80 mannequin commented Jan 13, 2017

    While investigating a bug report in one of my libraries (waveform80/picamera#355) I've come across a behaviour that appears in Python 3.6 but not prior versions. Specifically, calling super() in a sub-class of a ctypes scalar type appears to fail at the class definition stage. A minimal test case is as follows:

        import ctypes as ct
    
        class SuperTest(ct.c_uint32):
            def __repr__(self):
                return super().__repr__()

    This works happily under python 3.2, 3.4, and 3.5 (that I've tested), and also under 2.7 (with the appropriate modification to super's arguments). However, under 3.6 it elicits the following exception:

        Traceback (most recent call last):
          File "py36_ctypes.py", line 3, in <module>
            class SuperTest(ct.c_uint32):
        TypeError: __class__ set to <class '__main__.SuperTest'> defining 'SuperTest' as <class '__main__.SuperTest'>

    Reading through the "What's New" list in 3.6, I thought this might be something to do with the PEP-487 implementation (given it modified class construction), but having read through the PEP and associated patches I'm not so sure as I can't see anything that affects the setting of the "__class__" attribute (but don't rule it out on that basis; I'm no expert!).

    I'll admit that sub-classing one of ctypes' scalar types is a little odd, but given this works in prior versions and there doesn't appear to be anything in the documentation banning the practice (that I've found?) this might constitute a bug?

    @waveform80 waveform80 mannequin added topic-ctypes type-bug An unexpected behavior, bug, or error labels Jan 13, 2017
    @serhiy-storchaka serhiy-storchaka added the 3.7 (EOL) end of life label Jan 13, 2017
    @eryksun
    Copy link
    Contributor

    eryksun commented Jan 13, 2017

    In 3.6, type_new in Objects/typeobject.c sets the classcell in the dict if it's a cell object. It happens that CreateSwappedType in Modules/_ctypes/_ctypes.c re-uses the dict to create the swapped type (e.g. big endian), which in turn updates the classcell. Thus in builtin___build_class__ in Python/bltinmodule.c, the check cell_cls != cls ends up being true, which leads to the observed TypeError. CreateSwappedType should be able to avoid this by either deleting "classcell" from the dict or creating a copy without it before passing it to type_new.

    @eryksun
    Copy link
    Contributor

    eryksun commented Jan 14, 2017

    Here's a patch that deletes __classcell__ from the dict before calling type_new.

    @ncoghlan
    Copy link
    Contributor

    Eryk's diagnosis sounds right to me, and the suggested patch will get this back to working as well as it did in Python 3.5.

    However, it's worth noting that that state itself was already broken when it comes to zero-argument super() support on the type definition with reversed endianness:

    >>> import ctypes as ct
    >>> class SuperText(ct.c_uint32):
    ...     def __repr__(self):
    ...         return super().__repr__()
    ... 
    >>> SuperText.__ctype_le__(1)
    <SuperText object at 0x7fde526daea0>
    >>> SuperText.__ctype_be__(1)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "<stdin>", line 3, in __repr__
    TypeError: super(type, obj): obj must be an instance or subtype of type
    

    The apparently nonsensical error message comes from both the original type and the type with swapped endianness having the same representation:

    >>> SuperText.__ctype_le__
    <class '__main__.SuperText'>
    >>> SuperText.__ctype_be__
    <class '__main__.SuperText'>

    @waveform80
    Copy link
    Mannequin Author

    waveform80 mannequin commented Jan 14, 2017

    I confess I'm going to have to read a bit more about Python internals before I can understand Eryk's analysis (this is my first encounter with "cell objects"), but many thanks for the rapid analysis and patch!

    I'm not too concerned about the state being broken with reversed endianness; I don't think that's going to affect any of my use-cases in the near future, but it's certainly useful to know in case it does come up.

    @eryksun
    Copy link
    Contributor

    eryksun commented Jan 14, 2017

    OK, this is completely broken and needs a more thoughtful solution than my simpleminded hack. Here's a practical example of the problem, tested in 3.5.2:

        class MyInt(ctypes.c_int):
            def __repr__(self):
                return super().__repr__()
    
        class Struct(ctypes.BigEndianStructure):
            _fields_ = (('i', MyInt),)
        >>> s = Struct()
        >>> s.i
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
          File "<stdin>", line 3, in __repr__
        TypeError: super(type, obj): obj must be an instance or subtype of type

    @ncoghlan
    Copy link
    Contributor

    Yeah, re-using Python-level method objects in different types is genuinely invalid when combined with __class__ or zero-argument super(), as there's no way to make the __class__ closure refer to two different classes at runtime - it will always refer back to the original defining class.

    And while ctypes could be updated to do the right thing for functions, classmethod, staticmethod, abstractmethod, property, etc, there's nothing systematic it can do that would work for arbitrary descriptors (any of which may have a zero-argument-super-using method definition hidden inside their internal state).

    @eryksun
    Copy link
    Contributor

    eryksun commented Jan 14, 2017

    Resolving this would be straightforward if we could use a subclass for the swapped type, but ctypes simple types behave differently for subclasses. A simple subclass doesn't automatically call the getfunc to get a converted value when returned as a function result, struct field, or array index.

    @serhiy-storchaka
    Copy link
    Member

    See also similar issue with scrapy: scrapy-plugins/scrapy-djangoitem#18.

    @ncoghlan
    Copy link
    Contributor

    The scrapy case looks to just be the new metaclass constraint that's already covered in the "Porting to Python 3.6" guide: https://github.com/scrapy/scrapy/pull/2509/files

    The ctypes case is more complicated, as its actually *reusing* the same class namespace to define two different classes, which is fundamentally incompatible with the way zero-argument super works.

    @AsdgerGdsfe
    Copy link
    Mannequin

    AsdgerGdsfe mannequin commented Mar 17, 2018

    Hey 3.6 is pretty old now so can we get this patch merged I'd really like this code to start working again, appreciate all your hard work!

    class Something(ctypes.c_ulong):
      def __repr__(self):
        return super(Something, self).value
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: __class__ set to <class '__main__.Something'> defining 'Something' as <class '__main__.Something'>

    @httpsgithubcomhakril
    Copy link
    Mannequin

    httpsgithubcomhakril mannequin commented Oct 25, 2019

    Hello,

    I have a Python2 project that relies heavily on ctypes and I cannot migrate this project to Python3 due to this bug. (I target 3.6)
    I have some experience with CPython and submitting patchs and I would like to know what I can do to help moving this issue forward.

    Thanks !

    @httpsgithubcomhakril
    Copy link
    Mannequin

    httpsgithubcomhakril mannequin commented Jan 18, 2020

    Hello,

    As this issue may never be fixed for python3.6. I wanted to post a solution to bypass the bug. It may be useful for the next person stumbling on this as I have.

    The __class__ closure is only created if a function use the word super(). This closure allow to call super() without argument.

    By using another name than super() the closure is not created and your code can work. Only downside is that you need to call super in its explicit form super(Cls, self). But it is better that not working at all (and it is compatible python2).

    Here is a sample:

    super_bypass_issue29270 = super
    
    class Something(ctypes.c_ulong):
      def __repr__(self):
        return "BYPASS: " + super_bypass_issue29270(Something, self).__repr__()
    
    s = Something(42)
    print(s)

    BYPASS: <Something object at 0x00000134C9BA6848>

    @eryksun eryksun added 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes and removed 3.7 (EOL) end of life labels Mar 19, 2021
    @eryksun eryksun changed the title super call in ctypes sub-class fails in 3.6 super call in ctypes subclass fails Mar 19, 2021
    @vstinner
    Copy link
    Member

    CreateSwappedType() is an helper function used by the _ctypes.PyCSimpleType type. Python script reproducing this ctypes bug:
    ---

    class PyCSimpleType(type):
        def __new__(cls, name, bases, dct):
            print(f"PyCSimpleType: create {name} class")
            cell = dct.get('__classcell__', None)
            # type.__new__() sets __classcell__
            x = super().__new__(cls, name, bases, dct)
            if cell is not None:
                print(f"PyCSimpleType: after first type.__new__() call: __classcell__={cell.cell_contents}")
    
            name2 = name + "_Swapped"
            dct2 = dict(dct, __qualname__=name2)
            # Calling type.__new__() again with the same cell object overrides
            # __classcell__
            x.__ctype_be__ = super().__new__(cls, name2, bases, dct2)
            if cell is not None:
                print(f"PyCSimpleType: after second type.__new__() call: __classcell__={cell.cell_contents}")
    
            return x
    
    class BaseItem:
        pass
    
    class Item(BaseItem, metaclass=PyCSimpleType):
        def get_class(self):
            # get '__class__' to create a closure
            return __class__
    # Alternative to create a closure:
    #def __repr__(self):
    #    return super().__repr__()
    

    ---

    Output:
    ---

    PyCSimpleType: create Item class
    PyCSimpleType: after first type.__new__() call: __classcell__=<class '__main__.Item'>
    PyCSimpleType: after second type.__new__() call: __classcell__=<class '__main__.Item_Swapped'>
    Traceback (most recent call last):
      File "meta.py", line 23, in <module>
        class Item(BaseItem, metaclass=PyCSimpleType):
    TypeError: __class__ set to <class '__main__.Item_Swapped'> defining 'Item' as <class '__main__.Item'>

    It's not a bug in Python types, but a bug specific to the _ctypes.PyCSimpleType type which prevents creating subclasses which use closures ("__class__", "super()", etc.).

    @vstinner vstinner added 3.11 only security fixes and removed 3.8 only security fixes labels Mar 28, 2022
    @vstinner vstinner changed the title super call in ctypes subclass fails ctypes: fail to create a _ctypes._SimpleCData subclass using a closure like calling super() without arguments Mar 28, 2022
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @furkanonder
    Copy link
    Sponsor Contributor

    PR is ready for review.

    @serhiy-storchaka serhiy-storchaka removed the 3.10 only security fixes label Feb 1, 2024
    @serhiy-storchaka serhiy-storchaka added 3.12 bugs and security fixes 3.13 new features, bugs and security fixes and removed 3.9 only security fixes labels Feb 1, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes 3.12 bugs and security fixes 3.13 new features, bugs and security fixes topic-ctypes type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants