From f7ee5c2410cc174ddfc86e961d0579cbe218d03b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:21:24 +0100 Subject: [PATCH 1/4] fix UBSan failures for `anextawaitableobject` --- Objects/iterobject.c | 45 +++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/Objects/iterobject.c b/Objects/iterobject.c index 8b33a7ded3ffd6..77fb38ae0c6791 100644 --- a/Objects/iterobject.c +++ b/Objects/iterobject.c @@ -312,9 +312,12 @@ typedef struct { PyObject *default_value; } anextawaitableobject; +#define anextawaitableobject_CAST(op) ((anextawaitableobject *)(op)) + static void -anextawaitable_dealloc(anextawaitableobject *obj) +anextawaitable_dealloc(PyObject *op) { + anextawaitableobject *obj = anextawaitableobject_CAST(op); _PyObject_GC_UNTRACK(obj); Py_XDECREF(obj->wrapped); Py_XDECREF(obj->default_value); @@ -322,8 +325,9 @@ anextawaitable_dealloc(anextawaitableobject *obj) } static int -anextawaitable_traverse(anextawaitableobject *obj, visitproc visit, void *arg) +anextawaitable_traverse(PyObject *op, visitproc visit, void *arg) { + anextawaitableobject *obj = anextawaitableobject_CAST(op); Py_VISIT(obj->wrapped); Py_VISIT(obj->default_value); return 0; @@ -360,7 +364,7 @@ anextawaitable_getiter(anextawaitableobject *obj) } static PyObject * -anextawaitable_iternext(anextawaitableobject *obj) +anextawaitable_iternext(PyObject *op) { /* Consider the following class: * @@ -382,6 +386,7 @@ anextawaitable_iternext(anextawaitableobject *obj) * Then `await anext(gen)` can just call * gen.__anext__().__next__() */ + anextawaitableobject *obj = anextawaitableobject_CAST(op); PyObject *awaitable = anextawaitable_getiter(obj); if (awaitable == NULL) { return NULL; @@ -400,11 +405,14 @@ anextawaitable_iternext(anextawaitableobject *obj) static PyObject * -anextawaitable_proxy(anextawaitableobject *obj, char *meth, PyObject *arg) { +anextawaitable_proxy(anextawaitableobject *obj, char *meth, PyObject *arg) +{ PyObject *awaitable = anextawaitable_getiter(obj); if (awaitable == NULL) { return NULL; } + // 'arg' may be a tuple (if coming from a METH_VARARGS method) + // or a single object (if coming from a METH_O method). PyObject *ret = PyObject_CallMethod(awaitable, meth, "O", arg); Py_DECREF(awaitable); if (ret != NULL) { @@ -424,20 +432,27 @@ anextawaitable_proxy(anextawaitableobject *obj, char *meth, PyObject *arg) { static PyObject * -anextawaitable_send(anextawaitableobject *obj, PyObject *arg) { +anextawaitable_send(PyObject *op, PyObject *arg) +{ + anextawaitableobject *obj = anextawaitableobject_CAST(op); return anextawaitable_proxy(obj, "send", arg); } static PyObject * -anextawaitable_throw(anextawaitableobject *obj, PyObject *arg) { - return anextawaitable_proxy(obj, "throw", arg); +anextawaitable_throw(PyObject *op, PyObject *args) +{ + anextawaitableobject *obj = anextawaitableobject_CAST(op); + return anextawaitable_proxy(obj, "throw", args); } static PyObject * -anextawaitable_close(anextawaitableobject *obj, PyObject *arg) { - return anextawaitable_proxy(obj, "close", arg); +anextawaitable_close(PyObject *op, PyObject *args) +{ + // TODO(picnixz): what happens if we pass an argument to close()? + anextawaitableobject *obj = anextawaitableobject_CAST(op); + return anextawaitable_proxy(obj, "close", args); } @@ -461,9 +476,9 @@ PyDoc_STRVAR(close_doc, static PyMethodDef anextawaitable_methods[] = { - {"send",(PyCFunction)anextawaitable_send, METH_O, send_doc}, - {"throw",(PyCFunction)anextawaitable_throw, METH_VARARGS, throw_doc}, - {"close",(PyCFunction)anextawaitable_close, METH_VARARGS, close_doc}, + {"send", anextawaitable_send, METH_O, send_doc}, + {"throw", anextawaitable_throw, METH_VARARGS, throw_doc}, + {"close", anextawaitable_close, METH_VARARGS, close_doc}, {NULL, NULL} /* Sentinel */ }; @@ -481,7 +496,7 @@ PyTypeObject _PyAnextAwaitable_Type = { sizeof(anextawaitableobject), /* tp_basicsize */ 0, /* tp_itemsize */ /* methods */ - (destructor)anextawaitable_dealloc, /* tp_dealloc */ + anextawaitable_dealloc, /* tp_dealloc */ 0, /* tp_vectorcall_offset */ 0, /* tp_getattr */ 0, /* tp_setattr */ @@ -498,12 +513,12 @@ PyTypeObject _PyAnextAwaitable_Type = { 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */ 0, /* tp_doc */ - (traverseproc)anextawaitable_traverse, /* tp_traverse */ + anextawaitable_traverse, /* tp_traverse */ 0, /* tp_clear */ 0, /* tp_richcompare */ 0, /* tp_weaklistoffset */ PyObject_SelfIter, /* tp_iter */ - (unaryfunc)anextawaitable_iternext, /* tp_iternext */ + anextawaitable_iternext, /* tp_iternext */ anextawaitable_methods, /* tp_methods */ }; From 951fcdc356cbbbec384ccb956b08d7f303c344c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:22:20 +0100 Subject: [PATCH 2/4] fix UBSan failures for `calliterobject` --- Objects/iterobject.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Objects/iterobject.c b/Objects/iterobject.c index 77fb38ae0c6791..595aacf1778da1 100644 --- a/Objects/iterobject.c +++ b/Objects/iterobject.c @@ -187,6 +187,8 @@ typedef struct { PyObject *it_sentinel; /* Set to NULL when iterator is exhausted */ } calliterobject; +#define calliterobject_CAST(op) ((calliterobject *)(op)) + PyObject * PyCallIter_New(PyObject *callable, PyObject *sentinel) { @@ -202,7 +204,7 @@ PyCallIter_New(PyObject *callable, PyObject *sentinel) static void calliter_dealloc(PyObject *op) { - calliterobject *it = (calliterobject*)op; + calliterobject *it = calliterobject_CAST(op); _PyObject_GC_UNTRACK(it); Py_XDECREF(it->it_callable); Py_XDECREF(it->it_sentinel); @@ -210,8 +212,9 @@ calliter_dealloc(PyObject *op) } static int -calliter_traverse(calliterobject *it, visitproc visit, void *arg) +calliter_traverse(PyObject *op, visitproc visit, void *arg) { + calliterobject *it = calliterobject_CAST(op); Py_VISIT(it->it_callable); Py_VISIT(it->it_sentinel); return 0; @@ -220,7 +223,7 @@ calliter_traverse(calliterobject *it, visitproc visit, void *arg) static PyObject * calliter_iternext(PyObject *op) { - calliterobject *it = (calliterobject*)op; + calliterobject *it = calliterobject_CAST(op); PyObject *result; if (it->it_callable == NULL) { @@ -253,7 +256,7 @@ calliter_iternext(PyObject *op) static PyObject * calliter_reduce(PyObject *op, PyObject *Py_UNUSED(ignored)) { - calliterobject *it = (calliterobject*)op; + calliterobject *it = calliterobject_CAST(op); PyObject *iter = _PyEval_GetBuiltin(&_Py_ID(iter)); /* _PyEval_GetBuiltin can invoke arbitrary code, @@ -294,7 +297,7 @@ PyTypeObject PyCallIter_Type = { 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */ 0, /* tp_doc */ - (traverseproc)calliter_traverse, /* tp_traverse */ + calliter_traverse, /* tp_traverse */ 0, /* tp_clear */ 0, /* tp_richcompare */ 0, /* tp_weaklistoffset */ From 5cb6efe04b69727476eb17c2efc723e2bce2df74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 24 Mar 2025 11:04:20 +0100 Subject: [PATCH 3/4] amend TODO notes --- Objects/iterobject.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Objects/iterobject.c b/Objects/iterobject.c index 36e4afae75979b..bfda7f6d47f698 100644 --- a/Objects/iterobject.c +++ b/Objects/iterobject.c @@ -453,7 +453,8 @@ anextawaitable_throw(PyObject *op, PyObject *args) static PyObject * anextawaitable_close(PyObject *op, PyObject *args) { - // TODO(picnixz): what happens if we pass an argument to close()? + // TODO(picnixz): investigate why close() is marked with METH_VARARGS + // despite its docs indicating that it should be using METH_NOARGS. anextawaitableobject *obj = anextawaitableobject_CAST(op); return anextawaitable_proxy(obj, "close", args); } From 59d406ce1af3b639262fb24f8c508b614a3ee0d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 24 Mar 2025 15:17:41 +0100 Subject: [PATCH 4/4] remove unnecessary TODO --- Objects/iterobject.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/Objects/iterobject.c b/Objects/iterobject.c index bfda7f6d47f698..539fe360504c40 100644 --- a/Objects/iterobject.c +++ b/Objects/iterobject.c @@ -453,8 +453,6 @@ anextawaitable_throw(PyObject *op, PyObject *args) static PyObject * anextawaitable_close(PyObject *op, PyObject *args) { - // TODO(picnixz): investigate why close() is marked with METH_VARARGS - // despite its docs indicating that it should be using METH_NOARGS. anextawaitableobject *obj = anextawaitableobject_CAST(op); return anextawaitable_proxy(obj, "close", args); }