It’s like dialing the wrong number and then going on as if you’re talking to the person you intended to anyway
Today I was helping one of my teammates debug an issue he had been looking at for a while. The symptom was a crash here:
HRESULT CGadget::GetPropertyValue(PROPERTYID propertyId, VARIANT *pRetVal)
… ad nauseam …
pRetVal->vt = VT_EMPTY; <<<<< CRASH!
and at this point in time, pRetVal is null, and propertyId is some bogus value. Hmm, interesting. Tracing it back, we see that there is a thread in a loop accepting requests from other threads and dispatching the appropriate type of request (after all this is in a popular UI framework…) and depending on the type of request, different parameter types will be used. The request that makes everything happens to be one for getting the gadget’s bounding rectangle which just redirects to somebody else’s implementation:
HRESULT CGadget::get_BoundingRectangle(Rect *pRect)
However we see that the disassembly for the call to CGadget::get_BoundingRectangle instruction is actually calling into our CGadget::GetPropertyValue…and even worse, the value being passed as the propertyId parameter is the value of the pointer pRect! So what’s going on?
It turns out that CGadget has an interesting multiple inheritance structure, where it implements a number of interfaces, in our simplified example let’s call them Interface1 and Interface2.
get_BoundingRectangle happens to be the on the 5th slot in Interface1’s vtable, and GetPropertyValue happens to be on the 5th slot of Interface2’s vtable.
The guy that holds the reference to CGadget actually only talks to it through Interface2, but this interface is being obtained in a somewhat peculiar manner:
_pI2 = pI2;
CTrinket *pTrinket = new CTrinket(reinterpret_cast<Interface2*>(this));
So here it is, the culprit is that the code is doing a reinterpret_cast on an interface – even though we know the object inherits (implements) from this class/interface, reinterpret_cast will not fix up the offsets so that the pointer returned points to Interface2 – it will just grab the memory address of the this object and treat it as if it already has a pointer to an Interface2 object; the problem is that the in-memory layout for CGadget, has an Interface1 at the start, and the Interface2 objects starts sizeof(Interface1) bytes later!
This is one of those cases where a C-style cast (or really, a static_cast) would likely be the appropriate choice instead.
Edit: or actually, don’t even cast it, as the compiler will know how to cast this into one of its base classes (thanks Raymond for pointing that out :))