Skip to content

Commit 1d6a2e6

Browse files
picnixzencukou
picnixz
and
encukou
authored
pythongh-111178: fix UBSan for example code in extending/newtypes_tutorial docs (pythonGH-131606)
Co-authored-by: Petr Viktorin <[email protected]>
1 parent d372472 commit 1d6a2e6

File tree

5 files changed

+120
-70
lines changed

5 files changed

+120
-70
lines changed

‎Doc/extending/newtypes_tutorial.rst

+56-26
Original file line numberDiff line numberDiff line change
@@ -250,16 +250,17 @@ Because we now have data to manage, we have to be more careful about object
250250
allocation and deallocation. At a minimum, we need a deallocation method::
251251

252252
static void
253-
Custom_dealloc(CustomObject *self)
253+
Custom_dealloc(PyObject *op)
254254
{
255+
CustomObject *self = (CustomObject *) op;
255256
Py_XDECREF(self->first);
256257
Py_XDECREF(self->last);
257-
Py_TYPE(self)->tp_free((PyObject *) self);
258+
Py_TYPE(self)->tp_free(self);
258259
}
259260

260261
which is assigned to the :c:member:`~PyTypeObject.tp_dealloc` member::
261262

262-
.tp_dealloc = (destructor) Custom_dealloc,
263+
.tp_dealloc = Custom_dealloc,
263264

264265
This method first clears the reference counts of the two Python attributes.
265266
:c:func:`Py_XDECREF` correctly handles the case where its argument is
@@ -270,11 +271,31 @@ the object's type might not be :class:`!CustomType`, because the object may
270271
be an instance of a subclass.
271272

272273
.. note::
273-
The explicit cast to ``destructor`` above is needed because we defined
274-
``Custom_dealloc`` to take a ``CustomObject *`` argument, but the ``tp_dealloc``
275-
function pointer expects to receive a ``PyObject *`` argument. Otherwise,
276-
the compiler will emit a warning. This is object-oriented polymorphism,
277-
in C!
274+
275+
The explicit cast to ``CustomObject *`` above is needed because we defined
276+
``Custom_dealloc`` to take a ``PyObject *`` argument, as the ``tp_dealloc``
277+
function pointer expects to receive a ``PyObject *`` argument.
278+
By assigning to the the ``tp_dealloc`` slot of a type, we declare
279+
that it can only be called with instances of our ``CustomObject``
280+
class, so the cast to ``(CustomObject *)`` is safe.
281+
This is object-oriented polymorphism, in C!
282+
283+
In existing code, or in previous versions of this tutorial,
284+
you might see similar functions take a pointer to the subtype
285+
object structure (``CustomObject*``) directly, like this::
286+
287+
Custom_dealloc(CustomObject *self)
288+
{
289+
Py_XDECREF(self->first);
290+
Py_XDECREF(self->last);
291+
Py_TYPE(self)->tp_free((PyObject *) self);
292+
}
293+
...
294+
.tp_dealloc = (destructor) Custom_dealloc,
295+
296+
This does the same thing on all architectures that CPython
297+
supports, but according to the C standard, it invokes
298+
undefined behavior.
278299

279300
We want to make sure that the first and last names are initialized to empty
280301
strings, so we provide a ``tp_new`` implementation::
@@ -352,8 +373,9 @@ We also define an initialization function which accepts arguments to provide
352373
initial values for our instance::
353374

