Amin Mesbah

Dangers of the Default Copy Constructor

Posted on 2017-01-07

Learning C++ is hard. Arrays, pointers, compilation, the stack and the heap, and memory allocation all seem straightforward to those versed in their subtleties. However, the beginner is confronted with a cluster of difficult concepts demanding seemingly abstruse knowledge about the C++ spec. and the inner workings of computers. These matters are complicated further when combined with classes, objects, and the host of concepts associated with object-oriented programming. The syntax, subtle and unforgiving, hardly makes the initial steps of the journey any easier.

Tutoring C++ has provided me with ample opportunity to clarify and explain such concepts. I've become familiar with the pain points of the first two semesters of C++, and I'm confident in my ability to help students work through them. Yet I am still a beginner and constantly learning new things. Students occasionally run into perplexing bugs that make quite evident the limits of my understanding.

On one such occasion, I was helping a student debug her code for a string class. Her professor had provided a header file, MyString.h, and a test program, my_string_test.cpp. Both of these, the professor had warned, were not to be altered. The student's task was to implement the class in MyString.cpp.

For the sake of clarity (not to mention adherence to academic ethics), the code here included is greatly simplified. Numerous methods and overloaded operators are omitted.

MyString.h

#ifndef MyString_H
#define MyString_H

#include <iostream>
#include <cstring>

class MyString;
std::ostream &operator <<(std::ostream &strm, MyString &str);


class MyString
{
    public:
        MyString();
        MyString(const char *str);
        ~MyString();

        int str_length() const;
        const char* get_string() const;

        friend std::ostream &operator <<(std::ostream &strm, MyString &str);

    private:
        char *the_string;
        // The number of characters in the string up to, but not
        // including, the null terminator '\0'.
        int length;
};
#endif

my_string_test.cpp

#include "MyString.h"

int main()
{
    // String literal constructor
    MyString first("A string literal.");
    std::cout << "first: " << first << std::endl;

    // Copy constructor
    MyString second(first);
    std::cout << "second: " << second << std::endl;

    return 0;
}

The class keeps track of a null-terminated array of characters. It has two member variables:

• the_string is a char* to the memory where the null-terminated string is stored.
• length is an int that stores the number of characters not including the null terminator.

When she came in for help, the student had already implemented and tested most of the class. Her implementation looked something like this:

MyString.cpp

#include "MyString.h"
#include <iostream>
#include <cstring>

MyString::MyString()
{
    length = 0;
    the_string = new char[1];
    the_string[0] = '\0';
}

MyString::MyString(const char *str)
{
    length = std::strlen(str);

    the_string = new char[length + 1];

    for (int i=0; i < length; i++)
    {
        the_string[i] = str[i];
    }

    the_string[length] = '\0';
}

MyString::~MyString()
{
    delete [] the_string;
}

int MyString::str_length() const
{
    return length;
}

const char* MyString::get_string() const
{
    return the_string;
}

std::ostream &operator <<(std::ostream &strm, MyString &str)
{
    strm << str.the_string;
    return strm;
}

The Problem

Until she had implemented the destructor, MyString::~MyString(), everything had apparently been working perfectly. Running the program, I was surprised to see the following debug errors:

Overlooking these somewhat enigmatic error messages, I commented out the destructor's definition. The program executed without any issues:

The destructor was simple, merely freeing the memory pointed to by the_string. I wondered how it could be causing these problems. I decided to step through the program line by line in the Visual Studio debugger. We found that the debug errors didn't appear until the execution of return 0 at the end of main(). Baffling.

Visual Studio's disassembly view divulged the details of what the processor was being told to do. There were 2 calls to MyString::~MyString() (1 for each MyString object) after return 0:

[...]
0027645F  cmp         esi,esp  
00276461  call        __RTC_CheckEsp (0271352h)  
    return 0;
00276466  mov         dword ptr [ebp-0F8h],0  
00276470  mov         byte ptr [ebp-4],0  
00276474  lea         ecx,[second]  
00276477  call        MyString::~MyString (02711CCh)  
0027647C  mov         dword ptr [ebp-4],0FFFFFFFFh  
00276483  lea         ecx,[first]  
00276486  call        MyString::~MyString (02711CCh)  
0027648B  mov         eax,dword ptr [ebp-0F8h]  
}
[...]

