classification
Title: Fatal error in type union
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.11, Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: kj, miss-islington, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2021-06-22 06:32 by serhiy.storchaka, last changed 2021-06-23 09:55 by kj. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 26848 merged kj, 2021-06-22 11:10
PR 26852 merged miss-islington, 2021-06-22 14:37
Messages (6)
msg396307 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-06-22 06:32
The following example crashes:

class TypeVar:
    @property
    def __module__(self):
        1/0

str | TypeVar()


Output:
Fatal Python error: _Py_CheckSlotResult: Slot | of type type succeeded with an exception set
Python runtime state: initialized
Traceback (most recent call last):
  File "<stdin>", line 4, in __module__
ZeroDivisionError: division by zero
Aborted (core dumped)

The problem in Objects/unionobject.c is that is_typing_module() (and therefore is_typevar() and is_special_form()) can return not only 0 and 1, but -1 as a signal of error, but is_unionable() does not check results for error and interprets it as boolean true.
msg396320 - (view) Author: Ken Jin (kj) * (Python committer) Date: 2021-06-22 11:11
A possible simple fix is to change these lines https://github.com/python/cpython/blob/main/Objects/unionobject.c#L294-L301

to:
```
    return (
        is_typevar(obj) |
        is_new_type(obj) |
        is_special_form(obj) |
        PyType_Check(obj) |
        PyObject_TypeCheck(obj, &Py_GenericAliasType) |
        (int)(type == &_Py_UnionType));
```

However, that may slow down union a little since we lose the short-circuiting that `||` provides over `|`, and all checks have to be evaluated.

Checking each result individually and mimicking the short circuiting behavior works too, so I did that. What do you think Serhiy?
msg396333 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-06-22 13:54
New changeset adfa1ba398c74720b42f16f06fd3ec0353599fa5 by Ken Jin in branch 'main':
bpo-44483: Fix crash in union object with bad ``__module__`` (GH-26848)
https://github.com/python/cpython/commit/adfa1ba398c74720b42f16f06fd3ec0353599fa5
msg396395 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-06-23 09:39
New changeset 7e6cad7e303b3991360a0fe332b0d21aa0f6fe5e by Miss Islington (bot) in branch '3.10':
bpo-44483: Fix crash in union object with bad ``__module__`` (GH-26848) (GH-26852)
https://github.com/python/cpython/commit/7e6cad7e303b3991360a0fe332b0d21aa0f6fe5e
msg396396 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-06-23 09:44
Yes, Checking each result individually is a right way. Not only because performance, but because calling any Python code while an error is set will cause a crash in debug build and weird bugs in release build. It is better to return as fast as you have an error.

Thank you for your PR Ken Jin.
msg396398 - (view) Author: Ken Jin (kj) * (Python committer) Date: 2021-06-23 09:55
Oh, that's a good point too. Thanks for the explanation, reviews and merge Serhiy.
History
Date User Action Args
2021-06-23 09:55:58kjsetmessages: + msg396398
2021-06-23 09:44:11serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg396396

stage: patch review -> resolved
2021-06-23 09:39:21serhiy.storchakasetmessages: + msg396395
2021-06-22 14:37:28miss-islingtonsetnosy: + miss-islington

pull_requests: + pull_request25433
stage: patch review
2021-06-22 13:54:51serhiy.storchakasetmessages: + msg396333
2021-06-22 11:11:10kjsetnosy: serhiy.storchaka, kj
messages: + msg396320
stage: patch review -> (no value)
2021-06-22 11:10:05kjsetkeywords: + patch
nosy: + kj

pull_requests: + pull_request25429
stage: patch review
2021-06-22 06:32:26serhiy.storchakacreate