Alex Rivera | Logout

C++ calling completely wrong (virtual) method of an object

Asked 2011-01-25T21:20:10.477
16

I have some C++ code (written by someone else) which appears to be calling the wrong function. Here's the situation:

UTF8InputStreamFromBuffer* cstream = foo();
wstring fn = L"foo";
DocumentReader* reader;

if (a_condition_true_for_some_files_false_for_others) {
    reader = (DocumentReader*) _new GoodDocumentReader();
} else {
    reader = (DocumentReader*) _new BadDocumentReader();
}

// the crash happens inside the following call
// when a BadDocumentReader is used
doc = reader->readDocument(*cstream, fn);

The files for which the condition is true are processed fine; the ones for which it is false crash. The class hierarchy for DocumentReader looks like this:

class GenericDocumentReader {
    virtual Document* readDocument(InputStream &strm, const wchar_t * filename) = 0;
}

class DocumentReader : public GenericDocumentReader {
    virtual Document* readDocument(InputStream &strm, const wchar_t * filename) {
        // some stuff
    }
};

class GoodDocumentReader : public DocumentReader {
    Document* readDocument(InputStream & strm, const wchar_t * filename);
}

class BadDocumentReader : public DocumentReader {
    virtual Document* readDocument(InputStream &stream, const wchar_t * filename);
    virtual Document* readDocument(const LocatedString *source, const wchar_t * filename);
    virtual Document* readDocument(const LocatedString *source, const wchar_t * filename, Symbol inputType);
}

The following are also relevant:

class UTF8InputStreamFromBuffer : public wistringstream {
    // foo
};
typedef std::basic_istream<wchar_t> InputStream;

Running in a Visual C++ debugger, it shows that the readDocument call on a BadDocumentReader is calling not

readDocument(InputStream&, const wchar_t*)

but rather

readDocument(const LocatedString* source, const wchar_t *, Symbol)

                    
                    
                    
Edit
Report

1 Answer

0

Based on the assembly, it seems pretty clear that the binding is dynamic and from the first entry of the vtable. The question is which virtual table!?! I would suggest you use a static_cast instead of a C-style cast (of course, @VJo: dynamic_cast is not needed in this case!). There is nothing in the standard that requires a pointer BadDocumentReader* ptr to have the same actual value (address) as its cast static_cast<DocumentReader*>(ptr). This would explain why it binds the call to the first entry of the vtable of BadDocumentReader and not to the vtable of its base class. And, btw, you shouldn't need a cast at all in this case.

One possibility that doesn't really agree with the asm, but still good to know. Because you create the BadDocumentReader in the same scope as you are calling the reader->readDocument, the compiler gets a little too clever and decides that it can resolve the call without having to look it up in the vtable dynamically. This is because it knows that the "real" type of the reader pointer is actually BadDocumentReader. So it bipasses the vtable and binds the call statically. At least, that is one possibility which I had happen to me in an almost identical situation. Based on the asm, however, I'm pretty sure the first possibility is the one occurring in your case.

answered 2011-01-26T01:45:25.747

Your Answer