classification
Title: issubclass segfaults on objects with weird __getattr__
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: Bruno Oliveira, Carl.Friedrich.Bolz, Daniel Lepage, Dennis Sweeney, Jim Fasarakis-Hilliard, db3l, gregory.p.smith, iritkatriel, louielu, lukasz.langa, miss-islington, pablogsal, r.david.murray, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2017-06-05 00:25 by Daniel Lepage, last changed 2021-11-15 05:42 by Dennis Sweeney. This issue is now closed.

Files
File name Uploaded Description Edit
segfault.crash Daniel Lepage, 2017-06-05 00:25 Crash Log
Pull Requests
URL Status Linked Edit
PR 29017 closed gregory.p.smith, 2021-10-18 01:53
PR 29048 merged Dennis Sweeney, 2021-10-19 00:59
PR 29175 merged miss-islington, 2021-10-22 21:24
PR 29178 merged miss-islington, 2021-10-22 23:05
PR 29258 merged Dennis Sweeney, 2021-10-28 01:54
PR 29414 merged miss-islington, 2021-11-04 20:20
PR 29415 merged lukasz.langa, 2021-11-04 20:24
Messages (22)
msg295150 - (view) Author: Daniel Lepage (Daniel Lepage) Date: 2017-06-05 00:25
The following code causes a segmentation fault:
class Failure(object):
    def __getattr__(self, attr):
        return (self, None)
issubclass(Failure(), int)

I am running a macbook pro, OS X 10.12.4, and have observed the problem in python 3.5.2, 3.6.0, and 3.6.1.

It appears that returning (self,) causes it to go into an infinite loop attempting to get `x.__bases__`, and returning `(self, y)` for any value `y` causes it to attempt to get `x.__bases__` 262,030 times and then segfault.

A crash log is attached.
msg295151 - (view) Author: Daniel Lepage (Daniel Lepage) Date: 2017-06-05 00:26
I tried it on python 2.7.12 and python 2.6.9 since I had them lying around and got the same behavior.
msg295152 - (view) Author: Louie Lu (louielu) * Date: 2017-06-05 02:24
I can reproduce this bugs on 3.7, Linux.

Python 3.7.0a0 (heads/master:d3bedf356a, Jun  5 2017, 10:21:52) 
[GCC 6.3.1 20170306] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> class Foo(object):
...  def __getattr__(self, attr):
...   return (self, None)
... 
>>> issubclass(Foo(), int)

[1]    21897 segmentation fault (core dumped)  ./python
msg295153 - (view) Author: Louie Lu (louielu) * Date: 2017-06-05 02:31
Without this segfault, I think you do a wrong operation. In other cases, for example:

>>> issubclass(10, int)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: issubclass() arg 1 must be a class


`issubclass` will return TypeError on instance, if you test as `issubclass(Failure, int)` or `issubclass(Failure().__class__, int`, it should get the correct answer.
msg295179 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2017-06-05 12:03
Yes, but we try to make it not possible to segfault the interpreter with pure python code that isn't intentionally mucking with C-level stuff, so this is a bug we would like to fix.
msg295181 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-05 12:27
This looks as yet one example of issues similar of issue14010.
msg401381 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-09-08 13:21
Reproduced on 3.11.
msg404151 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2021-10-18 00:48
This is a stack overflow in Objects/abstract.c because we infinitely recurse within abstract_issubclass().

We should probably do some validity checking on the bases tuple that abstract_get_bases returns (ideally within abstract_get_bases?).  The tuple must contain types.
msg404152 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2021-10-18 01:06
Python metaprogramming allows type-like things to be bases, not just things that pass PyType_Check().  so being that strict isn't going to work.

The other classic way to prevent this is to track what you're recursing on to avoid a loop.  i.e. Keeping a set of PyObjects that have been seen and not recursing if the value is in that.
msg404157 - (view) Author: Carl Friedrich Bolz-Tereick (Carl.Friedrich.Bolz) * Date: 2021-10-18 07:10
PyPy raises a RecursionError here, which sounds like an ok outcome. So simply checking for the recursion would also be a way of fixing this...
msg404158 - (view) Author: Dennis Sweeney (Dennis Sweeney) * (Python triager) Date: 2021-10-18 07:52
Oh yeah -- Py_EnterRecursiveCall/Py_LeaveRecursiveCall in abstract_get_bases would be simpler. Also, the set approach also probably still c-stack overflows on

