check that a member function really operates on an object of the expected type

check that a member function really operates on an object of the expected type

[Erik]

[-- Attachment #1: Type: text/plain, Size: 960 bytes --]

A common kind of bug is that a member function (method) operates on an
object that is not (any more) of the expected type. This can happen in
badly designed code, with redundant parent references (that become
stale) and destructors that are misused to do something else than
releasing resources. To catch these errors, one would have to add an
assertion in the form of a dynamic_cast to the beginning of every method:
void Some_Type::some_method() {
    dynamic_cast<Some_Type
&>(static_cast<Root_Ancestor_Type_Of_Some_Type &>(*this));
    //  do something
}

This will throw std::bad_cast if the bug in question is encountered. Of
course it is not practically possible to add such an assertion to the
beginning of every method, but there should be a compiler flag to enable
this (in debug builds). Is there such a flag? Is there a feature request
for it? If not, I will add one. I attach a little example of this bug in
a real but trimmed down piece of code.

[-- Attachment #2: prov.cc --]
[-- Type: text/x-c++src, Size: 1430 bytes --]

#include <cassert>

namespace UI { //  the widget set (UI toolkit)

struct Panel {
	Panel (Panel * const Parent) :
		_parent(Parent), _firstchild(0), _lastchild(0)
	{
		assert(Parent != this);
		if (_parent) {
			_next = _parent->_firstchild;
			_prev = 0;
			if (_next)
				_next->_prev = this;
			else
				_parent->_lastchild = this;
			_parent->_firstchild = this;
		} else
			_prev = _next = 0;
	}

	virtual ~Panel() {
		while (_firstchild)
			delete _firstchild;
		if (_parent) {
			if (_prev)
				_prev->_next = _next;
			else
				_parent->_firstchild = _next;
			if (_next)
				_next->_prev = _prev;
			else
				_parent->_lastchild = _prev;
		}
	}

	Panel * parent() const {return _parent;}

private:
	Panel * _parent;
	Panel *_next, * _prev;
	Panel *_firstchild, *_lastchild;
};

}


//  the main user interface of the application
struct Main_User_Interface : public UI::Panel {
	Main_User_Interface () : UI::Panel (0) {}
	void set_need_redraw() {
		dynamic_cast<Main_User_Interface &>(static_cast<UI::Panel &>(*this));
	}
};

//  a little window that can be open in the main user interface
struct Some_Window : public UI::Panel {
	Some_Window (Main_User_Interface & Parent) :
		UI::Panel (&Parent), m_parent(Parent)
	{}
	~Some_Window () {m_parent.set_need_redraw();}
private:
	Main_User_Interface & m_parent;
};

int main() {
	Main_User_Interface main_user_interface;
	new Some_Window (main_user_interface);
	return 0;
}

Re: check that a member function really operates on an object of the expected type

[Ian Lance Taylor]

Erik <esigra@gmail.com> writes:

> This will throw std::bad_cast if the bug in question is encountered. Of
> course it is not practically possible to add such an assertion to the
> beginning of every method, but there should be a compiler flag to enable
> this (in debug builds). Is there such a flag? Is there a feature request
> for it?

There is no such flag or feature request.  Moreover, even if there were
such a flag, it would not work reliably for your example.  If an object
has been destroyed, what dynamic_cast will return is essentially random.
I would argue against adding such a feature to the compiler.  This is an
area better addressed by -fmudflap or valgrind.

Ian

Re: check that a member function really operates on an object of the expected type

[Erik]

Ian Lance Taylor skrev:
> Erik <esigra@gmail.com> writes:
>   
>> This will throw std::bad_cast if the bug in question is encountered. Of
>> course it is not practically possible to add such an assertion to the
>> beginning of every method, but there should be a compiler flag to enable
>> this (in debug builds). Is there such a flag? Is there a feature request
>> for it?
>>     
>
> even if there were such a flag, it would not work reliably for your example.

It must work reliably for my example (and it does). This is explained in
"The C++ Programming Language" by Bjarne Stroustrup. Quote from chapter
15.4.3 (page 414):
   A class is more than simply a region of memory (§4.9.6). A class
object is built from "raw memory" by its constructors and it reverts to
"raw memory" as its destructors are executed. Construction is bottom up
and destruction is top down, and a class object is an object to the
extent that it has been constructed or destroyed. This reflects the
rules for RTTI, exception handling (§14.4.7), and virtual function.
   It is extremely unwise to rely on details of the order of
construction and destruction, but that order can be observed by calling
virtual functions, dynamic_cast, or typeid (§15.4.4) at a point where
the object isn't complete. For example, if the constructor for Component
in the hierarchy from §15.4.2 calls a virtual function, it will invoke a
version defined for Storable or Component, but not one from Receiver,
Transmitter, or Radio. At that point of construction, the object isn't
yet a Radio; it is merely a partially constructed object. It is best to
avoid calling virtual functions during construction and destruction.


> This is an area better addressed by -fmudflap or valgrind.
>   

Valgrind only keeps track of whether a certain piece of memory is
allocated and initialized. To use Bjarne Stroustrup's words, it works on
the "raw memory" level. It does not understand C++ semantics, object
construction/destruction or dynamic_casts. It will not detect access to
a piece of memory that was part of an object but has reverted to "raw
memory" by a destructor. So it can not detect the bug in question.
(Since the object is not completely destroyed yet, its memory block is
still allocated, so Valgrind will not react on the bug. I do not know
what -fmudflap can do.)

Re: check that a member function really operates on an object of the expected type

[Ian Lance Taylor]

Erik <esigra@gmail.com> writes:

> Ian Lance Taylor skrev:
>> Erik <esigra@gmail.com> writes:
>>   
>>> This will throw std::bad_cast if the bug in question is encountered. Of
>>> course it is not practically possible to add such an assertion to the
>>> beginning of every method, but there should be a compiler flag to enable
>>> this (in debug builds). Is there such a flag? Is there a feature request
>>> for it?
>>>     
>>
>> even if there were such a flag, it would not work reliably for your example.
>
> It must work reliably for my example (and it does).

Perhaps I misunderstood your example.  It seemed that your class was
holding a reference to an object which had been passed to delete.  If
that is what was happening, then my statement is correct.

If you are instead holding a reference to a partially but not completely
destroyed object, then I agree that dynamic_cast can work correctly.
However, I personally don't see a sufficient reason to add yet another
compiler option to support an arguably broken programming style.

Ian

Re: check that a member function really operates on an object of the expected type

[Erik]

Ian Lance Taylor skrev:
> If you are instead holding a reference to a partially but not completely
> destroyed object, then I agree that dynamic_cast can work correctly.
> However, I personally don't see a sufficient reason to add yet another
> compiler option to support an arguably broken programming style.
>   

The purpose of the compiler option is certainly not to support a broken
programming style, but to detect it! (I recently fixed 2 such bugs in a
software with over 100 000 lines of C++ and would appreciate some help
to see if there are more of them.)