From bda6a70b99f64480cb1ced9ce50d9bb9ebe21f9a 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, 16 Dec 2024 14:40:26 +0100 Subject: [PATCH] gh-111178: fix UBSan failures in `_elementtree.c` (#127982) --- Modules/_elementtree.c | 133 +++++++++++++++++++++++++---------------- 1 file changed, 80 insertions(+), 53 deletions(-) diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index e134e096e044b71..355f322d304c2fd 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -184,7 +184,7 @@ elementtree_traverse(PyObject *m, visitproc visit, void *arg) static void elementtree_free(void *m) { - elementtree_clear((PyObject *)m); + (void)elementtree_clear((PyObject *)m); } /* helpers */ @@ -257,6 +257,7 @@ typedef struct { } ElementObject; +#define _Element_CAST(op) ((ElementObject *)(op)) #define Element_CheckExact(st, op) Py_IS_TYPE(op, (st)->Element_Type) #define Element_Check(st, op) PyObject_TypeCheck(op, (st)->Element_Type) @@ -648,8 +649,9 @@ subelement(PyObject *self, PyObject *args, PyObject *kwds) } static int -element_gc_traverse(ElementObject *self, visitproc visit, void *arg) +element_gc_traverse(PyObject *op, visitproc visit, void *arg) { + ElementObject *self = _Element_CAST(op); Py_VISIT(Py_TYPE(self)); Py_VISIT(self->tag); Py_VISIT(JOIN_OBJ(self->text)); @@ -666,8 +668,9 @@ element_gc_traverse(ElementObject *self, visitproc visit, void *arg) } static int -element_gc_clear(ElementObject *self) +element_gc_clear(PyObject *op) { + ElementObject *self = _Element_CAST(op); Py_CLEAR(self->tag); _clear_joined_ptr(&self->text); _clear_joined_ptr(&self->tail); @@ -680,8 +683,9 @@ element_gc_clear(ElementObject *self) } static void -element_dealloc(ElementObject* self) +element_dealloc(PyObject *op) { + ElementObject *self = _Element_CAST(op); PyTypeObject *tp = Py_TYPE(self); /* bpo-31095: UnTrack is needed before calling any callbacks */ @@ -689,13 +693,13 @@ element_dealloc(ElementObject* self) Py_TRASHCAN_BEGIN(self, element_dealloc) if (self->weakreflist != NULL) - PyObject_ClearWeakRefs((PyObject *) self); + PyObject_ClearWeakRefs(op); /* element_gc_clear clears all references and deallocates extra */ - element_gc_clear(self); + (void)element_gc_clear(op); - tp->tp_free((PyObject *)self); + tp->tp_free(self); Py_DECREF(tp); Py_TRASHCAN_END } @@ -1478,9 +1482,9 @@ _elementtree_Element_itertext_impl(ElementObject *self, PyTypeObject *cls) static PyObject* -element_getitem(PyObject* self_, Py_ssize_t index) +element_getitem(PyObject *op, Py_ssize_t index) { - ElementObject* self = (ElementObject*) self_; + ElementObject *self = _Element_CAST(op); if (!self->extra || index < 0 || index >= self->extra->length) { PyErr_SetString( @@ -1494,9 +1498,9 @@ element_getitem(PyObject* self_, Py_ssize_t index) } static int -element_bool(PyObject* self_) +element_bool(PyObject *op) { - ElementObject* self = (ElementObject*) self_; + ElementObject *self = _Element_CAST(op); if (PyErr_WarnEx(PyExc_DeprecationWarning, "Testing an element's truth value will always return True " "in future versions. Use specific 'len(elem)' or " @@ -1583,8 +1587,9 @@ _elementtree_Element_keys_impl(ElementObject *self) } static Py_ssize_t -element_length(ElementObject* self) +element_length(PyObject *op) { + ElementObject *self = _Element_CAST(op); if (!self->extra) return 0; @@ -1675,10 +1680,10 @@ _elementtree_Element_remove_impl(ElementObject *self, PyObject *subelement) } static PyObject* -element_repr(ElementObject* self) +element_repr(PyObject *op) { int status; - + ElementObject *self = _Element_CAST(op); if (self->tag == NULL) return PyUnicode_FromFormat("", self); @@ -1728,9 +1733,9 @@ _elementtree_Element_set_impl(ElementObject *self, PyObject *key, } static int -element_setitem(PyObject* self_, Py_ssize_t index, PyObject* item) +element_setitem(PyObject *op, Py_ssize_t index, PyObject* item) { - ElementObject* self = (ElementObject*) self_; + ElementObject *self = _Element_CAST(op); Py_ssize_t i; PyObject* old; @@ -1762,10 +1767,10 @@ element_setitem(PyObject* self_, Py_ssize_t index, PyObject* item) return 0; } -static PyObject* -element_subscr(PyObject* self_, PyObject* item) +static PyObject * +element_subscr(PyObject *op, PyObject *item) { - ElementObject* self = (ElementObject*) self_; + ElementObject *self = _Element_CAST(op); if (PyIndex_Check(item)) { Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError); @@ -1775,7 +1780,7 @@ element_subscr(PyObject* self_, PyObject* item) } if (i < 0 && self->extra) i += self->extra->length; - return element_getitem(self_, i); + return element_getitem(op, i); } else if (PySlice_Check(item)) { Py_ssize_t start, stop, step, slicelen, i; @@ -1815,9 +1820,9 @@ element_subscr(PyObject* self_, PyObject* item) } static int -element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value) +element_ass_subscr(PyObject *op, PyObject *item, PyObject *value) { - ElementObject* self = (ElementObject*) self_; + ElementObject *self = _Element_CAST(op); if (PyIndex_Check(item)) { Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError); @@ -1827,7 +1832,7 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value) } if (i < 0 && self->extra) i += self->extra->length; - return element_setitem(self_, i, value); + return element_setitem(op, i, value); } else if (PySlice_Check(item)) { Py_ssize_t start, stop, step, slicelen, newlen, i; @@ -1998,30 +2003,34 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value) } static PyObject* -element_tag_getter(ElementObject *self, void *closure) +element_tag_getter(PyObject *op, void *closure) { + ElementObject *self = _Element_CAST(op); PyObject *res = self->tag; return Py_NewRef(res); } static PyObject* -element_text_getter(ElementObject *self, void *closure) +element_text_getter(PyObject *op, void *closure) { + ElementObject *self = _Element_CAST(op); PyObject *res = element_get_text(self); return Py_XNewRef(res); } static PyObject* -element_tail_getter(ElementObject *self, void *closure) +element_tail_getter(PyObject *op, void *closure) { + ElementObject *self = _Element_CAST(op); PyObject *res = element_get_tail(self); return Py_XNewRef(res); } static PyObject* -element_attrib_getter(ElementObject *self, void *closure) +element_attrib_getter(PyObject *op, void *closure) { PyObject *res; + ElementObject *self = _Element_CAST(op); if (!self->extra) { if (create_extra(self, NULL) < 0) return NULL; @@ -2040,31 +2049,34 @@ element_attrib_getter(ElementObject *self, void *closure) } static int -element_tag_setter(ElementObject *self, PyObject *value, void *closure) +element_tag_setter(PyObject *op, PyObject *value, void *closure) { _VALIDATE_ATTR_VALUE(value); + ElementObject *self = _Element_CAST(op); Py_SETREF(self->tag, Py_NewRef(value)); return 0; } static int -element_text_setter(ElementObject *self, PyObject *value, void *closure) +element_text_setter(PyObject *op, PyObject *value, void *closure) { _VALIDATE_ATTR_VALUE(value); + ElementObject *self = _Element_CAST(op); _set_joined_ptr(&self->text, Py_NewRef(value)); return 0; } static int -element_tail_setter(ElementObject *self, PyObject *value, void *closure) +element_tail_setter(PyObject *op, PyObject *value, void *closure) { _VALIDATE_ATTR_VALUE(value); + ElementObject *self = _Element_CAST(op); _set_joined_ptr(&self->tail, Py_NewRef(value)); return 0; } static int -element_attrib_setter(ElementObject *self, PyObject *value, void *closure) +element_attrib_setter(PyObject *op, PyObject *value, void *closure) { _VALIDATE_ATTR_VALUE(value); if (!PyDict_Check(value)) { @@ -2073,6 +2085,7 @@ element_attrib_setter(ElementObject *self, PyObject *value, void *closure) Py_TYPE(value)->tp_name); return -1; } + ElementObject *self = _Element_CAST(op); if (!self->extra) { if (create_extra(self, NULL) < 0) return -1; @@ -2106,11 +2119,14 @@ typedef struct { int gettext; } ElementIterObject; +#define _ElementIter_CAST(op) ((ElementIterObject *)(op)) + static void -elementiter_dealloc(ElementIterObject *it) +elementiter_dealloc(PyObject *op) { - PyTypeObject *tp = Py_TYPE(it); + PyTypeObject *tp = Py_TYPE(op); + ElementIterObject *it = _ElementIter_CAST(op); Py_ssize_t i = it->parent_stack_used; it->parent_stack_used = 0; /* bpo-31095: UnTrack is needed before calling any callbacks */ @@ -2127,8 +2143,9 @@ elementiter_dealloc(ElementIterObject *it) } static int -elementiter_traverse(ElementIterObject *it, visitproc visit, void *arg) +elementiter_traverse(PyObject *op, visitproc visit, void *arg) { + ElementIterObject *it = _ElementIter_CAST(op); Py_ssize_t i = it->parent_stack_used; while (i--) Py_VISIT(it->parent_stack[i].parent); @@ -2162,8 +2179,9 @@ parent_stack_push_new(ElementIterObject *it, ElementObject *parent) } static PyObject * -elementiter_next(ElementIterObject *it) +elementiter_next(PyObject *op) { + ElementIterObject *it = _ElementIter_CAST(op); /* Sub-element iterator. * * A short note on gettext: this function serves both the iter() and @@ -2354,6 +2372,8 @@ typedef struct { elementtreestate *state; } TreeBuilderObject; + +#define _TreeBuilder_CAST(op) ((TreeBuilderObject *)(op)) #define TreeBuilder_CheckExact(st, op) Py_IS_TYPE((op), (st)->TreeBuilder_Type) /* -------------------------------------------------------------------- */ @@ -2444,8 +2464,9 @@ _elementtree_TreeBuilder___init___impl(TreeBuilderObject *self, } static int -treebuilder_gc_traverse(TreeBuilderObject *self, visitproc visit, void *arg) +treebuilder_gc_traverse(PyObject *op, visitproc visit, void *arg) { + TreeBuilderObject *self = _TreeBuilder_CAST(op); Py_VISIT(Py_TYPE(self)); Py_VISIT(self->pi_event_obj); Py_VISIT(self->comment_event_obj); @@ -2467,8 +2488,9 @@ treebuilder_gc_traverse(TreeBuilderObject *self, visitproc visit, void *arg) } static int -treebuilder_gc_clear(TreeBuilderObject *self) +treebuilder_gc_clear(PyObject *op) { + TreeBuilderObject *self = _TreeBuilder_CAST(op); Py_CLEAR(self->pi_event_obj); Py_CLEAR(self->comment_event_obj); Py_CLEAR(self->end_ns_event_obj); @@ -2489,11 +2511,11 @@ treebuilder_gc_clear(TreeBuilderObject *self) } static void -treebuilder_dealloc(TreeBuilderObject *self) +treebuilder_dealloc(PyObject *self) { PyTypeObject *tp = Py_TYPE(self); PyObject_GC_UnTrack(self); - treebuilder_gc_clear(self); + (void)treebuilder_gc_clear(self); tp->tp_free(self); Py_DECREF(tp); } @@ -3061,6 +3083,9 @@ typedef struct { PyObject *elementtree_module; } XMLParserObject; + +#define _XMLParser_CAST(op) ((XMLParserObject *)(op)) + /* helpers */ LOCAL(PyObject*) @@ -3751,8 +3776,9 @@ _elementtree_XMLParser___init___impl(XMLParserObject *self, PyObject *target, } static int -xmlparser_gc_traverse(XMLParserObject *self, visitproc visit, void *arg) +xmlparser_gc_traverse(PyObject *op, visitproc visit, void *arg) { + XMLParserObject *self = _XMLParser_CAST(op); Py_VISIT(Py_TYPE(self)); Py_VISIT(self->handle_close); Py_VISIT(self->handle_pi); @@ -3772,8 +3798,9 @@ xmlparser_gc_traverse(XMLParserObject *self, visitproc visit, void *arg) } static int -xmlparser_gc_clear(XMLParserObject *self) +xmlparser_gc_clear(PyObject *op) { + XMLParserObject *self = _XMLParser_CAST(op); elementtreestate *st = self->state; if (self->parser != NULL) { XML_Parser parser = self->parser; @@ -3800,11 +3827,11 @@ xmlparser_gc_clear(XMLParserObject *self) } static void -xmlparser_dealloc(XMLParserObject* self) +xmlparser_dealloc(PyObject *self) { PyTypeObject *tp = Py_TYPE(self); PyObject_GC_UnTrack(self); - xmlparser_gc_clear(self); + (void)xmlparser_gc_clear(self); tp->tp_free(self); Py_DECREF(tp); } @@ -4172,7 +4199,7 @@ static PyMemberDef xmlparser_members[] = { }; static PyObject* -xmlparser_version_getter(XMLParserObject *self, void *closure) +xmlparser_version_getter(PyObject *op, void *closure) { return PyUnicode_FromFormat( "Expat %d.%d.%d", XML_MAJOR_VERSION, @@ -4180,7 +4207,7 @@ xmlparser_version_getter(XMLParserObject *self, void *closure) } static PyGetSetDef xmlparser_getsetlist[] = { - {"version", (getter)xmlparser_version_getter, NULL, NULL}, + {"version", xmlparser_version_getter, NULL, NULL}, {NULL}, }; @@ -4229,20 +4256,20 @@ static struct PyMemberDef element_members[] = { static PyGetSetDef element_getsetlist[] = { {"tag", - (getter)element_tag_getter, - (setter)element_tag_setter, + element_tag_getter, + element_tag_setter, "A string identifying what kind of data this element represents"}, {"text", - (getter)element_text_getter, - (setter)element_text_setter, + element_text_getter, + element_text_setter, "A string of text directly after the start tag, or None"}, {"tail", - (getter)element_tail_getter, - (setter)element_tail_setter, + element_tail_getter, + element_tail_setter, "A string of text directly after the end tag, or None"}, {"attrib", - (getter)element_attrib_getter, - (setter)element_attrib_setter, + element_attrib_getter, + element_attrib_setter, "A dictionary containing the element's attributes"}, {NULL}, };