A customer encountered the following problem:
class Shape { public: virtual bool Is2D() { return false; } }; class Shape2D : public Shape { public: virtual bool Is2D() { return true; } }; Shape *FindShape(Cookie cookie); void BuyPaint(Cookie cookie) { Shape2D *shape = static_cast<Shape2D *>(FindShape(cookie)); if (shape->Is2D()) { .. do all sorts of stuff ... } }
The BuyPaint
function converts the cookie back
to a Shape
object, and then checks if the object
is a Shape2D
object by calling Is2D
.
If so, then it does some more stuff to figure out what type of paint
to buy.
(Note to nitpickers: The actual scenario was not like this, but I presented it this way to illustrate the point. If you say “You should’ve used RTTI” or “You should’ve had a BuyPaint method on the Shape class”, then you’re missing the point.)
The programmers figured they’d save some typing by casting the
result of FindShape
to a Shape2D
right away,
because after all, since Is2D
is a virtual
method, it will call the right version of the function,
either Shape::Is2D
or Shape2D::Is2D
,
depending on the actual type of the underlying object.
But when compiler optimizations were turned on, they discovered
that the call to Is2D
was optimized away,
and the BuyPaint
function merely assumed
that it was always operating on a Shape2D
object.
It then ended up trying to buy paint even for one-dimensional objects
like points and lines.
Compiler optimization bug? Nope. Code bug due to reliance on undefined behavior.
The C++ language says (9.3.1)
“If a nonstatic member function of a class X
is called for an object that is not of type X
,
or of a type derived from X
, the behavior is undefined.”
In other words,
if you are invoking a method on an object of type X
,
then you are promising that it really is of type X
,
or a class derived from it.
The code above violates this rule,
because it is invoking the Is2D
method
on a Shape2D*
,
which therefore comes with the promise
“This really is a Shape2D
object
(or something derived from it).”
But this is a promise the code cannot deliver on, because the
object returned by FindShape
might be a simple
Shape
.
The compiler ran with the (false) promise and said,
“Well, since you are guaranteeing that the object is at least
a Shape2D
,
and since I have studied your code and determined that no
classes which further derive from Shape2D
override
the Is2D
method,
I have therefore proved that the final overrider is
Shape2D::Is2D
and can therefore
inline that method.”
Result: The compiler inlines Shape2D::Is2D
,
which returns true
, so the “if” test can be optimized out.
The compiler can assume that BuyPaint
is always
called with cookies that represent two-dimensional objects.
The fix is to do the annoying typing that the original authors were trying to avoid:
void BuyPaint(Cookie cookie) { Shape *shape = FindShape(cookie); if (shape->Is2D()) { Shape2D *shape2d = static_cast<Shape2D *>(shape); .. do all sorts of stuff (with shape2d) ... } }
0 comments