From be385ff48137e09b46b6ffb819752f4e8c62b73f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 25 Jan 2025 11:31:24 +0100 Subject: [PATCH 1/9] remove redundant casts in `Modules/_interpchannelsmodule.c` --- Modules/_interpchannelsmodule.c | 55 +++++++++++++++++---------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/Modules/_interpchannelsmodule.c b/Modules/_interpchannelsmodule.c index 75d69ade1d3c9b..335e41a276cfb8 100644 --- a/Modules/_interpchannelsmodule.c +++ b/Modules/_interpchannelsmodule.c @@ -2268,6 +2268,8 @@ typedef struct channelid { _channels *channels; } channelid; +#define _channelid_CAST(op) ((channelid *)(op)) + struct channel_id_converter_data { PyObject *module; int64_t cid; @@ -2283,8 +2285,8 @@ channel_id_converter(PyObject *arg, void *ptr) module_state *state = get_module_state(data->module); assert(state != NULL); if (PyObject_TypeCheck(arg, state->ChannelIDType)) { - cid = ((channelid *)arg)->cid; - end = ((channelid *)arg)->end; + cid = _channelid_CAST(arg)->cid; + end = _channelid_CAST(arg)->end; } else if (PyIndex_Check(arg)) { cid = PyLong_AsLongLong(arg); @@ -2398,8 +2400,8 @@ _channelid_new(PyObject *mod, PyTypeObject *cls, static void channelid_dealloc(PyObject *self) { - int64_t cid = ((channelid *)self)->cid; - _channels *channels = ((channelid *)self)->channels; + int64_t cid = _channelid_CAST(self)->cid; + _channels *channels = _channelid_CAST(self)->channels; PyTypeObject *tp = Py_TYPE(self); tp->tp_free(self); @@ -2420,7 +2422,7 @@ channelid_repr(PyObject *self) PyTypeObject *type = Py_TYPE(self); const char *name = _PyType_Name(type); - channelid *cidobj = (channelid *)self; + channelid *cidobj = _channelid_CAST(self); const char *fmt; if (cidobj->end == CHANNEL_SEND) { fmt = "%s(%" PRId64 ", send=True)"; @@ -2437,21 +2439,21 @@ channelid_repr(PyObject *self) static PyObject * channelid_str(PyObject *self) { - channelid *cidobj = (channelid *)self; + channelid *cidobj = _channelid_CAST(self); return PyUnicode_FromFormat("%" PRId64 "", cidobj->cid); } static PyObject * channelid_int(PyObject *self) { - channelid *cidobj = (channelid *)self; + channelid *cidobj = _channelid_CAST(self); return PyLong_FromLongLong(cidobj->cid); } static Py_hash_t channelid_hash(PyObject *self) { - channelid *cidobj = (channelid *)self; + channelid *cidobj = _channelid_CAST(self); PyObject *pyid = PyLong_FromLongLong(cidobj->cid); if (pyid == NULL) { return -1; @@ -2483,10 +2485,10 @@ channelid_richcompare(PyObject *self, PyObject *other, int op) goto done; } - channelid *cidobj = (channelid *)self; + channelid *cidobj = _channelid_CAST(self); int equal; if (PyObject_TypeCheck(other, state->ChannelIDType)) { - channelid *othercidobj = (channelid *)other; + channelid *othercidobj = (channelid *)other; // fast safe cast equal = (cidobj->end == othercidobj->end) && (cidobj->cid == othercidobj->cid); } else if (PyLong_Check(other)) { @@ -2606,9 +2608,10 @@ _channelid_shared(PyThreadState *tstate, PyObject *obj, _PyXIData_t *data) return -1; } struct _channelid_xid *xid = (struct _channelid_xid *)_PyXIData_DATA(data); - xid->cid = ((channelid *)obj)->cid; - xid->end = ((channelid *)obj)->end; - xid->resolve = ((channelid *)obj)->resolve; + channelid *cidobj = _channelid_CAST(obj); + xid->cid = cidobj->cid; + xid->end = cidobj->end; + xid->resolve = cidobj->resolve; return 0; } @@ -2616,7 +2619,7 @@ static PyObject * channelid_end(PyObject *self, void *end) { int force = 1; - channelid *cidobj = (channelid *)self; + channelid *cidobj = _channelid_CAST(self); if (end != NULL) { PyObject *obj = NULL; int err = newchannelid(Py_TYPE(self), cidobj->cid, *(int *)end, @@ -2649,11 +2652,11 @@ static int _channelid_end_send = CHANNEL_SEND; static int _channelid_end_recv = CHANNEL_RECV; static PyGetSetDef channelid_getsets[] = { - {"end", (getter)channelid_end, NULL, + {"end", channelid_end, NULL, PyDoc_STR("'send', 'recv', or 'both'")}, - {"send", (getter)channelid_end, NULL, + {"send", channelid_end, NULL, PyDoc_STR("the 'send' end of the channel"), &_channelid_end_send}, - {"recv", (getter)channelid_end, NULL, + {"recv", channelid_end, NULL, PyDoc_STR("the 'recv' end of the channel"), &_channelid_end_recv}, {NULL} }; @@ -2662,16 +2665,16 @@ PyDoc_STRVAR(channelid_doc, "A channel ID identifies a channel and may be used as an int."); static PyType_Slot channelid_typeslots[] = { - {Py_tp_dealloc, (destructor)channelid_dealloc}, + {Py_tp_dealloc, channelid_dealloc}, {Py_tp_doc, (void *)channelid_doc}, - {Py_tp_repr, (reprfunc)channelid_repr}, - {Py_tp_str, (reprfunc)channelid_str}, + {Py_tp_repr, channelid_repr}, + {Py_tp_str, channelid_str}, {Py_tp_hash, channelid_hash}, {Py_tp_richcompare, channelid_richcompare}, {Py_tp_getset, channelid_getsets}, // number slots - {Py_nb_int, (unaryfunc)channelid_int}, - {Py_nb_index, (unaryfunc)channelid_int}, + {Py_nb_int, channelid_int}, + {Py_nb_index, channelid_int}, {0, NULL}, }; @@ -2904,10 +2907,10 @@ channelsmod_create(PyObject *self, PyObject *args, PyObject *kwds) if (state == NULL) { return NULL; } - PyObject *cidobj = NULL; + channelid *cidobj = NULL; int err = newchannelid(state->ChannelIDType, cid, 0, &_globals.channels, 0, 0, - (channelid **)&cidobj); + &cidobj); if (handle_channel_error(err, self, cid)) { assert(cidobj == NULL); err = channel_destroy(&_globals.channels, cid); @@ -2917,8 +2920,8 @@ channelsmod_create(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } assert(cidobj != NULL); - assert(((channelid *)cidobj)->channels != NULL); - return cidobj; + assert(cidobj->channels != NULL); + return (PyObject *)cidobj; } PyDoc_STRVAR(channelsmod_create_doc, From ae33ded243c899c8fc006be42bc3ecf24c37ed1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 25 Jan 2025 11:32:25 +0100 Subject: [PATCH 2/9] suppress unused return values in `Modules/_interpchannelsmodule.c` --- Modules/_interpchannelsmodule.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/_interpchannelsmodule.c b/Modules/_interpchannelsmodule.c index 335e41a276cfb8..4aca4c3fde1bff 100644 --- a/Modules/_interpchannelsmodule.c +++ b/Modules/_interpchannelsmodule.c @@ -3556,7 +3556,7 @@ module_traverse(PyObject *mod, visitproc visit, void *arg) { module_state *state = get_module_state(mod); assert(state != NULL); - traverse_module_state(state, visit, arg); + (void)traverse_module_state(state, visit, arg); return 0; } @@ -3567,18 +3567,18 @@ module_clear(PyObject *mod) assert(state != NULL); // Now we clear the module state. - clear_module_state(state); + (void)clear_module_state(state); return 0; } static void module_free(void *mod) { - module_state *state = get_module_state(mod); + module_state *state = get_module_state((PyObject *)mod); assert(state != NULL); // Now we clear the module state. - clear_module_state(state); + (void)clear_module_state(state); _globals_fini(); } From 712953d226d0804308c799060ef6878157494ca4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 25 Jan 2025 11:35:03 +0100 Subject: [PATCH 3/9] suppress unused return values in `Modules/_interpqueuesmodule.c` --- Modules/_interpqueuesmodule.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Modules/_interpqueuesmodule.c b/Modules/_interpqueuesmodule.c index 808938a9e8cd16..d069880e2750de 100644 --- a/Modules/_interpqueuesmodule.c +++ b/Modules/_interpqueuesmodule.c @@ -1934,7 +1934,7 @@ static int module_traverse(PyObject *mod, visitproc visit, void *arg) { module_state *state = get_module_state(mod); - traverse_module_state(state, visit, arg); + (void)traverse_module_state(state, visit, arg); return 0; } @@ -1944,17 +1944,17 @@ module_clear(PyObject *mod) module_state *state = get_module_state(mod); // Now we clear the module state. - clear_module_state(state); + (void)clear_module_state(state); return 0; } static void module_free(void *mod) { - module_state *state = get_module_state(mod); + module_state *state = get_module_state((PyObject *)mod); // Now we clear the module state. - clear_module_state(state); + (void)clear_module_state(state); _globals_fini(); } @@ -1968,7 +1968,7 @@ static struct PyModuleDef moduledef = { .m_slots = module_slots, .m_traverse = module_traverse, .m_clear = module_clear, - .m_free = (freefunc)module_free, + .m_free = module_free, }; PyMODINIT_FUNC From 7dad5529e4d6d532f3d0cb957d5b7a7225a2b6fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 25 Jan 2025 11:36:54 +0100 Subject: [PATCH 4/9] fix UBSan failures for `XIBufferViewObject` --- Modules/_interpretersmodule.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Modules/_interpretersmodule.c b/Modules/_interpretersmodule.c index fcd0baf696f943..1a671bcff3b96e 100644 --- a/Modules/_interpretersmodule.c +++ b/Modules/_interpretersmodule.c @@ -83,6 +83,8 @@ typedef struct { int64_t interpid; } XIBufferViewObject; +#define _XIBufferViewObject_CAST(op) ((XIBufferViewObject *)(op)) + static PyObject * xibufferview_from_xid(PyTypeObject *cls, _PyXIData_t *data) { @@ -100,8 +102,9 @@ xibufferview_from_xid(PyTypeObject *cls, _PyXIData_t *data) } static void -xibufferview_dealloc(XIBufferViewObject *self) +xibufferview_dealloc(PyObject *op) { + XIBufferViewObject *self = _XIBufferViewObject_CAST(op); PyInterpreterState *interp = _PyInterpreterState_LookUpID(self->interpid); /* If the interpreter is no longer alive then we have problems, since other objects may be using the buffer still. */ @@ -124,20 +127,21 @@ xibufferview_dealloc(XIBufferViewObject *self) } static int -xibufferview_getbuf(XIBufferViewObject *self, Py_buffer *view, int flags) +xibufferview_getbuf(PyObject *op, Py_buffer *view, int flags) { /* Only PyMemoryView_FromObject() should ever call this, via _memoryview_from_xid() below. */ + XIBufferViewObject *self = _XIBufferViewObject_CAST(op); *view = *self->view; - view->obj = (PyObject *)self; + view->obj = op; // XXX Should we leave it alone? view->internal = NULL; return 0; } static PyType_Slot XIBufferViewType_slots[] = { - {Py_tp_dealloc, (destructor)xibufferview_dealloc}, - {Py_bf_getbuffer, (getbufferproc)xibufferview_getbuf}, + {Py_tp_dealloc, xibufferview_dealloc}, + {Py_bf_getbuffer, xibufferview_getbuf}, // We don't bother with Py_bf_releasebuffer since we don't need it. {0, NULL}, }; From 81d231567cde2c535794beeb99eadc2dcb38fc4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 25 Jan 2025 11:38:01 +0100 Subject: [PATCH 5/9] suppress unused return values in `Modules/_interpretersmodule.c` --- Modules/_interpretersmodule.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Modules/_interpretersmodule.c b/Modules/_interpretersmodule.c index 1a671bcff3b96e..fcba1bbeee2f1a 100644 --- a/Modules/_interpretersmodule.c +++ b/Modules/_interpretersmodule.c @@ -1552,7 +1552,7 @@ module_traverse(PyObject *mod, visitproc visit, void *arg) { module_state *state = get_module_state(mod); assert(state != NULL); - traverse_module_state(state, visit, arg); + (void)traverse_module_state(state, visit, arg); return 0; } @@ -1561,16 +1561,16 @@ module_clear(PyObject *mod) { module_state *state = get_module_state(mod); assert(state != NULL); - clear_module_state(state); + (void)clear_module_state(state); return 0; } static void module_free(void *mod) { - module_state *state = get_module_state(mod); + module_state *state = get_module_state((PyObject *)mod); assert(state != NULL); - clear_module_state(state); + (void)clear_module_state(state); } static struct PyModuleDef moduledef = { @@ -1582,7 +1582,7 @@ static struct PyModuleDef moduledef = { .m_slots = module_slots, .m_traverse = module_traverse, .m_clear = module_clear, - .m_free = (freefunc)module_free, + .m_free = module_free, }; PyMODINIT_FUNC From d4483e0ab8e193aabdaede0b883012cda47cbdb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 7 Feb 2025 12:22:41 +0100 Subject: [PATCH 6/9] do not use the macro when the type is already checked --- Modules/_interpchannelsmodule.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Modules/_interpchannelsmodule.c b/Modules/_interpchannelsmodule.c index 4aca4c3fde1bff..5ce0255aae9b81 100644 --- a/Modules/_interpchannelsmodule.c +++ b/Modules/_interpchannelsmodule.c @@ -2285,8 +2285,8 @@ channel_id_converter(PyObject *arg, void *ptr) module_state *state = get_module_state(data->module); assert(state != NULL); if (PyObject_TypeCheck(arg, state->ChannelIDType)) { - cid = _channelid_CAST(arg)->cid; - end = _channelid_CAST(arg)->end; + cid = ((channelid *)arg)->cid; + end = ((channelid *)arg)->end; } else if (PyIndex_Check(arg)) { cid = PyLong_AsLongLong(arg); @@ -2398,10 +2398,11 @@ _channelid_new(PyObject *mod, PyTypeObject *cls, } static void -channelid_dealloc(PyObject *self) +channelid_dealloc(PyObject *op) { - int64_t cid = _channelid_CAST(self)->cid; - _channels *channels = _channelid_CAST(self)->channels; + channelid *self = _channelid_CAST(self); + int64_t cid = self->cid; + _channels *channels = self->channels; PyTypeObject *tp = Py_TYPE(self); tp->tp_free(self); From e2ee75e25a9eb8ee87087b1dc777e1b929c1f256 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 7 Feb 2025 12:57:48 +0100 Subject: [PATCH 7/9] fix compilation --- Modules/_interpchannelsmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_interpchannelsmodule.c b/Modules/_interpchannelsmodule.c index 5ce0255aae9b81..cc68ceaad718a2 100644 --- a/Modules/_interpchannelsmodule.c +++ b/Modules/_interpchannelsmodule.c @@ -2400,7 +2400,7 @@ _channelid_new(PyObject *mod, PyTypeObject *cls, static void channelid_dealloc(PyObject *op) { - channelid *self = _channelid_CAST(self); + channelid *self = _channelid_CAST(op); int64_t cid = self->cid; _channels *channels = self->channels; From 2794f871b566d5c6799682845ac11f7886bbc53f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 8 Feb 2025 10:26:52 +0100 Subject: [PATCH 8/9] Do not add an underscore to the macro if none is needed. --- Modules/_interpchannelsmodule.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Modules/_interpchannelsmodule.c b/Modules/_interpchannelsmodule.c index cc68ceaad718a2..fc54277cdc0ba0 100644 --- a/Modules/_interpchannelsmodule.c +++ b/Modules/_interpchannelsmodule.c @@ -2268,7 +2268,7 @@ typedef struct channelid { _channels *channels; } channelid; -#define _channelid_CAST(op) ((channelid *)(op)) +#define channelid_CAST(op) ((channelid *)(op)) struct channel_id_converter_data { PyObject *module; @@ -2400,7 +2400,7 @@ _channelid_new(PyObject *mod, PyTypeObject *cls, static void channelid_dealloc(PyObject *op) { - channelid *self = _channelid_CAST(op); + channelid *self = channelid_CAST(op); int64_t cid = self->cid; _channels *channels = self->channels; @@ -2423,7 +2423,7 @@ channelid_repr(PyObject *self) PyTypeObject *type = Py_TYPE(self); const char *name = _PyType_Name(type); - channelid *cidobj = _channelid_CAST(self); + channelid *cidobj = channelid_CAST(self); const char *fmt; if (cidobj->end == CHANNEL_SEND) { fmt = "%s(%" PRId64 ", send=True)"; @@ -2440,21 +2440,21 @@ channelid_repr(PyObject *self) static PyObject * channelid_str(PyObject *self) { - channelid *cidobj = _channelid_CAST(self); + channelid *cidobj = channelid_CAST(self); return PyUnicode_FromFormat("%" PRId64 "", cidobj->cid); } static PyObject * channelid_int(PyObject *self) { - channelid *cidobj = _channelid_CAST(self); + channelid *cidobj = channelid_CAST(self); return PyLong_FromLongLong(cidobj->cid); } static Py_hash_t channelid_hash(PyObject *self) { - channelid *cidobj = _channelid_CAST(self); + channelid *cidobj = channelid_CAST(self); PyObject *pyid = PyLong_FromLongLong(cidobj->cid); if (pyid == NULL) { return -1; @@ -2486,7 +2486,7 @@ channelid_richcompare(PyObject *self, PyObject *other, int op) goto done; } - channelid *cidobj = _channelid_CAST(self); + channelid *cidobj = channelid_CAST(self); int equal; if (PyObject_TypeCheck(other, state->ChannelIDType)) { channelid *othercidobj = (channelid *)other; // fast safe cast @@ -2609,7 +2609,7 @@ _channelid_shared(PyThreadState *tstate, PyObject *obj, _PyXIData_t *data) return -1; } struct _channelid_xid *xid = (struct _channelid_xid *)_PyXIData_DATA(data); - channelid *cidobj = _channelid_CAST(obj); + channelid *cidobj = channelid_CAST(obj); xid->cid = cidobj->cid; xid->end = cidobj->end; xid->resolve = cidobj->resolve; @@ -2620,7 +2620,7 @@ static PyObject * channelid_end(PyObject *self, void *end) { int force = 1; - channelid *cidobj = _channelid_CAST(self); + channelid *cidobj = channelid_CAST(self); if (end != NULL) { PyObject *obj = NULL; int err = newchannelid(Py_TYPE(self), cidobj->cid, *(int *)end, From 9c6f69c8c22e6b67c8a696dab7bad0907dbfc58e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 8 Feb 2025 10:27:30 +0100 Subject: [PATCH 9/9] Do Do not use `_` + capital letter in new code as it is also UB. --- Modules/_interpretersmodule.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/_interpretersmodule.c b/Modules/_interpretersmodule.c index fcba1bbeee2f1a..d4d90b5f31b7ab 100644 --- a/Modules/_interpretersmodule.c +++ b/Modules/_interpretersmodule.c @@ -83,7 +83,7 @@ typedef struct { int64_t interpid; } XIBufferViewObject; -#define _XIBufferViewObject_CAST(op) ((XIBufferViewObject *)(op)) +#define XIBufferViewObject_CAST(op) ((XIBufferViewObject *)(op)) static PyObject * xibufferview_from_xid(PyTypeObject *cls, _PyXIData_t *data) @@ -104,7 +104,7 @@ xibufferview_from_xid(PyTypeObject *cls, _PyXIData_t *data) static void xibufferview_dealloc(PyObject *op) { - XIBufferViewObject *self = _XIBufferViewObject_CAST(op); + XIBufferViewObject *self = XIBufferViewObject_CAST(op); PyInterpreterState *interp = _PyInterpreterState_LookUpID(self->interpid); /* If the interpreter is no longer alive then we have problems, since other objects may be using the buffer still. */ @@ -131,7 +131,7 @@ xibufferview_getbuf(PyObject *op, Py_buffer *view, int flags) { /* Only PyMemoryView_FromObject() should ever call this, via _memoryview_from_xid() below. */ - XIBufferViewObject *self = _XIBufferViewObject_CAST(op); + XIBufferViewObject *self = XIBufferViewObject_CAST(op); *view = *self->view; view->obj = op; // XXX Should we leave it alone?