class C:
    def __getattr__(self, attr):
        class A: pass
        class B: pass
        A.__getattr__ = B.__getattr__ = C.__getattr__
        return (A(), B())

issubclass(C(), int)
msg404829 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2021-10-22 21:24
New changeset 423fa1c1817abfa8c3d1bc308ddbbd8f28b69d68 by Dennis Sweeney in branch 'main':
bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (GH-29048)
https://github.com/python/cpython/commit/423fa1c1817abfa8c3d1bc308ddbbd8f28b69d68
msg404831 - (view) Author: miss-islington (miss-islington) Date: 2021-10-22 21:47
New changeset f812fef2f8f12441ce559335645433c8124e7db5 by Miss Islington (bot) in branch '3.10':
bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (GH-29048)
https://github.com/python/cpython/commit/f812fef2f8f12441ce559335645433c8124e7db5
msg405144 - (view) Author: Dennis Sweeney (Dennis Sweeney) * (Python triager) Date: 2021-10-28 01:56
I think this broke some buildbots.

https://buildbot.python.org/all/#/builders/256/builds/264
https://buildbot.python.org/all/#/builders/370/builds/263

I opened a PR to temporarily decrease the recursion limit so that the C stack doesn't overflow in these new test.
msg405173 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-10-28 10:11
New changeset d56375a0dd4cee162081b173310298a3d32af293 by Dennis Sweeney in branch 'main':
bpo-30570: Fix segfault on buildbots caused by stack overflow from recursion in tests (GH-29258)
https://github.com/python/cpython/commit/d56375a0dd4cee162081b173310298a3d32af293
msg405750 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-11-04 20:20
New changeset 1e29dce1138a39e095ba47ab4c1e445fd08716e2 by Miss Islington (bot) in branch '3.9':
bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (GH-29048) (GH-29178)
https://github.com/python/cpython/commit/1e29dce1138a39e095ba47ab4c1e445fd08716e2
msg405753 - (view) Author: miss-islington (miss-islington) Date: 2021-11-04 20:45
New changeset 1f3ae5c1ca5a8e7696bad414c1de38e0f5f1e2c3 by Miss Islington (bot) in branch '3.10':
bpo-30570: Fix segfault on buildbots caused by stack overflow from recursion in tests (GH-29258)
https://github.com/python/cpython/commit/1f3ae5c1ca5a8e7696bad414c1de38e0f5f1e2c3
msg405754 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-11-04 20:52
New changeset f701237db2611140e578cebbdfef91ae4714af4e by Łukasz Langa in branch '3.9':
[3.9] bpo-30570: Fix segfault on buildbots caused by stack overflow from recursion in tests (GH-29258) (GH-29415)
https://github.com/python/cpython/commit/f701237db2611140e578cebbdfef91ae4714af4e
msg405755 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-11-04 20:53
Thanks, Dennis! ✨ 🍰 ✨
msg406328 - (view) Author: David Bolen (db3l) * Date: 2021-11-14 17:41
I don't know if this is a buildbot, test or 3.9-specific issue but this commit appears to have introduced a permanent initial failure (but success on retry) in test_pickle on both Windows 10 3.9 builders.  First failure for my builder at https://buildbot.python.org/all/#/builders/425/builds/450

Fatal Python error: _Py_CheckRecursiveCall: Cannot recover from stack overflow.
0:14:47 load avg: 4.57 Re-running failed tests in verbose mode
0:14:47 load avg: 4.57 Re-running test_pickle in verbose mode

