On Mon, 1 Dec 2014 21:38:45 +0000 Nathaniel Smith <n...@pobox.com> wrote: > > object_set_class is responsible for checking whether it's okay to take > an object of class "oldto" and convert it to an object of class > "newto". Basically it's goal is just to avoid crashing the interpreter > (as would quickly happen if you e.g. allowed "[].__class__ = dict"). > Currently the rules (spread across object_set_class and > compatible_for_assignment) are: > > (1) both oldto and newto have to be heap types > (2) they have to have the same tp_dealloc > (3) they have to have the same tp_free > (4) if you walk up the ->tp_base chain for both types until you find > the most-ancestral type that has a compatible struct layout (as > checked by equiv_structs), then either > (4a) these ancestral types have to be the same, OR > (4b) these ancestral types have to have the same tp_base, AND they > have to have added the same slots on top of that tp_base (e.g. if you > have class A(object): pass and class B(object): pass then they'll both > have added a __dict__ slot at the same point in the instance struct, > so that's fine; this is checked in same_slots_added). > > The only place the code assumes that it is dealing with heap types is > in (4b)
I'm not sure. Many operations are standardized on heap types that can have arbitrary definitions on static types (I'm talking about the tp_ methods). You'd have to review them to double check. For example, a heap type's tp_new increments the type's refcount, so you have to adjust the instance refcount if you cast it from a non-heap type to a heap type, and vice-versa (see slot_tp_new()). (this raises the interesting question "what happens if you assign to __class__ from a __del__ method?") > -- same_slots_added unconditionally casts the ancestral types > to (PyHeapTypeObject*). AFAICT that's why step (1) is there, to > protect this code. But I don't think the check actually works -- step > (1) checks that the types we're trying to assign are heap types, but > this is no guarantee that the *ancestral* types will be heap types. > [Also, the code for __bases__ assignment appears to also call into > this code with no heap type checks at all.] E.g., I think if you do > > class MyList(list): > __slots__ = () > > class MyDict(dict): > __slots__ = () > > MyList().__class__ = MyDict() > > then you'll end up in same_slots_added casting PyDict_Type and > PyList_Type to PyHeapTypeObjects and then following invalid pointers > into la-la land. (The __slots__ = () is to maintain layout > compatibility with the base types; if you find builtin types that > already have __dict__ and weaklist and HAVE_GC then this example > should still work even with perfectly empty subclasses.) > > Okay, so suppose we move the heap type check (step 1) down into > same_slots_added (step 4b), since AFAICT this is actually more correct > anyway. This is almost enough to enable __class__ assignment on > modules, because the cases we care about will go through the (4a) > branch rather than (4b), so the heap type thing is irrelevant. > > The remaining problem is the requirement that both types have the same > tp_dealloc (step 2). ModuleType itself has tp_dealloc == > module_dealloc, while all(?) heap types have tp_dealloc == > subtype_dealloc. Here again, though, I'm not sure what purpose this > check serves. subtype_dealloc basically cleans up extra slots, and > then calls the base class tp_dealloc. So AFAICT it's totally fine if > oldto->tp_dealloc == module_dealloc, and newto->tp_dealloc == > subtype_dealloc, so long as newto is a subtype of oldto -- b/c this > means newto->tp_dealloc will end up calling oldto->tp_dealloc anyway. > OTOH it's not actually a guarantee of anything useful to see that > oldto->tp_dealloc == newto->tp_dealloc == subtype_dealloc, because > subtype_dealloc does totally different things depending on the > ancestry tree -- MyList and MyDict above pass the tp_dealloc check, > even though list.tp_dealloc and dict.tp_dealloc are definitely *not* > interchangeable. > > So I suspect that a more correct way to do this check would be something like > > PyTypeObject *old__real_deallocer = oldto, *new_real_deallocer = newto; > while (old_real_deallocer->tp_dealloc == subtype_dealloc) > old_real_deallocer = old_real_deallocer->tp_base; > while (new_real_deallocer->tp_dealloc == subtype_dealloc) > new_real_deallocer = new_real_deallocer->tp_base; > if (old_real_deallocer->tp_dealloc != new_real_deallocer) > error out; Sounds good. > Module subclasses would pass this check. Alternatively it might make > more sense to add a check in equiv_structs that > (child_type->tp_dealloc == subtype_dealloc || child_type->tp_dealloc > == parent_type->tp_dealloc); I think that would accomplish the same > thing in a somewhat cleaner way. There's no "child" and "parent" types in equiv_structs(). Regards Antoine. _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
