-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
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.
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
TypeVar with bad arguments segfault/misbehavior #118814
Comments
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 From my pyenv non-debug install: >>> typing.TypeVar(name="T", bound=type, covariant=True)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: constraints must be a tuple
>>> typing.TypeVar(name="T", bound=(type,), covariant=True)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: A single constraint is not allowed
>>> typing.TypeVar(name="T", bound=(type, int), covariant=True)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: Constraints cannot be combined with bound=...
>>> typing.TypeVar(name="T", bound=(type, int))
T
>>> typing.TypeVar(name="T", bound=(type, int)).__bound__
>>> typing.TypeVar(name="T", bound=(type, int)).__constraints__
((...), <class 'int'>)
>>> typing.TypeVar(name="T", bound=tuple(), covariant=True)
zsh: segmentation fault python The debug compile segfaults when given the two-item testcase, but accepts a simple 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: typing.TypeVar(name="T", bound=tuple(), covariant="OhNo").__bound__
ForwardRef('OhNo') |
The crash is in Argument Clinic code, cc @erlend-aasland.
|
@JelleZijlstra that crash is the result of passing 0x1 into
(gdb) up
#1 0x0000000100129b72 in typevar_new (type=<optimized out>, args=<optimized out>, kwargs=<optimized out>) at Objects/clinic/typevarobject.c.h:100
100 infer_variance = PyObject_IsTrue(fastargs[5]);
(gdb) info locals
_kwtuple = {_this_is_not_used = {_gc_next = 0, _gc_prev = 0}, ob_base = {ob_base = {{ob_refcnt = 4294967295, ob_refcnt_split = {4294967295, 0}}, ob_type = 0x100520c60},
ob_size = 5}, ob_item = {'name', 'bound', 'covariant', 'contravariant', 'infer_variance'}}
_keywords = {0x100376e78 "name", 0x1003a352c "bound", 0x1003a3532 "covariant", 0x1003a353c "contravariant", 0x1003a354a "infer_variance", 0x0}
_parser = {initialized = -1, format = 0x0, keywords = 0x10048fd80, fname = 0x1003a3559 "typevar", custom_msg = 0x0, pos = 0, min = 0, max = 0,
kwtuple = ('name', 'bound', 'covariant', 'contravariant', 'infer_variance'), next = 0x10051a1e8}
argsbuf = {'T', <type at remote 0x100521130>, 0x0, 0x0, 0x0, <unknown at remote 0x1>}
return_value = 0x0
nargs = <optimized out>
noptargs = <optimized out>
constraints = <type at remote 0x100521130>
covariant = 0
contravariant = 0
infer_variance = 0
fastargs = 0x7ff7bfefec30
bound = None
name = 'T'
(gdb) p fastargs[5]
$1 = <unknown at remote 0x1> Note the contents of argsbuf, which is constructed by |
Right, and that code is generated by Argument Clinic (note the path Also, thanks for the detailed bug report! |
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. |
I'm trying to reproduce this1 on macOS, but so far I've been unable to do so. A couple of observations:
Footnotes
|
|
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. |
I can't reproduce it in 3.13, is the reason that #118009 seems to only have been backported to 3.12? |
I couldn't reproduce it the first time because I used |
That does it, thanks! Should this be a release blocker for 3.13 then? |
3.13rc1 has already been released so it's not a blocker. But I don't know whether we'll merge it before releasing 3.13.0. @Yhg1s If my fix is correct, should it land or not? and if my fix is not correct, does this count as a blocker of 3.13.0? |
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. |
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: self.assertEqual(func(1, kw1=True), (1, (), True, False))
self.assertEqual(func(1, kw2=True), (1, (), False, True))
self.assertEqual(func(1, kw1=True, kw2=True), (1, (), True, True))
self.assertEqual(func(1, kw2=True, kw1=True), (1, (), True, True))
self.assertEqual(func(1, 2, 3, 4), (1, (2, 3, 4), False, False))
self.assertEqual(func(1, 2, 3, 4, kw1=True), (1, (2, 3, 4), True, False))
self.assertEqual(func(1, 2, 3, 4, kw1=True), (1, (2, 3, 4), True, False))
self.assertEqual(func(1, 2, 3, 4, kw1=True, kw2=True), (1, (2, 3, 4), True, True))
self.assertEqual(func(a=1), (1, (), False, False))
self.assertEqual(func(a=1, kw1=True), (1, (), True, False))
self.assertEqual(func(kw1=True, a=1), (1, (), True, False))
self.assertEqual(func(a=1, kw2=True), (1, (), False, True))
self.assertEqual(func(kw2=True, a=1), (1, (), False, True)) with /*[clinic input]
vararg_with_multiple_defaults
a: object
*args: object
kw1: bool = False
kw2: bool = False
[clinic start generated code]*/
static PyObject *
vararg_with_multiple_defaults_impl(PyObject *module, PyObject *a,
PyObject *args, int kw1, int kw2)
/*[clinic end generated code: output=ae7ee8d22dfc7fbf input=534d91e23e6d360b]*/
{
PyObject *obj_kw1 = kw1 ? Py_True : Py_False;
PyObject *obj_kw2 = kw2 ? Py_True : Py_False;
return pack_arguments_newref(4, a, args, obj_kw1, obj_kw2);
} 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: self.assertEqual(func(a=1, kw1=True, kw2=True), (1, (), True, True))
self.assertEqual(func(a=1, kw2=True, kw1=True), (1, (), True, True))
self.assertEqual(func(kw1=True, a=1, kw2=True), (1, (), True, True))
self.assertEqual(func(kw2=True, a=1, kw1=True), (1, (), True, True))
self.assertEqual(func(kw1=True, kw2=True, a=1), (1, (), True, True))
self.assertEqual(func(kw2=True, kw1=True, a=1), (1, (), True, True)) I think the logic of |
…eyword Fix _PyArg_UnpackKeywordsWithVararg for the case when argument for positional-or-keyword parameter is passed by keyword. There was only one such case in the stdlib -- the TypeVar constructor.
There are several issues here.
|
…GH-122664) Fix _PyArg_UnpackKeywordsWithVararg for the case when argument for positional-or-keyword parameter is passed by keyword. There was only one such case in the stdlib -- the TypeVar constructor.
…ed by keyword (pythonGH-122664) Fix _PyArg_UnpackKeywordsWithVararg for the case when argument for positional-or-keyword parameter is passed by keyword. There was only one such case in the stdlib -- the TypeVar constructor. (cherry picked from commit 540fcc6) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…ed by keyword (pythonGH-122664) Fix _PyArg_UnpackKeywordsWithVararg for the case when argument for positional-or-keyword parameter is passed by keyword. There was only one such case in the stdlib -- the TypeVar constructor. (cherry picked from commit 540fcc6) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…eyword (pythonGH-122664) Fix _PyArg_UnpackKeywordsWithVararg for the case when argument for positional-or-keyword parameter is passed by keyword. There was only one such case in the stdlib -- the TypeVar constructor.
Crash report
What happened?
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) thevarargs
slot is positioned at the end of the mandatory positional parameters slots. But in the test case, thevarargs
slot is being overwritten by thebound
slot because the keyword argument copy loop that begins here isn't aware ofvarargs
.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 thevarargs
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
toi
so the line readsif (i < vararg) {
, then keyword arguments that look up before thevarargs
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:
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
_PyArg_UnpackKeywordsWithVararg
#122558The text was updated successfully, but these errors were encountered: