This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: type_new() removes __qualname__ from the input dictionary
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: meador.inge, pitrou, python-dev, sbt, vstinner
Priority: normal Keywords: patch

Created on 2012-02-23 00:27 by vstinner, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
qualname_dict.patch pitrou, 2012-02-23 17:41 review
type_new.patch vstinner, 2012-02-23 20:08 review
type_new-2.patch vstinner, 2012-02-23 21:15 review
Messages (9)
msg154020 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-02-23 00:27
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.
msg154059 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-02-23 11:15
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
  {}
msg154075 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-02-23 17:41
Here is a patch.
(I can't reproduce the crash, btw)
msg154083 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2012-02-23 20:03
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))
msg154085 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-02-23 20:08
> 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).
msg154086 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2012-02-23 20:13
haypo, I can reproduce the segfault using the 'None' test case on your patch as well.
msg154089 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-02-23 21:15
I merged the two patches (qualname_dict.patch and type_new.patch) and added a fix for non-string __qualname__ => type_new-2.patch.
msg154140 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2012-02-24 15:59
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.
msg154176 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-02-25 00:34
New changeset b83ae75beaca by Victor Stinner in branch 'default':
Close #14095: type.__new__() doesn't remove __qualname__ key from the class
http://hg.python.org/cpython/rev/b83ae75beaca
History
Date User Action Args
2022-04-11 14:57:27adminsetgithub: 58303
2012-02-25 00:34:21python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg154176

resolution: fixed
stage: patch review -> resolved
2012-02-24 15:59:11meador.ingesetmessages: + msg154140
2012-02-23 21:15:21vstinnersetfiles: + type_new-2.patch

messages: + msg154089
2012-02-23 20:13:59meador.ingesetmessages: + msg154086
2012-02-23 20:08:48vstinnersetfiles: + type_new.patch

messages: + msg154085
2012-02-23 20:03:34meador.ingesetmessages: + msg154083
2012-02-23 17:41:30pitrousetfiles: + qualname_dict.patch
keywords: + patch
messages: + msg154075

stage: patch review
2012-02-23 15:57:09meador.ingesetnosy: + meador.inge
2012-02-23 11:15:15sbtsetnosy: + sbt
messages: + msg154059
2012-02-23 00:27:51vstinnersetcomponents: + Interpreter Core
2012-02-23 00:27:45vstinnercreate