On Tue., 10 Dec. 2019, 5:17 am MRAB, <[email protected]> wrote:
> On 2019-12-09 18:22, Christian Tismer wrote:
> >
> >
> > Hi Nick,
> >
> > after staring long at the code, I fount something funny in
> > typeobject.c #286 ff:
> >
> >
> > static void
> > type_mro_modified(PyTypeObject *type, PyObject *bases) {
> > /*
> > Check that all base classes or elements of the MRO of type are
> > able to be cached. This function is called after the base
> > classes or mro of the type are altered.
> >
> > Unset HAVE_VERSION_TAG and VALID_VERSION_TAG if the type
> > has a custom MRO that includes a type which is not officially
> > super type, or if the type implements its own mro() method.
> >
> > Called from mro_internal, which will subsequently be called on
> > each subclass when their mro is recursively updated.
> > */
> > Py_ssize_t i, n;
> > int custom = (Py_TYPE(type) != &PyType_Type);
> > int unbound;
> > PyObject *mro_meth = NULL;
> > PyObject *type_mro_meth = NULL;
> >
> > if (!PyType_HasFeature(type, Py_TPFLAGS_HAVE_VERSION_TAG))
> > return;
> >
> > if (custom) {
> > _Py_IDENTIFIER(mro);
> > mro_meth = lookup_maybe_method(
> > (PyObject *)type, &PyId_mro, &unbound);
> > if (mro_meth == NULL)
> > goto clear;
> > type_mro_meth = lookup_maybe_method(
> > (PyObject *)&PyType_Type, &PyId_mro, &unbound);
> > if (type_mro_meth == NULL)
> > goto clear;
> > if (mro_meth != type_mro_meth)
> > goto clear;
> > Py_XDECREF(mro_meth);
> > Py_XDECREF(type_mro_meth);
> > }
> >
> >
> > Look at the "if (custom)" clause.
> > "mro_meth = lookup_maybe_method(" uses lookup_maybe_method which
> > gives a borrowed reference. The same holds for "type_mro_meth".
> >
> > But then both are decreffed, which IMHO is not correct.
> >
> Look at what happens at the label "clear": it DECREFs them.
>
> If mro_meth != NULL or mro_meth != type_mro_meth, they'll get DECREFed
> at "clear".
>
I believe Christian's point is that this entire "if (custom) {" branch
looks suspicious, as it assumes "lookup_maybe_method" will increment the
refcount on the returned object. If that assumption is incorrect, we're
going to get DECREFs without a preceding INCREF.
The specific code path is also obscure enough that it's plausible the test
suite may not currently cover it (as it requires doing something that calls
"type_mro_modified" on a type with a custom metaclass).
Cheers,
Nick.
>
_______________________________________________
Python-Dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at
https://mail.python.org/archives/list/[email protected]/message/QUVDUH6X2UKULIDTZQR67LECT54WOCZN/
Code of Conduct: http://python.org/psf/codeofconduct/
