Issue 41654: Segfault when raising MemoryError

Messages (13) msg376028 - (view) Author: Oleg Hoefling (hoefling) * Date: 2020-08-28 10:15

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>

msg376030 - (view) Author: Oleg Hoefling (hoefling) * Date: 2020-08-28 10:30

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

msg376031 - (view) Author: STINNER Victor (vstinner) * Date: 2020-08-28 11:50

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

msg376119 - (view) Author: Hai Shi (shihai1991) * Date: 2020-08-30 18:17

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

msg376128 - (view) Author: Pablo Galindo Salgado (pablogsal) * Date: 2020-08-30 19:43

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.

msg376203 - (view) Author: Pablo Galindo Salgado (pablogsal) * Date: 2020-09-01 18:40

New changeset 9b648a95ccb4c3b14f1e87158f5c9f5dbb2f62c0 by Pablo Galindo in branch 'master':
bpo-41654: Fix deallocator of MemoryError to account for subclasses (GH-22020)
https://github.com/python/cpython/commit/9b648a95ccb4c3b14f1e87158f5c9f5dbb2f62c0

msg376209 - (view) Author: Pablo Galindo Salgado (pablogsal) * Date: 2020-09-01 20:41

New changeset d14775ddbb067bcfa6eca516d3cbe968a8c1334e by Pablo Galindo in branch '3.9':
[3.9] bpo-41654: Fix deallocator of MemoryError to account for subclasses (GH-22020) (GH-22045)
https://github.com/python/cpython/commit/d14775ddbb067bcfa6eca516d3cbe968a8c1334e

msg376210 - (view) Author: Pablo Galindo Salgado (pablogsal) * Date: 2020-09-01 20:41

New changeset 77f4000ae0d43a2685face80e7f14d4aba053973 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)
https://github.com/python/cpython/commit/77f4000ae0d43a2685face80e7f14d4aba053973

msg376222 - (view) Author: STINNER Victor (vstinner) * Date: 2020-09-02 09:57

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

msg376372 - (view) Author: STINNER Victor (vstinner) * Date: 2020-09-04 15:15

"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) {
      |                       ^~

msg376433 - (view) Author: Pablo Galindo Salgado (pablogsal) * Date: 2020-09-05 16:08

New changeset 6ae61959ec51a6b3dddc8e665ce6a7b8aeb26c04 by Pablo Galindo in branch '3.8':
bpo-41654: Explicitly cast PyExc_MemoryError to PyTypeObject to avoid warning (GH-22102)
https://github.com/python/cpython/commit/6ae61959ec51a6b3dddc8e665ce6a7b8aeb26c04

msg377424 - (view) Author: STINNER Victor (vstinner) * Date: 2020-09-23 21:25

New changeset bbeb223e9a5e9f9374df384efa386b4068a65c0e by Victor Stinner in branch 'master':
bpo-41654: Fix compiler warning in MemoryError_dealloc() (GH-22387)
https://github.com/python/cpython/commit/bbeb223e9a5e9f9374df384efa386b4068a65c0e

msg388863 - (view) Author: STINNER Victor (vstinner) * Date: 2021-03-16 17:36

New changeset 1f0cde678406749524d11e852a16bf243cef5c5f by Miss Islington (bot) in branch '3.9':
bpo-41654: Fix compiler warning in MemoryError_dealloc() (GH-22387) (GH-24894)
https://github.com/python/cpython/commit/1f0cde678406749524d11e852a16bf243cef5c5f

History Date User Action Args 2022-04-11 14:59:35adminsetgithub: 85820 2021-03-16 17:36:50vstinnersetmessages: + msg388863 2021-03-16 15:43:20miss-islingtonsetnosy: + miss-islington

pull_requests: + pull_request23656

2020-09-23 21:25:45vstinnersetmessages: + msg377424 2020-09-23 15:49:54vstinnersetpull_requests: + pull_request21429 2020-09-05 16:08:01pablogsalsetmessages: + msg376433 2020-09-04 22:27:32pablogsalsetpull_requests: + pull_request21188 2020-09-04 15:15:16vstinnersetmessages: + msg376372 2020-09-02 09:57:13vstinnersetmessages: + msg376222 2020-09-01 20:42:29pablogsalsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved 2020-09-01 20:41:03pablogsalsetmessages: + msg376210 2020-09-01 20:41:03pablogsalsetmessages: + msg376209 2020-09-01 19:07:10pablogsalsetpull_requests: + pull_request21143 2020-09-01 19:06:49pablogsalsetpull_requests: + pull_request21142 2020-09-01 18:40:07pablogsalsetmessages: + msg376203 2020-08-30 19:43:45pablogsalsetmessages: + msg376128 2020-08-30 19:40:32pablogsalsetkeywords: + patch
nosy: + pablogsal

pull_requests: + pull_request21120
stage: patch review

2020-08-30 18:17:33shihai1991setmessages: + msg376119 2020-08-29 15:49:59corona10setnosy: + corona10
2020-08-28 13:15:00shihai1991setnosy: + shihai1991
2020-08-28 11:50:11vstinnersetfiles: + bug.py
nosy: + vstinner
messages: + msg376031 2020-08-28 10:30:39hoeflingsetmessages: + msg376030 2020-08-28 10:15:59hoeflingcreate