sys.flags.__new__ crashes

[Trundle]

BPO	13204
Nosy	@amauryfa, @pitrou, @vstinner, @ezio-melotti, @merwok, @Trundle
Files	sys_flags__new__crash.patch sys_flags__new__crash_2.7.patch issue13204.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 2014-04-28.11:09:58.458>
created_at = <Date 2011-10-17.22:14:25.187>
labels = ['type-crash']
title = 'sys.flags.__new__ crashes'
updated_at = <Date 2014-04-28.11:09:58.457>
user = 'https://github.com/Trundle'

bugs.python.org fields:

activity = <Date 2014-04-28.11:09:58.457>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2014-04-28.11:09:58.458>
closer = 'pitrou'
components = []
creation = <Date 2011-10-17.22:14:25.187>
creator = 'Trundle'
dependencies = []
files = ['23431', '23432', '35068']
hgrepos = []
issue_num = 13204
keywords = ['patch', 'needs review']
message_count = 12.0
messages = ['145763', '147089', '147092', '147100', '147103', '147106', '147490', '147563', '217338', '217359', '217360', '217362']
nosy_count = 8.0
nosy_names = ['amaury.forgeotdarc', 'pitrou', 'vstinner', 'ezio.melotti', 'eric.araujo', 'Trundle', 'jesstess', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue13204'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

[Trundle]

It's not possible (by intention) to instantiate a new instance of sys.flags. This is achieved by setting the "tp_new" slot to NULL (in _PySys_Init()), after PyType_Ready() is called, which means that a slot wrapper is added to the type dict for the "tp_new" slot (because the slot != NULL at that time). The problem is now that if one calls sys.flags.__new__ directly, a null pointer dereference occurs in tp_new_wrapper().

Attached is a patch that fixes the crash and adds a test.

[merwok]

I’m not sure it is useful to fix this bug.

[ezio-melotti]

Why not?

[merwok]

Because the class of sys.flags is an implementation detail. Most people won’t try to instantiate it, IMO.

[ezio-melotti]

That's probably true, but IMHO it's not a valid reason to keep the buggy behavior.

[amauryfa]

Why would we want to prevent users from creating new instances of FlagsType?

[merwok]

You are right. Even if it’s an undocumented internal type, there is no reason not to fix it. There are plenty of similar crash fixes committed in the repo.

[pitrou]

Thanks for the patch. You should add the same tests for sys.version_info and sys.getwindowsversion.

[jesstess]

Thanks for reporting this and providing a patch, Trundle.

The Python 3 patch didn't apply cleanly anymore, so I regenerated it and added tests for sys.version_info and sys.getwindowsversion.

• The patch passes make patchcheck
• The full test suite passes with this patch.
• Testing manually, without this patch each of the following segfaults, and with the patch they raise errors instead:

import sys; sys.flags.__new__(type(sys.flags))
import sys; sys.version_info.__new__(type(sys.version_info))
import sys; sys.getwindowsversion().__new__(type(sys.getwindowsversion()))

One important caveat is that while I confirmed the sys.getwindowsversion segfault on Windows, I don't have Visual Studio set up so I couldn't build and test the new test for sys.getwindowsversion (I ran the full test suite on OSX, where this test is skipped).

[pitrou]

Thanks for the patch, Jessica. It seems to work under Windows here.

[python-dev]

New changeset 7052fdd90a11 by Antoine Pitrou in branch '3.4':
Issue bpo-13204: Calling sys.flags.__new__ would crash the interpreter, now it raises a TypeError.
http://hg.python.org/cpython/rev/7052fdd90a11

New changeset a14012352f65 by Antoine Pitrou in branch 'default':
Issue bpo-13204: Calling sys.flags.__new__ would crash the interpreter, now it raises a TypeError.
http://hg.python.org/cpython/rev/a14012352f65

[pitrou]

I've committed the patch to 3.4 and 3.5. I'm closing the issue, I don't think fixing 2.7 is important at this point.