C implementations of the tp_compare slot usually expect both of their 
arguments to have the type for which that slot is defined (this isn't 
documented anywhere that I can find, but many core types assume this, 
and extension authors are likely to do the same). A problem occurs if a 
user-defined nb_coerce slot (__coerce__) returns objects with different 
types. Avoid the problem by refusing to call a non-Python tp_compare 
unless the arguments have the same type. Indiscriminately calling 
tp_compare with different types is wrong as long as there are 
implementations that don't check the type of the second argument, but 
Python implementations should be allowed to receive a different type.

Other options for fixing the problem:
  - Make PyNumber_CoerceEx require that the results have the same 
type. This would prevent a user-defined __coerce__ from 
communicating an arbitrary object to a user-defined __cmp__. 
Furthermore, there are uses for coerce besides comparison, and those 
uses might not require identical types.
  - Change tp_compare implementations to check the type of the 
second argument. This might be the most desirable long-term solution, 
but as I understand it, tp_richcompare is preferred over tp_compare for 
new code, and old (current) code is likely assuming that the types are 
the same.

Addresses bug #980352

Logged In: YES 
user_id=4771

Is your patch complete?  There seem to be a lot of places in object.c that call tp_compare slots.  Shouldn't we try to make sure that none of these places can ever be called with two different types?

It also looks like we have to do something special about a value of &_PyObject_SlotCompare in tp_compare, because this function from typeobject.c checks itself the type of its arguments and does specific things with them.  This should be thought about.

Moreover there is the issue of the user subclassing built-in types, e.g. PyInt_Type.tp_compare is capable of comparing different user subclasses of "int" for each other.

Logged In: YES 
user_id=908995

The patch is intended to be complete, but it's certainly possible that I 
missed something. Every other call to tp_compare checks that the 
operands have the same type or the same compare function; the only 
exceptions are if one of operands passes PyInstance_Check or if it's 
_PyObject_SlotCompare (it's treated specially in try_3way_compare).

I'm actually more concerned about restricting too much than having 
missing something. The tests I wrote try to make sure that the 
user-visible changes are limited to the absence of a crash, but I'm not 
sure that I didn't accidently break some code that used to work.

Comparing subclasses of builtins to builtins still works, too. On line 599 
of the post-patch object.c, we see:

	/* If both have the same (non-NULL) tp_compare, use it. */
	if (f != NULL && f == w->ob_type->tp_compare) {
		c = (*f)(v, w);
		return adjust_tp_compare(c);
	}

which takes care of that case.

Logged In: YES 
user_id=4771

Sorry, I did not read well enough what object.c was doing. 
I agree now that your patch should protect again all
crashes, and that the concern is whether it is too restrictive.

Before rich comparison, there was no obvious sane way to
directly compare objects of different types in C extension
types.  I remember having the problem back in Python 1.5.2;
at the time I solved the problem with an nb_coerce that
returned "fake" objects of a common type, and a tp_compare
that decoded these fake objects.  Now I see that it would
have been simpler to solve the problem by either:

1) writing a nb_coerce that doesn't do anything (but which
is implemented; otherwise tp_compare don't get called at all).
This is what would be broken by the present patch.

2) easier yet, just making sure that both types have the
same tp_compare slot.  We don't need nb_coerce at all in
this case.  The tp_compare would check for the type of both
arguments anyway.  The present patch wouldn't break that.

There is no other reasonable safe way that I can of to fix
the problem (1)...  But admitedly the whole issue is a bit
far-fetched.  So I'd suggest that the issue is deferred
until Python 2.4 is out, and then we put the patch into the
pre-2.5 CVS head.  It's a bit late in the 2.4 release
process to risk breaking potential C extension modules.

A note about the patch: in the first comment you say that v
and w have different types, but that's potentially wrong --
they could have the same type but no tp_compare slot.

The new single test introduced after PyNumber_CoerceEx()
might be too restrictive: it would be enough if v and w have
the same tp_compare slot after coercion.  Pushing things a
bit over the edge of sanity, we might ask "what if coercion
returns old-style instances, or something with
_PyObject_SlotCompare?".  So maybe we should try the whole
try_3way_compare again after coercion (without of course
trying to coerce more than once!).

Logged In: YES 
user_id=4771

I added a minor change after the coercion, to accept two 
objects not necessarily of the same type but with the same 
tp_compare.

Applied as planned in the 2.5 CVS as:

    Objects/object.c  rev 2.225
    Lib/test/test_coercion.py  rev 1.8