Welcome to MSDN Blogs Sign in | Join | Help
Today’s Bug is Brought to You by “Don’t Mix C and C++”

Take this for example:

#include <iostream>

#include <iomanip>

 

 

class Storage

{

public:

      Storage() : m_val( -1 ) {}

      int m_val;

};

 

class Holder

{

public:

      Holder() : m_double( 50000 ) {}

 

      union

      {

            double m_double;

            struct

            {

                  Storage m_storage;

                  int m_id;

            };

      };

};

 

int _tmain(int argc, _TCHAR* argv[])

{

      TestClass t;

      std::cout << std::setprecision(20) << t.m_double;

      return 0;

}

 

So where does 50000.03125 come from? It’s nowhere in the code.

It comes from mixing the 50000 passed into m_double and the constructor for Storage later assigning m_val to be -1.

But why? What’s the proper order things get initialized in a union? What if I have two classes each which have their own constructors, which one gets called since they both occupy the same memory?

The C++ standard says thusly:

An object of a class with a non-trivial constructor (12.1), a non-trivial copy constructor (12.8), a non-trivial destructor (12.4), or a non-trivial copy assignment operator (13.5.3, 12.8) cannot be a member of a union, nor can an array of such objects.

So, the above should result in a compiler error, but it doesn’t as of VC++ 9. In fact, if you change the code to have a named struct:

union

      {

            double m_double;

            struct MyStruct

            {

                  Storage m_storage;

                  int m_id;

            };

            MyStruct m_struct;

      };

You get the correct C2620 error:

 error C2620: member 'Holder::m_struct' of union 'Holder::<unnamed-tag>' has user-defined constructor or non-trivial default constructor

A similar coding error has existed in the Office code base for a while, but went undetected because of how the memory corruption occurs. Since 50000 becomes 50000.03125, nobody noticed that the sub-pixel rendering of certain shapes was slightly incorrect until a separate, seemingly unrelated assert kept popping up saying two numbers it expected to be equal were not. If the corruption had occurred in an integer, or the storage class and m_id were in a different order or the compiler had correctly shown an error on this or the original developer had used a named struct, the problem would have been easily identified. But since all these things didn't happen, we have small deviations in the original double value.

What did happen was good test automation, which routinely hit the assertion and we were able to trace the root cause of several bugs and fix the problem.

Posted: Wednesday, January 07, 2009 10:55 PM by Chris Becker
Filed under: ,

Comments

Cory Nelson said:

Guess you'll be having more fun with C++0x support then, where non-trivial classes can be part of unions!

# January 7, 2009 9:20 PM
Leave a Comment

(required) 

(required) 

(optional)

(required) 

  
Enter Code Here: Required

Comment Notification

If you would like to receive an email when updates are made to this post, please register here

Subscribe to this post's comments using RSS

Page view tracker