The first destructor call would execute as expected, but the second would raise the error messages. A few carefully observed runs through the debugger revealed the problem: The first MyString object's destructor was freeing memory for both objects' pointers. This was the state after that first call:

The first destructor frees memory for both objects! (full screenshot)

Note that first.the_string and second.the_string are both pointing to the same memory address! The second destructor was trying to free the same memory as the first. Memory can't be freed twice, so an exception was raised. Finally we understood the cause of the error messages.

When two pointers reference the same memory address, they are said to be 'aliased'. Pointer aliasing is a harbinger of untold woe, and should generally be avoided. I searched through the student's code, but I could find no statement that could have aliased the two the_string pointers. How could it be happening if the student had written no code to make it happen? Many minutes were spent carefully stepping through each line of the code before we found the unexpected source of the problem: The professor's code.

The professor had included a test of the MyString class copy constructor in my_string_test.cpp, but no prototype for it in MyString.h. The student, having been forbidden to alter these files, had not defined the copy constructor in MyString.cpp. One would expect the compiler to loudly complain about such a situation, but it appeared instead to be implicitly creating a constructor of its own:

[...]
    MyString second(first);
00276415  push        8  
00276417  lea         ecx,[second]  
0027641A  call        MyString::__autoclassinit2 (02713CFh)  
0027641F  mov         eax,dword ptr [first]  
00276422  mov         dword ptr [second],eax  
00276425  mov         ecx,dword ptr [ebp-18h]  
00276428  mov         dword ptr [ebp-28h],ecx  
0027642B  mov         byte ptr [ebp-4],1  
[...]

This was the state after executing that second mov. Note the aliased pointers:

The default copy constructor sets both pointers to the same memory address! (full screenshot)

After a bit of research, we learned that when no copy constructor is defined, the compiler will automatically generate a default copy constructor:

A compiler-generated copy constructor sets up a new object and performs a memberwise copy of the contents of the object to be copied. If base class or member constructors exist, they are called; otherwise, bitwise copying is performed.

This copy constructor performs naive memberwise assignment, copying the values of each member variable of one object to the corresponding variable in the other. This can be adequate for extremely basic classes, but proves to be disastrous when classes have pointer member variables. The address of one pointer is directly copied to another, and so it went with the_string.

The Solution

As is the case with many errors, once we understood its cause, it was trivial to fix. The student created a proper copy constructor and, in an entirely justifiable violation of the rules, added a prototype for it to MyString.h.

// MyString.cpp

MyString::MyString(MyString &obj)
{
    length = obj.length;

    the_string = new char[length + 1];

    for (int i=0; i < length; ++i)
    {
        the_string[i] = obj.the_string[i];
    }

    the_string[length] = '\0';
}

Stepping through the code a final time, we could see that, after this new copy constructor was called, each MyString object had its own memory:

Each pointer has its own address (full screenshot)

There was no more pointer aliasing, and both destructors freed the appropriate memory without issue:

First destructor successful (full screenshot)

Second destructor successful (full screenshot)

Lessons Learned

In retrospect, the cause of this bug was right before our eyes. Those more experienced with C++ probably would have recognized it immediately, but it took us quite a while to even figure out where to look. A number of factors contributed to this:

• We assumed that there were no mistakes in the code provided by the professor. This assumption delayed us significantly. It misdirected our analytical efforts and limited the questions we asked. In the end, all code is written by fallible humans (or by code which was in turn written by fallible humans). Don't assume code is bug free, verify that it is.
• We initially found Visual Studio's error messages to be quite cryptic, and made the unfortunate mistake of not closely examining them. If we had put in a bit more effort, we would have noticed at least 2 valuable hints:
  • "Access violation"
  • "Heap corruption"
• We weren't familiar with what the compiler does behind the scenes when constructors and destructors aren't explicitly defined.

Though time consuming and occasionally frustrating, this was a valuable opportunity to develop skills of careful observation, thoughtful questioning, and systematic experimentation that are so essential to developing software. Both student and tutor departed with new knowledge and understanding, and perhaps a healthy fear of leaving special methods undefined.