[issue3720] segfault in for loop with evil iterator
Terry J. Reedy tjre...@udel.edu added the comment: Too late for 2.6.6 ;-) -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Changes by Alexander Belopolsky belopol...@users.sourceforge.net: -- keywords: +26backport ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Guido van Rossum gu...@python.org added the comment: I'm okay with that hack for 2.6 and 2.5. -- nosy: +gvanrossum ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Benjamin Peterson benja...@python.org added the comment: I think PyObject_NextNotImplemented should be renamed to _PyObject_NextNotImplemented. Aside from that, I think the patch is ready for committing. -- nosy: +benjamin.peterson ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Gregory P. Smith g...@krypto.org added the comment: Any crash is a potential security problem. This needs to be fixed in 2.6.x without breaking 2.6.0 extension compatibility. (requiring extensions to be recompiled to fix the problem for an extension is fine, but they still need to load and work normally even if they have not been) As trunk r68560 can't be backported? what will? Is there an appropriate pre-existing hackish substitute for PyObj_NextNotImplemented or will it require adding a test anytime the tp_nextiter slot is used? -- components: +Interpreter Core nosy: +gregory.p.smith ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Amaury Forgeot d'Arc amaur...@gmail.com added the comment: Fixed in r68560 (trunk) and r68561 (py3k). Difficult to backport: extensions compiled with 2.6.x would not run on 2.6.0. -- keywords: -needs review resolution: - fixed status: open - pending ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Amaury Forgeot d'Arc amaur...@gmail.com added the comment: Yes, a hack: What about setting tp_iternext to PyObject_GetIter? they happen to have the same signature. Yes, calling next() will call iter() instead; but an iterator is often its own iterator, and more importantly, PyIter_Check() is also called. And the error message is surprisingly almost correct: TypeError: iter() returned non-iterator of type 'BadIterator' It's not completely correct since the error occurred while calling the __next__() method of the iterator. See attached patch for 2.6. I can see one cause of incompatibility: if someone designed an extension type in C where tp_iternext is already PyObject_GetIter. It's is insane but valid, and the patch would break it. It's not worth the trouble I suppose... -- keywords: +needs review status: pending - open Added file: http://bugs.python.org/file12711/next-nevernull-26.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Gregory P. Smith g...@krypto.org added the comment: nice hack! :) I'm going to guess that existing code in the wild setting tp_iternext = PyObject_GetIter is rare. I certainly can not rule it entirely out but I don't see anything in the open source world using http://www.google.com/codesearch I'd be okay with this hack. Let the release manager and/or BDFL make this call. ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Changes by STINNER Victor [EMAIL PROTECTED]: -- nosy: +haypo ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Changes by Jesús Cea Avión [EMAIL PROTECTED]: -- nosy: +jcea ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Armin Rigo [EMAIL PROTECTED] added the comment: Maybe the file 'next-nevernull.patch' is not complete? I don't see any change in the definition of PyIter_Check(). ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Amaury Forgeot d'Arc [EMAIL PROTECTED] added the comment: Oops, sorry. I have too many files opened at the same time. Here is an updated patch. I removed all the assert(PyIter_Check(it)), our evil iterator used to make them fail anyway in debug mode; and in the case of a null function pointer, the C stack gives the same information. Added file: http://bugs.python.org/file11347/next-nevernull-2.patch ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Changes by Amaury Forgeot d'Arc [EMAIL PROTECTED]: Removed file: http://bugs.python.org/file11317/next-nevernull.patch ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Amaury Forgeot d'Arc [EMAIL PROTECTED] added the comment: Did you notice that the definition of PyIter_Check() also changed? class T(object): ... def __iter__(self): return self ... iter(T()) Traceback (most recent call last): File stdin, line 1, in module TypeError: iter() returned non-iterator of type 'T' Or did you refer to .so extensions modules that are not recompiled between 2.5 and 2.6? (I don't know if this still works) ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Armin Rigo [EMAIL PROTECTED] added the comment: Hacking at typeobject.c should always be done extremely carefully. I don't have time to review this patch as thouroughly as I think it needs to be. (A quick issue is that it seems to break PyIter_Check() which will always return true now, but I'd recommend a much more thourough review.) ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Amaury Forgeot d'Arc [EMAIL PROTECTED] added the comment: Amaury, if you decide to go forward with this, please clean-up the patches to eliminate common subexpressions. I already considered this, but generated machine code was identical, so I chose the more readable code. Wonder if there is an alternate solution that keeps the next slot filled with something that raises an Attribute error. That's an interesting idea, and not difficult to implement, see attached patch. The penalty here is an extra comparison in PyIter_Check(). And no change in the event loop... Armin's crashers now correctly raise a TypeError (and the final patch should convert them as unit tests) Added file: http://bugs.python.org/file11317/next-nevernull.patch ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Raymond Hettinger [EMAIL PROTECTED] added the comment: It was the ajaksu2 patches that needed clean-up. ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Raymond Hettinger [EMAIL PROTECTED] added the comment: The next-nevernull patch is much cleaner than I expected. Nice work. The assertion in abstract.c can be changed to: assert(iter==PyObject_NextNotImplemented || PyIter_Check(iter)); Armin, are you happy with the new approach? Though del obj.next isn't doing exactly what you would expect, that seems harmless to me. ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Amaury Forgeot d'Arc [EMAIL PROTECTED] added the comment: Good catch! Here is a patch for 3.0. -- keywords: +needs review, patch nosy: +amaury.forgeotdarc priority: - release blocker Added file: http://bugs.python.org/file11303/baditer.patch ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Ismail Donmez [EMAIL PROTECTED] added the comment: Maybe do a s/object is no more an iterator/is no longer an iterator -- nosy: +cartman ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Changes by Amaury Forgeot d'Arc [EMAIL PROTECTED]: Added file: http://bugs.python.org/file11305/baditer.patch ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Changes by Amaury Forgeot d'Arc [EMAIL PROTECTED]: Removed file: http://bugs.python.org/file11303/baditer.patch ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Amaury Forgeot d'Arc [EMAIL PROTECTED] added the comment: New patches, with the suggested spelling. For 3.0 and 2.6. Added file: http://bugs.python.org/file11306/baditer-2.6.patch ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Raymond Hettinger [EMAIL PROTECTED] added the comment: It would be a damned shame to slow down the entire language to save code that is intentionally shooting itself in the foot. EVERYONE will pay a cost -- NO ONE will benefit. My two cents. -- nosy: +rhettinger ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Amaury Forgeot d'Arc [EMAIL PROTECTED] added the comment: Here are some timings, on winXP, vs2008 release build: # t.py def f(l=range(5000)): for x in l: pass # before the patch python -m timeit -s from t import f f() 1 loops, best of 3: 159 usec per loop # after the patch python -m timeit -s from t import f f() 1 loops, best of 3: 160 usec per loop and these figures are pretty stable on my machine. Is it too much to pay? Some may consider that potential crashes are more expensive. ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Raymond Hettinger [EMAIL PROTECTED] added the comment: Go for it. There's really no question about fixing possible segfaulters. Was just voicing my displeasure at this sort of exercise. The code has been around since at least 2.2 and hasn't caused the slightest problem. It bugs me to put cruft in the middle of otherwise tight loops. Fortunately, the check is cheap and branch predictable. BTW, your timings are domained by the function call and the range(5000) step. To cleanly time for-loop overhead, use: python -m timeit pass ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Armin Rigo [EMAIL PROTECTED] added the comment: The same approach can be used to segfault many more places. See http://svn.python.org/projects/python/trunk/Lib/test/crashers/iter.py . -- nosy: +arigo ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Daniel Diniz [EMAIL PROTECTED] added the comment: This patch fixes Armin's list of crashers for trunk. Looking for others like them. -- nosy: +ajaksu2 versions: +Python 2.6 Added file: http://bugs.python.org/file11311/itercrashers.diff ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Changes by Daniel Diniz [EMAIL PROTECTED]: Removed file: http://bugs.python.org/file11311/itercrashers.diff ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Daniel Diniz [EMAIL PROTECTED] added the comment: Hopefully the right patch this time :/ Added file: http://bugs.python.org/file11312/itercrashers.diff ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Raymond Hettinger [EMAIL PROTECTED] added the comment: Amaury, if you decide to go forward with this, please clean-up the patches to eliminate common subexpressions. Wonder if there is an alternate solution that keeps the next slot filled with something that raises an Attribute error. ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Terry J. Reedy [EMAIL PROTECTED] added the comment: I share Raymond's annoyance. The ultimate solution for segfaults is for bad pointer references to be catchable (trappable) the same as math errors are are now. I remember when 1/0 was fatal, not EDOM. Then the interpreter could print a traceback and SegfaultException message ;-) For this problem, I think there are alternatives to the current patch, which as Armin points out, would need to be extended to every built-in use of iterators. Is there any (sensible) way to make .__next__ (or the underlying tp_iternext slot) undelete-able? We already cannot change methods of built-ins. Or replace with def __next__(self): raise StopIteration. Or could iter() temporarily lock iterators in some way similar to what happens to dict during iteration? (Modifying actually does the action, and then raises RuntimeError). Modifying an active iterator strikes me as even less sane than modifying an underlying dict. The basic problem is that C code (unnecessarily?) does the same pointer tracing each iteration. Why not calculate np = *v-ob_type-tp_iternext just once? and just (more quickly) call np(v) each iteration? One could claim this is a semantic change, but so was the change to dicts. The equivalent in Python would be class BadIterator() : def __iter__(self) : return self def __next__(self) : # should be __next__ for python 3.0 del BadIterator.__next__ return 1 itnext = BadIterator().__next__ print(itnext()) print(itnext()) The second itnext call only fails because the del statement fails with AttributeError: __next__. I presume it would do the same if called from C. (My annoyance with Py3 changing .next to .__next__ is that it makes it harder to do the 'right thing' when iterating explicitly, which to me it to use a bound method. I wish next(it) returned it.__next__ instead of calling it.) -- nosy: +tjreedy ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Daniel Diniz [EMAIL PROTECTED] added the comment: Raymond, I think a different solution would be great, as the performance penalty might become nasty in tight loops if we miss some detail. Regarding the possible impact, I hope we can get a better estimate since the other examples of segfaulting that look like Armin's I've found are in itertools. I imagine you have the right tests to check the effect of any changes there. ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
Raymond Hettinger [EMAIL PROTECTED] added the comment: It's not just performance -- the patch code is grotesque. Lots of code would need to be changed just before the release candidate (usually a bad idea). And the underlying problem doesn't seem to be one that has *ever* affected a real user since 2.2. I have a hard time caring much about this problem. The minor performance hit only bugs me because it affects the inner- loops of just about every piece of real python code -- everyone will pay a cost for this change. Since the problem has existed so long with no ill effect, am unmarking the release blocker priority. Would rather have a thoughtful discussion on alternatives along with a careful, thorough, efficient patch for a bug release version. -- priority: release blocker - normal ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3720] segfault in for loop with evil iterator
New submission from Gideon Smeding [EMAIL PROTECTED]: The attached example crashes python. The crash is caused by an evil iterator that removes its own next function. -- files: baditerator.py messages: 72119 nosy: gideon severity: normal status: open title: segfault in for loop with evil iterator type: crash versions: Python 2.5, Python 3.0 Added file: http://bugs.python.org/file11300/baditerator.py ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue3720 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