354375
static int
355-
Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
376+
Custom_init(PyObject *op, PyObject *args, PyObject *kwds)
356377
{
378+
CustomObject *self = (CustomObject *) op;
357379
static char *kwlist[] = {"first", "last", "number", NULL};
358380
PyObject *first = NULL, *last = NULL, *tmp;
359381

@@ -379,7 +401,7 @@ initial values for our instance::
379401

380402
by filling the :c:member:`~PyTypeObject.tp_init` slot. ::
381403

382-
.tp_init = (initproc) Custom_init,
404+
.tp_init = Custom_init,
383405

384406
The :c:member:`~PyTypeObject.tp_init` slot is exposed in Python as the
385407
:meth:`~object.__init__` method. It is used to initialize an object after it's
@@ -451,8 +473,9 @@ We define a single method, :meth:`!Custom.name`, that outputs the objects name a
451473
concatenation of the first and last names. ::
452474

453475
static PyObject *
454-
Custom_name(CustomObject *self, PyObject *Py_UNUSED(ignored))
476+
Custom_name(PyObject *op, PyObject *Py_UNUSED(dummy))
455477
{
478+
CustomObject *self = (CustomObject *) op;
456479
if (self->first == NULL) {
457480
PyErr_SetString(PyExc_AttributeError, "first");
458481
return NULL;
@@ -486,7 +509,7 @@ Now that we've defined the method, we need to create an array of method
486509
definitions::
487510

488511
static PyMethodDef Custom_methods[] = {
489-
{"name", (PyCFunction) Custom_name, METH_NOARGS,
512+
{"name", Custom_name, METH_NOARGS,
490513
"Return the name, combining the first and last name"
491514
},
492515
{NULL} /* Sentinel */
@@ -543,15 +566,17 @@ we'll use custom getter and setter functions. Here are the functions for
543566
getting and setting the :attr:`!first` attribute::
544567

545568
static PyObject *
546-
Custom_getfirst(CustomObject *self, void *closure)
569+
Custom_getfirst(PyObject *op, void *closure)
547570
{
571+
CustomObject *self = (CustomObject *) op;
548572
Py_INCREF(self->first);
549573
return self->first;
550574
}
551575

552576
static int
553-
Custom_setfirst(CustomObject *self, PyObject *value, void *closure)
577+
Custom_setfirst(PyObject *op, PyObject *value, void *closure)
554578
{
579+
CustomObject *self = (CustomObject *) op;
555580
PyObject *tmp;
556581
if (value == NULL) {
557582
PyErr_SetString(PyExc_TypeError, "Cannot delete the first attribute");
@@ -583,9 +608,9 @@ new value is not a string.
583608
We create an array of :c:type:`PyGetSetDef` structures::
584609

585610
static PyGetSetDef Custom_getsetters[] = {
586-
{"first", (getter) Custom_getfirst, (setter) Custom_setfirst,
611+
{"first", Custom_getfirst, Custom_setfirst,
587612
"first name", NULL},
588-
{"last", (getter) Custom_getlast, (setter) Custom_setlast,
613+
{"last", Custom_getlast, Custom_setlast,
589614
"last name", NULL},
590615
{NULL} /* Sentinel */
591616
};
@@ -609,8 +634,9 @@ We also need to update the :c:member:`~PyTypeObject.tp_init` handler to only
609634
allow strings [#]_ to be passed::
610635

611636
static int
612-
Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
637+
Custom_init(PyObject *op, PyObject *args, PyObject *kwds)
613638
{
639+
CustomObject *self = (CustomObject *) op;
614640
static char *kwlist[] = {"first", "last", "number", NULL};
615641
PyObject *first = NULL, *last = NULL, *tmp;
616642

@@ -689,8 +715,9 @@ First, the traversal method lets the cyclic GC know about subobjects that could
689715
participate in cycles::
690716

691717
static int
692-
Custom_traverse(CustomObject *self, visitproc visit, void *arg)
718+
Custom_traverse(PyObject *op, visitproc visit, void *arg)
693719
{
720+
CustomObject *self = (CustomObject *) op;
694721
int vret;
695722
if (self->first) {
696723
vret = visit(self->first, arg);
@@ -716,8 +743,9 @@ functions. With :c:func:`Py_VISIT`, we can minimize the amount of boilerplate
716743
in ``Custom_traverse``::
717744

718745
static int
719-
Custom_traverse(CustomObject *self, visitproc visit, void *arg)
746+
Custom_traverse(PyObject *op, visitproc visit, void *arg)
720747
{
748+
CustomObject *self = (CustomObject *) op;
721749
Py_VISIT(self->first);
722750
Py_VISIT(self->last);
723751
return 0;
@@ -731,8 +759,9 @@ Second, we need to provide a method for clearing any subobjects that can
731759
participate in cycles::
732760

733761
static int
734-
Custom_clear(CustomObject *self)
762+
Custom_clear(PyObject *op)
735763
{
764+
CustomObject *self = (CustomObject *) op;
736765
Py_CLEAR(self->first);
737766
Py_CLEAR(self->last);
738767
return 0;
@@ -765,11 +794,11 @@ Here is our reimplemented deallocator using :c:func:`PyObject_GC_UnTrack`
765794
and ``Custom_clear``::
766795

767796
static void
768-
Custom_dealloc(CustomObject *self)
797+
Custom_dealloc(PyObject *op)
769798
{
770-
PyObject_GC_UnTrack(self);
771-
Custom_clear(self);
772-
Py_TYPE(self)->tp_free((PyObject *) self);
799+
PyObject_GC_UnTrack(op);
800+
(void)Custom_clear(op);
801+
Py_TYPE(op)->tp_free(op);
773802
}
774803

775804
Finally, we add the :c:macro:`Py_TPFLAGS_HAVE_GC` flag to the class flags::
@@ -825,9 +854,10 @@ When a Python object is a :class:`!SubList` instance, its ``PyObject *`` pointer
825854
can be safely cast to both ``PyListObject *`` and ``SubListObject *``::
826855

827856
static int
828-
SubList_init(SubListObject *self, PyObject *args, PyObject *kwds)
857+
SubList_init(PyObject *op, PyObject *args, PyObject *kwds)
829858
{
830-
if (PyList_Type.tp_init((PyObject *) self, args, kwds) < 0)
859+
SubListObject *self = (SubListObject *) op;
860+
if (PyList_Type.tp_init(op, args, kwds) < 0)
831861
return -1;
832862
self->state = 0;
833863
return 0;

‎Doc/includes/newtypes/custom2.c

+10-7
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ typedef struct {
1010
} CustomObject;
1111

1212
static void
13-
Custom_dealloc(CustomObject *self)
13+
Custom_dealloc(PyObject *op)
1414
{
15+
CustomObject *self = (CustomObject *) op;
1516
Py_XDECREF(self->first);
1617
Py_XDECREF(self->last);
17-
Py_TYPE(self)->tp_free((PyObject *) self);
18+
Py_TYPE(self)->tp_free(self);
1819
}
1920

2021
static PyObject *
@@ -39,8 +40,9 @@ Custom_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
3940
}
4041

4142
static int
42-
Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
43+
Custom_init(PyObject *op, PyObject *args, PyObject *kwds)
4344
{
45+
CustomObject *self = (CustomObject *) op;
4446
static char *kwlist[] = {"first", "last", "number", NULL};
4547
PyObject *first = NULL, *last = NULL;
4648

@@ -69,8 +71,9 @@ static PyMemberDef Custom_members[] = {
6971
};
7072

7173
static PyObject *
72-
Custom_name(CustomObject *self, PyObject *Py_UNUSED(ignored))
74+
Custom_name(PyObject *op, PyObject *Py_UNUSED(dummy))
7375
{
76+
CustomObject *self = (CustomObject *) op;
7477
if (self->first == NULL) {
7578
PyErr_SetString(PyExc_AttributeError, "first");
7679
return NULL;
@@ -83,7 +86,7 @@ Custom_name(CustomObject *self, PyObject *Py_UNUSED(ignored))
8386
}
8487

8588
static PyMethodDef Custom_methods[] = {
86-
{"name", (PyCFunction) Custom_name, METH_NOARGS,
89+
{"name", Custom_name, METH_NOARGS,
8790
"Return the name, combining the first and last name"
8891
},
8992
{NULL} /* Sentinel */
@@ -97,8 +100,8 @@ static PyTypeObject CustomType = {
97100
.tp_itemsize = 0,
98101
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
99102
.tp_new = Custom_new,
100-
.tp_init = (initproc) Custom_init,
101-
.tp_dealloc = (destructor) Custom_dealloc,
103+
.tp_init = Custom_init,
104+
.tp_dealloc = Custom_dealloc,
102105
.tp_members = Custom_members,
103106
.tp_methods = Custom_methods,
104107
};

‎Doc/includes/newtypes/custom3.c

+20-13
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ typedef struct {
1010
} CustomObject;
1111

1212
static void
13-
Custom_dealloc(CustomObject *self)
13+
Custom_dealloc(PyObject *op)
1414
{
15+
CustomObject *self = (CustomObject *) op;
1516
Py_XDECREF(self->first);
1617
Py_XDECREF(self->last);
17-
Py_TYPE(self)->tp_free((PyObject *) self);
18+
Py_TYPE(self)->tp_free(self);
1819
}
1920

2021
static PyObject *
@@ -39,8 +40,9 @@ Custom_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
3940
}
4041

4142
static int
42-
Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
43+
Custom_init(PyObject *op, PyObject *args, PyObject *kwds)
4344
{
45+
CustomObject *self = (CustomObject *) op;
4446
static char *kwlist[] = {"first", "last", "number", NULL};
4547
PyObject *first = NULL, *last = NULL;
4648

@@ -65,14 +67,16 @@ static PyMemberDef Custom_members[] = {
6567
};
6668

6769
static PyObject *
68-
Custom_getfirst(CustomObject *self, void *closure)
70+
Custom_getfirst(PyObject *op, void *closure)
6971
{
72+
CustomObject *self = (CustomObject *) op;
7073
return Py_NewRef(self->first);
7174
}
7275

7376
static int
74-
Custom_setfirst(CustomObject *self, PyObject *value, void *closure)
77+
Custom_setfirst(PyObject *op, PyObject *value, void *closure)
7578
{
79+
CustomObject *self = (CustomObject *) op;
7680
if (value == NULL) {
7781
PyErr_SetString(PyExc_TypeError, "Cannot delete the first attribute");
7882
return -1;
@@ -87,14 +91,16 @@ Custom_setfirst(CustomObject *self, PyObject *value, void *closure)
8791
}
8892

8993
static PyObject *
90-
Custom_getlast(CustomObject *self, void *closure)
94+
Custom_getlast(PyObject *op, void *closure)
9195
{
96+
CustomObject *self = (CustomObject *) op;
9297
return Py_NewRef(self->last);
9398
}
9499

95100
static int
96-
Custom_setlast(CustomObject *self, PyObject *value, void *closure)
101+
Custom_setlast(PyObject *op, PyObject *value, void *closure)
97102
{
103+
CustomObject *self = (CustomObject *) op;
98104
if (value == NULL) {
99105
PyErr_SetString(PyExc_TypeError, "Cannot delete the last attribute");
100106
return -1;
@@ -109,21 +115,22 @@ Custom_setlast(CustomObject *self, PyObject *value, void *closure)
109115
}
110116

111117
static PyGetSetDef Custom_getsetters[] = {
112-
{"first", (getter) Custom_getfirst, (setter) Custom_setfirst,
118+
{"first", Custom_getfirst, Custom_setfirst,
113119
"first name", NULL},
114-
{"last", (getter) Custom_getlast, (setter) Custom_setlast,
120+
{"last", Custom_getlast, Custom_setlast,
115121
"last name", NULL},
116122
{NULL} /* Sentinel */
117123
};
118124

119125
static PyObject *
120-
Custom_name(CustomObject *self, PyObject *Py_UNUSED(ignored))
126+
Custom_name(PyObject *op, PyObject *Py_UNUSED(dummy))
121127
{
128+
CustomObject *self = (CustomObject *) op;
122129
return PyUnicode_FromFormat("%S %S", self->first, self->last);
123130
}
124131

125132
static PyMethodDef Custom_methods[] = {
126-
{"name", (PyCFunction) Custom_name, METH_NOARGS,
133+
{"name", Custom_name, METH_NOARGS,
127134
"Return the name, combining the first and last name"
128135
},
129136
{NULL} /* Sentinel */
@@ -137,8 +144,8 @@ static PyTypeObject CustomType = {
137144
.tp_itemsize = 0,
138145
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
139146
.tp_new = Custom_new,
140-
.tp_init = (initproc) Custom_init,
141-
.tp_dealloc = (destructor) Custom_dealloc,
147+
.tp_init = Custom_init,
148+
.tp_dealloc = Custom_dealloc,
142149
.tp_members = Custom_members,
143150
.tp_methods = Custom_methods,
144151
.tp_getset = Custom_getsetters,

0 commit comments

Comments
 (0)