classification
Title: sys.flags.__new__ crashes
Type: crash Stage: resolved
Components: Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Trundle, amaury.forgeotdarc, eric.araujo, ezio.melotti, haypo, jesstess, pitrou, python-dev
Priority: normal Keywords: needs review, patch

Created on 2011-10-17 22:14 by Trundle, last changed 2014-04-28 11:09 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
sys_flags__new__crash.patch Trundle, 2011-10-17 22:14 review
sys_flags__new__crash_2.7.patch Trundle, 2011-10-17 22:15 review
issue13204.patch jesstess, 2014-04-28 03:36 review
Messages (12)
msg145763 - (view) Author: Andreas Stührk (Trundle) Date: 2011-10-17 22:14
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.
msg147089 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-05 15:59
I’m not sure it is useful to fix this bug.
msg147092 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-11-05 16:02
Why not?
msg147100 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-05 16:48
Because the class of sys.flags is an implementation detail.  Most people won’t try to instantiate it, IMO.
msg147103 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-11-05 17:10
That's probably true, but IMHO it's not a valid reason to keep the buggy behavior.
msg147106 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-11-05 20:14
Why would we want to prevent users from creating new instances of FlagsType?
msg147490 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-12 14:38
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.
msg147563 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-11-13 17:39
Thanks for the patch. You should add the same tests for sys.version_info and sys.getwindowsversion.
msg217338 - (view) Author: Jessica McKellar (jesstess) * Date: 2014-04-28 03:36
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).
msg217359 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-28 11:02
Thanks for the patch, Jessica. It seems to work under Windows here.
msg217360 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-28 11:08
New changeset 7052fdd90a11 by Antoine Pitrou in branch '3.4':
Issue #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 #13204: Calling sys.flags.__new__ would crash the interpreter, now it raises a TypeError.
http://hg.python.org/cpython/rev/a14012352f65
msg217362 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-28 11:09
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.
History
Date User Action Args
2014-04-28 11:09:58pitrousetstatus: open -> closed
resolution: fixed
messages: + msg217362

stage: patch review -> resolved
2014-04-28 11:08:35python-devsetnosy: + python-dev
messages: + msg217360
2014-04-28 11:02:19pitrousetmessages: + msg217359
versions: + Python 3.4
2014-04-28 03:36:15jesstesssetfiles: + issue13204.patch
versions: + Python 3.5, - Python 3.2, Python 3.3
nosy: + jesstess

messages: + msg217338

keywords: + needs review
2011-11-13 17:39:25pitrousetnosy: + pitrou

messages: + msg147563
stage: patch review
2011-11-12 14:38:29eric.araujosetmessages: + msg147490
2011-11-05 20:14:28amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg147106
2011-11-05 17:10:26ezio.melottisetmessages: + msg147103
2011-11-05 16:48:55eric.araujosetmessages: + msg147100
2011-11-05 16:02:22ezio.melottisetnosy: + ezio.melotti
messages: + msg147092
2011-11-05 15:59:02eric.araujosetnosy: + eric.araujo
messages: + msg147089
2011-10-17 23:55:56hayposetnosy: + haypo
2011-10-17 22:15:01Trundlesetfiles: + sys_flags__new__crash_2.7.patch
2011-10-17 22:14:25Trundlecreate