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

type_new() removes __qualname__ from the input dictionary #58303

Closed
vstinner opened this issue Feb 23, 2012 · 9 comments
Closed

type_new() removes __qualname__ from the input dictionary #58303

vstinner opened this issue Feb 23, 2012 · 9 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@vstinner
Copy link
Member

BPO 14095
Nosy @pitrou, @vstinner, @meadori
Files
  • qualname_dict.patch
  • type_new.patch
  • type_new-2.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 = <Date 2012-02-25.00:34:21.249>
    created_at = <Date 2012-02-23.00:27:44.996>
    labels = ['interpreter-core']
    title = 'type_new() removes __qualname__ from the input dictionary'
    updated_at = <Date 2012-02-25.00:34:21.247>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2012-02-25.00:34:21.247>
    actor = 'python-dev'
    assignee = 'none'
    closed = True
    closed_date = <Date 2012-02-25.00:34:21.249>
    closer = 'python-dev'
    components = ['Interpreter Core']
    creation = <Date 2012-02-23.00:27:44.996>
    creator = 'vstinner'
    dependencies = []
    files = ['24616', '24619', '24620']
    hgrepos = []
    issue_num = 14095
    keywords = ['patch']
    message_count = 9.0
    messages = ['154020', '154059', '154075', '154083', '154085', '154086', '154089', '154140', '154176']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'vstinner', 'meador.inge', 'python-dev', 'sbt']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue14095'
    versions = ['Python 3.3']

    @vstinner
    Copy link
    Member Author

    The C function type_new() creates a copy the dictionary for __dict__ and modifies the copy... except for __qualname__: it does modify the input dictionary before the copy.
    ---

    def f(): pass
    
    d = {'__qualname__': 42, '__new__': f}
    assert d['__new__'] is f
    assert '__qualname__' in d
    Enum = type.__new__(type, 'Enum', (), d)
    assert d['__new__'] is f
    assert '__qualname__' in d

    I don't know if it is expected. If not, the copy should be done before.

    @vstinner vstinner added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Feb 23, 2012
    @sbt
    Copy link
    Mannequin

    sbt mannequin commented Feb 23, 2012

    I get a segfault with

      Python 3.3.0a0 (default:31784350f849, Feb 23 2012, 11:07:41)
      [GCC 4.5.2] on linux
      Type "help", "copyright", "credits" or "license" for more information.
      >>> d = {'__qualname__':'XXX'}
      >>> T = type('foo', (), d)
      >>> d
      Segmentation fault

    On Windows I also get a crash. Wierdly, if I replace 'XXX' by 'foo',
    there is no segfault, but d is empty

      Python 3.3.0a0 (default:31784350f849, Feb 23 2012, 11:07:41)
      [GCC 4.5.2] on linux
      Type "help", "copyright", "credits" or "license" for more information.
      >>> d = {'__qualname__':'foo'}
      >>> T = type('foo', (), d)
      >>> d
      {}

    @pitrou
    Copy link
    Member

    pitrou commented Feb 23, 2012

    Here is a patch.
    (I can't reproduce the crash, btw)

    @meadori
    Copy link
    Member

    meadori commented Feb 23, 2012

    I can reproduce the segfault on F16:

    Python 3.3.0a0 (default:3828d93fd330, Feb 23 2012, 13:49:57) 
    [GCC 4.4.6] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> d = {'__qualname__':'XXX'}
    [66934 refs]
    >>> T = type('foo', (), d)
    [66956 refs]
    >>> T
    python: Objects/unicodeobject.c:301: _PyUnicode_CheckConsistency: Assertion `((((((PyObject*)(op))->ob_type))->tp_flags & ((1L<<28))) != 0)' failed.
    Aborted (core dumped)

    The patch looks mostly OK, but I can still segfault the interpreter by using a '__qualname__' of 'None':

    Python 3.3.0a0 (default:3828d93fd330+, Feb 23 2012, 13:55:57) 
    [GCC 4.4.6] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> d = {'__qualname__':'XXX'}
    [67128 refs]
    >>> T = type('foo', (), d)
    [67156 refs]
    >>> T
    <class '__main__.XXX'>
    [67161 refs]
    >>> d = {'__qualname__': None}
    [67161 refs]
    >>> T = type('foo', (), d)
    [67185 refs]
    >>> T
    python: Objects/unicodeobject.c:301: _PyUnicode_CheckConsistency: Assertion `((((((PyObject*)(op))->ob_type))->tp_flags & ((1L<<28))) != 0)' failed.
    Aborted (core dumped)

    Try this additional unit test:

            d = {'__qualname__': None}
            tp = type('foo', (), d)
            self.assertEqual(tp.__qualname__, None)
            self.assertEqual(tp.__dict__['__qualname__'], None)
            self.assertEqual(d, {'__qualname__': None})
            self.assertTrue(repr(tp))

    @vstinner
    Copy link
    Member Author

    Here is a patch.

    Oh, I also wrote a patch :-) My patch fixes this issue but also many memory leaks in error cases.

    (I can't reproduce the crash, btw)

    I can reproduce the crash, but only with Python compiled in release mode (not with Python compiled in debug mode).

    @meadori
    Copy link
    Member

    meadori commented Feb 23, 2012

    haypo, I can reproduce the segfault using the 'None' test case on your patch as well.

    @vstinner
    Copy link
    Member Author

    I merged the two patches (qualname_dict.patch and type_new.patch) and added a fix for non-string __qualname__ => type_new-2.patch.

    @meadori
    Copy link
    Member

    meadori commented Feb 24, 2012

    The change in error handling makes this a bit harder to review, but it otherwise looks OK if this is the intended behavior. I am not sure that it is.

    The original version:

    1. If __qualname__ was present in the original dictionary,
      then it was deleted.
    2. If __qualname__ was present in the original dictionary,
      then the qualname slot in the new type was left unitialized.

    Why (1) was done I don't know. (2) is obviously a bug.

    The patched version:

    1. Sets the slot qualname to the __qualname__ from the original
      dictionary (if present).
    2. Copies the __qualname__ attribute from the original dictionary
      to the new dictionary (if present).
    3. Leaves the original dictionary alone.

    The deletion and unitiliazed slot problems are gone, but I am not sure if (2) is needed. Just fixing (1) and (3) seems more reasonable to me.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 25, 2012

    New changeset b83ae75beaca by Victor Stinner in branch 'default':
    Close bpo-14095: type.__new__() doesn't remove __qualname__ key from the class
    http://hg.python.org/cpython/rev/b83ae75beaca

    @python-dev python-dev mannequin closed this as completed Feb 25, 2012
    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants