Segfault when raising MemoryError

[hoefling]

BPO	41654
Nosy	@vstinner, @corona10, @pablogsal, @miss-islington, @shihai1991, @hoefling
PRs	bpo-41654: Fix deallocator of MemoryError to account for subclasses #22020 [3.9] bpo-41654: Fix deallocator of MemoryError to account for subclasses (GH-22020) #22045 [3.8] bpo-41654: Fix deallocator of MemoryError to account for subclasses (GH-22020) #22046 [3.8] bpo-41654: Explicitly cast PyExc_MemoryError to PyTypeObject to avoid warning #22102 bpo-41654: Fix compiler warning in MemoryError_dealloc() #22387 [3.9] bpo-41654: Fix compiler warning in MemoryError_dealloc() (GH-22387) #24894
Files	bug.py

^{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 = None
closed_at = <Date 2020-09-01.20:42:29.365>
created_at = <Date 2020-08-28.10:15:59.130>
labels = ['interpreter-core', '3.8', '3.9', '3.10', '3.7', 'type-crash']
title = 'Segfault when raising MemoryError'
updated_at = <Date 2021-03-16.17:36:50.846>
user = 'https://github.com/hoefling'

bugs.python.org fields:

activity = <Date 2021-03-16.17:36:50.846>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2020-09-01.20:42:29.365>
closer = 'pablogsal'
components = ['Interpreter Core']
creation = <Date 2020-08-28.10:15:59.130>
creator = 'hoefling'
dependencies = []
files = ['49431']
hgrepos = []
issue_num = 41654
keywords = ['patch']
message_count = 13.0
messages = ['376028', '376030', '376031', '376119', '376128', '376203', '376209', '376210', '376222', '376372', '376433', '377424', '388863']
nosy_count = 6.0
nosy_names = ['vstinner', 'corona10', 'pablogsal', 'miss-islington', 'shihai1991', 'hoefling']
pr_nums = ['22020', '22045', '22046', '22102', '22387', '24894']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue41654'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10']

[hoefling]

First of all, I guess this is a somewhat obscure error that is unlikely to occur in a usual context, nevertheless IMO worth reporting. We observed this when unit-testing custom exception reporting mechanism, raising different exceptions in different contexts and then analyzing whether they are processed correctly.

This is a somewhat dull example I managed to extract from our tests:

from pathlib import Path
from unittest.mock import patch


class TestException(MemoryError):
    pass


class report_ctx:
    def __enter__(self):
        return self
    def __exit__(self, exc_type, exc_value, tb):
        report(exc_value)


class raises:
    def __init__(self, ex):
        self.ex = ex
    def __enter__(self):
        return self
    def __exit__(self, exc_type, exc_value, tb):
        return issubclass(exc_type, self.ex)


def report(ex):
    pass


def error():
    raise MemoryError


modname = Path(__file__).stem

for _ in range(10):
    with patch(f"{modname}.report"):
        with raises(MemoryError), report_ctx():
            raise MemoryError

        with raises(TestException):
            raise TestException

        with raises(MemoryError):
            error()

that yields:

Fatal Python error: Segmentation fault

Current thread 0x00007fcf0833b740 (most recent call first):
File "/home/oleg.hoefling/projects/private/python-memoryerror-segfault/main.py", line 38 in <module>
File "<frozen importlib._bootstrap>", line 228 in _call_with_frames_removed
File "<frozen importlib._bootstrap_external>", line 790 in exec_module
File "<frozen importlib._bootstrap>", line 680 in _load_unlocked
File "<frozen importlib._bootstrap>", line 986 in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 1007 in _find_and_load
File "/usr/lib64/python3.9/unittest/mock.py", line 1236 in _importer
File "/usr/lib64/python3.9/unittest/mock.py", line 1564 in <lambda>
File "/usr/lib64/python3.9/unittest/mock.py", line 1389 in __enter__
File "/home/oleg.hoefling/projects/private/python-memoryerror-segfault/main.py", line 36 in <module>

[hoefling]

If this is of any help, I've set up an example repository containing the snippet: https://github.com/hoefling/bpo-issue-41654

Here are the results of running the snippet in Travis with Python 3.{5-10} and Pypy 3.6: https://travis-ci.com/github/hoefling/bpo-issue-41654

[vstinner]

Aha, interesting bug report. I attached a simplified reproducer.

Output with a Python built in debug mode:
---
Objects/frameobject.c:590: _Py_NegativeRefcount: Assertion failed: object has negative ref count
<object at 0x25e8570 is freed>
Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

Current thread 0x00007efd7a2d0740 (most recent call first):
File "/home/vstinner/bug.py", line 29 in <module>
Abandon (core dumped)
---

[shihai1991]

Hm, Looks like we need check the double free risk of Py_DECREF().

[pablogsal]

The problem is that the deallocator of MemoryError class is not taking into account subclasses for the freelist management, but the allocator (tp_new) is.

[pablogsal]

New changeset 9b648a9 by Pablo Galindo in branch 'master':
bpo-41654: Fix deallocator of MemoryError to account for subclasses (GH-22020)
9b648a9

[pablogsal]

New changeset d14775d by Pablo Galindo in branch '3.9':
[3.9] bpo-41654: Fix deallocator of MemoryError to account for subclasses (GH-22020) (GH-22045)
d14775d

[pablogsal]

New changeset 77f4000 by Pablo Galindo in branch '3.8':
[3.8] [3.9] bpo-41654: Fix deallocator of MemoryError to account for subclasses (GH-22020) (GH-22046)
77f4000

[vstinner]

Thanks for the fix Pablo! Thanks for the funny bug report Oleg Hoefling! I didn't expect that anyone would ever subclass MemoryError.

[vstinner]

"AMD64 Arch Linux Asan 3.8" buildbot logs a compiler warning:
https://buildbot.python.org/all/#builders/580/builds/4

Objects/exceptions.c: In function ‘MemoryError_dealloc’:
Objects/exceptions.c:2298:23: warning: comparison of distinct pointer types lacks a cast
2298 | if (Py_TYPE(self) != PyExc_MemoryError) {
| ^~

[pablogsal]

New changeset 6ae6195 by Pablo Galindo in branch '3.8':
bpo-41654: Explicitly cast PyExc_MemoryError to PyTypeObject to avoid warning (GH-22102)
6ae6195

[vstinner]

New changeset bbeb223 by Victor Stinner in branch 'master':
bpo-41654: Fix compiler warning in MemoryError_dealloc() (GH-22387)
bbeb223

[vstinner]

New changeset 1f0cde6 by Miss Islington (bot) in branch '3.9':
bpo-41654: Fix compiler warning in MemoryError_dealloc() (GH-22387) (GH-24894)
1f0cde6