Skip to content

gh-143635: Fix crash in _Py_typing_type_repr #143670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sobolevn wants to merge 2 commits into
base: main
Choose a base branch
from
+42 −1
Open

gh-143635: Fix crash in _Py_typing_type_repr #143670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b3b3f5
gh-143635: Fix crash in `_Py_typing_type_repr`
sobolevn Jan 10, 2026
0eee5b8
gh-143635: Fix crash in `_Py_typing_type_repr`
sobolevn Jan 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions Lib/test/test_genericalias.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,43 @@ class MyGeneric:
self.assertEndsWith(repr(MyGeneric[[]]), 'MyGeneric[[]]')
self.assertEndsWith(repr(MyGeneric[[int, str]]), 'MyGeneric[[int, str]]')

def test_evil_repr1(self):
# gh-143635
class Zap:
def __init__(self, container):
self.container = container
def __getattr__(self, name):
if name == "__origin__":
self.container.clear()
return None
if name == "__args__":
return ()
raise AttributeError

params = []
params.append(Zap(params))
alias = type(list[int])(list, (params,))
repr_str = repr(alias)
self.assertTrue(repr_str.startswith("list[["), repr_str)

def test_evil_repr2(self):
class Zap:
def __init__(self, container):
self.container = container
def __getattr__(self, name):
if name == "__qualname__":
self.container.clear()
return "abcd"
if name == "__module__":
return None
raise AttributeError

params = []
params.append(Zap(params))
alias = type(list[int])(list, (params,))
repr_str = repr(alias)
self.assertTrue(repr_str.startswith("list[["), repr_str)

def test_exposed_type(self):
import types
a = types.GenericAlias(list, int)
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2026-01-11-20-11-36.gh-issue-143670.klnGoD.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes a crash in ``_Py_typing_type_repr`` function.
5 changes: 4 additions & 1 deletion Objects/typevarobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,14 @@ _Py_typing_type_repr(PyUnicodeWriter *writer, PyObject *p)
if (p == Py_Ellipsis) {
// The Ellipsis object
r = PyUnicode_FromString("...");
goto exit;
goto cleanup;
}

if (p == (PyObject *)&_PyNone_Type) {
return PyUnicodeWriter_WriteASCII(writer, "None", 4);
}

Py_INCREF(p); // gh-143635
Copy link
Member

@JelleZijlstra JelleZijlstra Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort of feels like the wrong place, shouldn't callers of the function ensure they have a reference to p that remains valid?

Copy link
Member

@picnixz picnixz Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so and we should just say that the function takes a strong reference (the function is not documented but it would be good to indicate this).

That means changing some PyList_GET_ITEM/PyTuple_GET_ITEM and adding some Py_INCREF for non-sequences.

Copy link
Member Author

@sobolevn sobolevn Jan 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I am not quite sure about it. Technically, we don't need a strong reference in regular cases, it is only needed for strange self-destructing ones.

We have multiple places where we just use Py_INCREF / Py_DECREF on a object before calling some python callbacks with it. Changing this function to require a strong refenrence will require quite a lot of refactoring for a very niche thing (and might be a breaking change for anyone using this function, even though it is not documented).

Let's say we want to refactor this place:

cpython/Objects/genericaliasobject.c

Lines 103 to 105 in e22b685

if (_Py_typing_type_repr(writer, alias->origin) < 0) {
goto error;
}

It would be:

Py_INCREF(alias->origin);
if (_Py_typing_type_repr(writer, alias->origin) < 0) {
     Py_DECREF(alias->origin);
     goto error;
}
Py_DECREF(alias->origin);

This does not seem like a good API to me :/

What are the potential limitations of the current approach?

Copy link
Member

@picnixz picnixz Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this function to require a strong refenrence will require quite a lot of refactoring for a very niche thing (and might be a breaking change for anyone using this function, even though it is not documented).

It's not exported as part of the public API so I'm not sure it's available by "others" (it's only an extern symbol in the internal API). Since it's internal we can also keep your change as it reduces the diff (I didn't think there would be that many places though)

Copy link
Contributor

@sergey-miryanov sergey-miryanov Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is worth to open a separate PR for adding a strong ref?

Copy link
Member

@picnixz picnixz Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I think it should be decided here.

Copy link
Member

@picnixz picnixz Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I think it should be decided here.

if ((rc = PyObject_HasAttrWithError(p, &_Py_ID(__origin__))) > 0 &&
(rc = PyObject_HasAttrWithError(p, &_Py_ID(__args__))) > 0)
{
Expand Down Expand Up @@ -316,6 +317,8 @@ _Py_typing_type_repr(PyUnicodeWriter *writer, PyObject *p)
use_repr:
r = PyObject_Repr(p);
exit:
Py_DECREF(p); // gh-143635
cleanup:
Py_XDECREF(qualname);
Py_XDECREF(module);
if (r == NULL) {
Expand Down
Loading