issubclass segfaults on objects with weird __getattr__

[DanielLepage]

BPO	30570
Nosy	@gpshead, @db3l, @cfbolz, @bitdancer, @ambv, @serhiy-storchaka, @nicoddemus, @DimitrisJim, @mlouielu, @pablogsal, @miss-islington, @sweeneyde, @iritkatriel
PRs	bpo-30570: fix an issubclass infinite recursion crash #29017 bpo-30570: Use Py_EnterRecursiveCall() in issubclass() #29048 [3.10] bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (GH-29048) #29175 [3.9] bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (GH-29048) #29178 bpo-30570: Fix segfault on buildbots caused by stack overflow from recursion in tests #29258 [3.10] bpo-30570: Fix segfault on buildbots caused by stack overflow from recursion in tests (GH-29258) #29414 [3.9] bpo-30570: Fix segfault on buildbots caused by stack overflow from recursion in tests (GH-29258) #29415
Files	segfault.crash: Crash Log

^{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 = 'https://github.com/gpshead'
closed_at = <Date 2021-11-04.20:53:21.931>
created_at = <Date 2017-06-05.00:25:34.296>
labels = ['interpreter-core', '3.10', '3.9', 'type-crash', '3.11']
title = 'issubclass segfaults on objects with weird __getattr__'
updated_at = <Date 2021-11-15.05:42:14.836>
user = 'https://bugs.python.org/DanielLepage'

bugs.python.org fields:

activity = <Date 2021-11-15.05:42:14.836>
actor = 'Dennis Sweeney'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2021-11-04.20:53:21.931>
closer = 'lukasz.langa'
components = ['Interpreter Core']
creation = <Date 2017-06-05.00:25:34.296>
creator = 'Daniel Lepage'
dependencies = []
files = ['46924']
hgrepos = []
issue_num = 30570
keywords = ['patch']
message_count = 22.0
messages = ['295150', '295151', '295152', '295153', '295179', '295181', '401381', '404151', '404152', '404157', '404158', '404829', '404831', '405144', '405173', '405750', '405753', '405754', '405755', '406328', '406330', '406340']
nosy_count = 14.0
nosy_names = ['gregory.p.smith', 'db3l', 'Carl.Friedrich.Bolz', 'r.david.murray', 'lukasz.langa', 'serhiy.storchaka', 'Bruno Oliveira', 'Daniel Lepage', 'Jim Fasarakis-Hilliard', 'louielu', 'pablogsal', 'miss-islington', 'Dennis Sweeney', 'iritkatriel']
pr_nums = ['29017', '29048', '29175', '29178', '29258', '29414', '29415']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue30570'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

[DanielLepage]

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.

[DanielLepage]

I tried it on python 2.7.12 and python 2.6.9 since I had them lying around and got the same behavior.

[mlouielu]

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

[mlouielu]

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.

[bitdancer]

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.

[serhiy-storchaka]

This looks as yet one example of issues similar of bpo-14010.

[iritkatriel]

Reproduced on 3.11.

[gpshead]

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.

[gpshead]

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.

[cfbolz]

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...

[sweeneyde]

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)

[gpshead]

New changeset 423fa1c by Dennis Sweeney in branch 'main':
bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (GH-29048)
423fa1c

[miss-islington]

New changeset f812fef by Miss Islington (bot) in branch '3.10':
bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (GH-29048)
f812fef

[sweeneyde]

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.

[pablogsal]

New changeset d56375a by Dennis Sweeney in branch 'main':
bpo-30570: Fix segfault on buildbots caused by stack overflow from recursion in tests (GH-29258)
d56375a

[ambv]

New changeset 1e29dce by Miss Islington (bot) in branch '3.9':
bpo-30570: Use Py_EnterRecursiveCall() in issubclass() (GH-29048) (GH-29178)
1e29dce

[miss-islington]

New changeset 1f3ae5c by Miss Islington (bot) in branch '3.10':
bpo-30570: Fix segfault on buildbots caused by stack overflow from recursion in tests (GH-29258)
1f3ae5c

[ambv]

New changeset f701237 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)
f701237

[ambv]

Thanks, Dennis! ✨ 🍰 ✨

[db3l]

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.

[db3l]

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.

[sweeneyde]

Since this isn't quite related to the original issue, I opened bpo-45806 to discuss.