Segfault on __getattr__ infinite recursion on certain attribute accesses #86075
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
assignee = None closed_at = <Date 2020-10-04.22:44:32.041> created_at = <Date 2020-10-02.10:49:51.390> labels = ['interpreter-core', '3.10', '3.8', '3.9', 'type-crash'] title = 'Segfault on __getattr__ infinite recursion on certain attribute accesses' updated_at = <Date 2020-10-04.22:44:32.041> user = 'https://github.com/bluetech'
activity = <Date 2020-10-04.22:44:32.041> actor = 'serhiy.storchaka' assignee = 'none' closed = True closed_date = <Date 2020-10-04.22:44:32.041> closer = 'serhiy.storchaka' components = ['Interpreter Core'] creation = <Date 2020-10-02.10:49:51.390> creator = 'bluetech' dependencies =  files =  hgrepos =  issue_num = 41909 keywords = ['patch'] message_count = 8.0 messages = ['377807', '377860', '377905', '377920', '377922', '377977', '377979', '377980'] nosy_count = 8.0 nosy_names = ['gvanrossum', 'loewis', 'nascheme', 'pitrou', 'serhiy.storchaka', 'The Compiler', 'bluetech', 'sparkingdark'] pr_nums = ['22536', '22550', '22551'] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'crash' url = 'https://bugs.python.org/issue41909' versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']
The text was updated successfully, but these errors were encountered:
The following program crashes with a segfault:
class Segfault: def __getattr__(self, name): self.unknown_attribute instance = Segfault() issubclass(instance, int) # int doesn't matter
Tested with Python 3.7, 3.8, 3.9rc2, and master in debug mode (commit 497126f).
The program is odd, but this affects pytest (see pytest-dev/pytest#2330).
Here is the backtrace with python master:
ran@ran:~/src/cpython$ gdb --args ./python segfault.py
Program received signal SIGSEGV, Segmentation fault.
(gdb) bt 29
(gdb) py-bt Traceback (most recent call first): File "/home/ran/src/cpython/segfault.py", line 3, in __getattr__ self.unknown_attribute File "/home/ran/src/cpython/segfault.py", line 3, in __getattr__ self.unknown_attribute File "/home/ran/src/cpython/segfault.py", line 3, in __getattr__ self.unknown_attribute File "/home/ran/src/cpython/segfault.py", line 3, in __getattr__ self.unknown_attribute File "/home/ran/src/cpython/segfault.py", line 3, in __getattr__ self.unknown_attribute File "/home/ran/src/cpython/segfault.py", line 3, in __getattr__ self.unknown_attribute File "/home/ran/src/cpython/segfault.py", line 3, in __getattr__ self.unknown_attribute File "/home/ran/src/cpython/segfault.py", line 3, in __getattr__ self.unknown_attribute --Type <RET> for more, q to quit, c to continue without paging--q
Interesting, when lookup any attribute you will get a RecursionError, but this is one of only two places where the recursion check is disabled (the other one is in interning strings).
You get a crash also when call isinstance(1, instance) or issubclass(int, instance). This is the same bug.
The implementation of issubclass() calls PyObject_IsSubclass() which calls object_issubclass() which calls recursive_issubclass() which calls check_class() which calls abstract_get_bases() which looks up attribute "__bases__" which leads to infinite recursion in __getattr__().
bpo-125278 0x000055555576116d in abstract_get_bases (cls=cls@entry=0x7fffeabcee10) at Objects/abstract.c:2340
The problem is that in abstract_get_bases() the recursion check is disabled by using macros Py_ALLOW_RECURSION/Py_END_ALLOW_RECURSION (added in 5b22213). I do not know why it was necessary. Currently tests are passed if enable recursion check, and this fixes this issue.
It is worth to mention that attribute __bases__ is looked up to support non-types in issubclass() and isinstance(). Originally it was added in bpo-464992 to support Zope extension ExtensionClass. I tested that the current code of ExtensionClass does not need it. So we could simplify the code and avoid recursion by just using tp_bases. But this needs wider discussion.
I don't think I remember what's so special about abstract_get_bases() to disable the recursion check. Maybe this macro is always just a micro-optimization? Then again it's hard to be sure, the behavior may depend on the platform.
Pulling in Antoine, who showed up in the email https://mail.python.org/pipermail/python-dev/2008-August/082106.html is mentioned in a long comment in ceval.h about the recursion protection.
Possibly the Py_ALLOW_RECURSION macro is intended to ensure that some handler code for first-order recursion errors doesn't get interrupted by second-order recursion errors? There's a limit of 50 stack frames which feels a bit dodgy.
Antoine give me a hint. The problem seems related to using PyDict_GetItem() when look up an attribute in the instance dict. It silences all exceptions (including RecursionError) and this results in silent "no such key in a dict".
But since 3.8 it was replaced with PyDict_GetItemWithError() which preserves RecursionError (see bpo-35459). I think we can remove Py_ALLOW_RECURSION/Py_END_ALLOW_RECURSION now.
As for other case in interning string, Py_ALLOW_RECURSION/Py_END_ALLOW_RECURSION was used to guard PyDict_GetItem(). But the current code uses PyDict_SetDefault() (added in 3.4) which does not silence errors. So Py_ALLOW_RECURSION/Py_END_ALLOW_RECURSION can be removed in that place too.