Repository navigation
TypeVar with bad arguments segfault/misbehavior #118814
Description
Crash report
What happened?
import typing
typing.TypeVar(name="X", bound=type)Triggers a segfault in python 3.12.3 on all platforms.
Does not reproduce on 3.11 or below.
This bug is similar to #110787, but that fix was never backported to 3.12, and additionally has a flaw that causes some arguments to be shifted in the previously-crashing cases.
When _PyArg_UnpackKeywordsWithVararg gets an input with insufficient positional parameters (which have been provided as keyword arguments) the varargs slot is positioned at the end of the mandatory positional parameters slots. But in the test case, the varargs slot is being overwritten by the bound slot because the keyword argument copy loop that begins here isn't aware of varargs.
If the minimal positionals are provided in the positionals tuple, https://github.com/sobolevn/cpython/blob/c4ca210f12a18edbe30b91aeb6e1915d74936caf/Python/getargs.c#L2525 line in 3.12 (missing the !=) is always true, and the keyword arguments are offset by 1, pushing them to the end of the array and leaving the varargs slot alone. But if there aren't and they need to be backfilled from the keyword arguments, nargs doesn't change in the loop, causing it to overwrite the varargs slot and additionally fail to completely fill the array (causing a segfault when the parent function tries to use that last garbage slot).
This can be fixed by changing the nargs to i so the line reads if (i < vararg) {, then keyword arguments that look up before the varargs entry are not offset, and those that look up after are offset, leaving that slot untouched and ensuring the array is properly filled.
Because i always begins at the end of where the provided positional arguments start, this will hopefully never accidentally overwrite positional arguments, and should solve the problem entirely.
The current fix with != is insufficient because if you provide a third parameter, the != becomes true again, and it reuses a slot. Thanks to a null check it doesn't segfault, but it does result in unexpected behavior:
import typing
T = typing.TypeVar(name="T", bound=type, covariant=True)
assert T.__covariant__fails with an AssertionError in the 3.13 tag and main.
(TypeVar("T", bound=type, covariant=True).__covariant__ is true, however)
CPython versions tested on:
3.11, 3.12, CPython main branch
Operating systems tested on:
macOS, Windows
Output from running 'python -VV' on the command line:
Python 3.14.0a0 (heads/main-dirty:cb6f75a32ca, May 8 2024, 20:35:11) [Clang 15.0.0 (clang-1500.3.9.4)]
Linked PRs
- gh-118814: Fix crash in
_PyArg_UnpackKeywordsWithVararg#122558 - gh-118814: Fix the TypeVar constructor when name is passed by keyword #122664
- [3.13] gh-118814: Fix the TypeVar constructor when name is passed by keyword (GH-122664) #122806
- [3.12] gh-118814: Fix the TypeVar constructor when name is passed by keyword (GH-122664) #122807
Activity
[-]TypeVar with bad arguments triggers segmentation fault[/-][+]TypeVar with bad arguments segfault/misbehavior[/+]bast0006 commentedon May 9, 2024
I now realize I'm linking to a 3.12 PR that was merged when I said "wasn't backported", sorry. Part of the fix was backported, but the getargs.c change wasn't. The behavior is additionally a bit messy. My 3.12 install from pyenv accepts the test case with a
TypeError: constraints must be a tuplethat implies it's not hitting the array-fill segfault (but is hitting bad slot-filling behavior).From my pyenv non-debug install:
The debug compile segfaults when given the two-item testcase, but accepts a simple
TypeVar(name="T")correctly.And after restarting python it isn't consistent in crashing, but it is consistent in behavior, making me think I hit on the right mechanism of action:
JelleZijlstra commentedon May 9, 2024
The crash is in Argument Clinic code, cc @erlend-aasland.
bast0006 commentedon May 9, 2024
@JelleZijlstra that crash is the result of passing 0x1 into
PyObject_IsTrue, which is what happens whentypevar_newgets the incorrectly parsed argument array back from_PyArg_UnpackKeywordsWithVararg.Note the contents of argsbuf, which is constructed by
_PyArg_UnpackKeywordsWithVararg. That last argument should be either NULL or a python type, but it's neither. You can also see thatnameis assigned right, butboundis None andtypehas been assigned intoconstraintsbecause of the bug.JelleZijlstra commentedon May 9, 2024
Right, and that code is generated by Argument Clinic (note the path
Objects/clinic/typevarobject.c.h).Also, thanks for the detailed bug report!
bast0006 commentedon May 9, 2024
Thanks! Hopefully it's helpful.
That is true. I just mean that I traced the bad data/parsing collision out of argument clinic code and into handwritten code in getargs.c, which makes me think the bug actually isn't in argument clinic code, even if it is triggered by it.
erlend-aasland commentedon May 20, 2024
I'm trying to reproduce this1 on macOS, but so far I've been unable to do so. A couple of observations:
Footnotes
the crash, that is ↩
erlend-aasland commentedon May 20, 2024
_PyArg_UnpackKeywordsWithVarargis doing some weird stuff here. Either this is a bug in_PyArg_UnpackKeywordsWithVararg, or it is a bug in how we call it from the generated code.picnixz commentedon Aug 1, 2024
I can reproduce the issue on Python 3.14.0a0 (heads/main:88030861e21, Aug 1 2024, 12:49:44) [GCC 7.5.0]. I'll try to see if I can do something for that.
devdanzin commentedon Aug 1, 2024
I can't reproduce it in 3.13, is the reason that #118009 seems to only have been backported to 3.12?
picnixz commentedon Aug 1, 2024
I couldn't reproduce it the first time because I used
TypeVar('x', bound=type). Check that you usedTypeVar(name='x', bound=type)for it to work (well in this case, to crash). But there is anyway an issue on 3.14.2 remaining items
JelleZijlstra commentedon Aug 1, 2024
We should definitely merge a fix for this issue into 3.13 even during the RC phase (segfaults are bugs). Not sure it should be marked as a release blocker though since 3.12 is also affected.
picnixz commentedon Aug 1, 2024
There's defnitely something wrong with AC here but I won't have time to fix it today.
I think it has to do with the number of defaults keywords. I see different results (sometimes test failures, sometimes crashes) depending on whether there are 2 or 3 keywords. We should test the following cases:
with
If the tests fail, then something's wrong. I couldn't test every combination but the following always crashes, whether I applied my (wrong) fix or not:
I think the logic of
_PyArg_UnpackKeywordsWithVarargshould be entirely reworked.pythongh-118814: Fix the TypeVar constructor when name is passed by k…
serhiy-storchaka commentedon Aug 5, 2024
There are several issues here.
_PyArg_UnpackKeywordsWithVararg. It is only manifested when keyword arguments are passed for keyword-or-positional parameters. Soprint()does not have this bug, but theTypeVarconstructor does. The fix is quite simple: gh-118814: Fix the TypeVar constructor when name is passed by keyword #122664._PyArg_UnpackKeywordsWithVararg. I'm going to re-design it in a separate issue, after fixing bugs in_PyArg_UnpackKeywordsWithVarargand Argument Clinic.gh-118814: Fix the TypeVar constructor when name is passed by keyword (…
[3.13] pythongh-118814: Fix the TypeVar constructor when name is pass…
[3.12] pythongh-118814: Fix the TypeVar constructor when name is pass…
[3.13] gh-118814: Fix the TypeVar constructor when name is passed by …
[3.12] gh-118814: Fix the TypeVar constructor when name is passed by …
pythongh-118814: Fix the TypeVar constructor when name is passed by k…