The 3.x and 3.10 builders seem fine, and the second try on 3.9 always seems to succeed (perhaps because it's just a single test running at that point), so this is only being reported as a buildbot warning.
msg406330 - (view) Author: David Bolen (db3l) * Date: 2021-11-14 18:56
So I'm guessing something is just borderline under 3.9 on Windows.

In some manual testing with a standalone build of 3.9 so far for me:
  -m test.test_pickle         always succeeds (executed directly)
  -m test test_pickle         always fails (executed via test module)
  -m test -w -j1 test_pickle  fails, but succeeds on retry

The failures seem to always occur in CPicklerTests.test_bad_getattr.  I'm not sure how to run that single test via the test module, but limiting to all CPicklerTests tests or all test_bad_getattr tests succeeds even through the test module.

The last scenario above (successful retry) has to use -j or else no retry (-w) takes place.  That's the path the buildbots are following.
msg406340 - (view) Author: Dennis Sweeney (Dennis Sweeney) * (Python triager) Date: 2021-11-15 05:42
Since this isn't quite related to the original issue, I opened bpo-45806 to discuss.
History
Date User Action Args
2021-11-15 05:42:14Dennis Sweeneysetmessages: + msg406340
2021-11-14 18:56:00db3lsetmessages: + msg406330
2021-11-14 17:41:01db3lsetnosy: + db3l
messages: + msg406328
2021-11-04 20:53:21lukasz.langasetstatus: open -> closed
resolution: fixed
messages: + msg405755

stage: patch review -> resolved
2021-11-04 20:52:38lukasz.langasetmessages: + msg405754
2021-11-04 20:45:14miss-islingtonsetmessages: + msg405753
2021-11-04 20:24:16lukasz.langasetpull_requests: + pull_request27667
2021-11-04 20:20:25lukasz.langasetnosy: + lukasz.langa
messages: + msg405750
2021-11-04 20:20:09miss-islingtonsetpull_requests: + pull_request27666
2021-10-28 10:11:25pablogsalsetnosy: + pablogsal
messages: + msg405173
2021-10-28 01:56:48Dennis Sweeneysetmessages: + msg405144
2021-10-28 01:54:43Dennis Sweeneysetpull_requests: + pull_request27522
2021-10-22 23:05:47miss-islingtonsetpull_requests: + pull_request27451
2021-10-22 21:47:03miss-islingtonsetmessages: + msg404831
2021-10-22 21:24:18miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request27448
2021-10-22 21:24:16gregory.p.smithsetmessages: + msg404829
2021-10-19 00:59:09Dennis Sweeneysetpull_requests: + pull_request27319
2021-10-18 07:52:49Dennis Sweeneysetnosy: + Dennis Sweeney
messages: + msg404158
2021-10-18 07:10:04Carl.Friedrich.Bolzsetnosy: + Carl.Friedrich.Bolz
messages: + msg404157
2021-10-18 01:53:22gregory.p.smithsetkeywords: + patch
stage: patch review
pull_requests: + pull_request27291
2021-10-18 01:06:31gregory.p.smithsetmessages: + msg404152
2021-10-18 00:48:55gregory.p.smithsetassignee: gregory.p.smith
2021-10-18 00:48:46gregory.p.smithsetnosy: + gregory.p.smith
messages: + msg404151
2021-09-08 13:21:00iritkatrielsetnosy: + iritkatriel

messages: + msg401381
versions: + Python 3.9, Python 3.10, Python 3.11, - Python 2.7, Python 3.5, Python 3.6, Python 3.7
2017-06-09 12:42:34Jim Fasarakis-Hilliardsetnosy: + Jim Fasarakis-Hilliard
2017-06-05 12:27:43serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg295181
2017-06-05 12:03:56r.david.murraysetnosy: + r.david.murray
messages: + msg295179
2017-06-05 02:31:55louielusetmessages: + msg295153
2017-06-05 02:24:27louielusetnosy: + louielu

messages: + msg295152
versions: + Python 3.7
2017-06-05 02:09:06Bruno Oliveirasetnosy: + Bruno Oliveira
2017-06-05 00:26:29Daniel Lepagesetmessages: + msg295151
2017-06-05 00:25:47Daniel Lepagesetversions: + Python 2.7
2017-06-05 00:25:34Daniel Lepagecreate