Aussie AI
C++ Class Bugs
-
Bonus Material for "Generative AI in C++"
-
by David Spuler, Ph.D.
C++ Class Bugs
Aliasing in the overloaded assignment operator
The definition of an "operator=" function for a class should always check for an assignment to itself (i.e., of the form "x=x"). Consider the following simple MyString class:
class MyString {
private:
char* m_str;
public:
MyString() { m_str = new char[1]; m_str[0] = '\0'; }
MyString(char* s)
{
m_str = new char[strlen(s) + 1]; strcpy(m_str, s);
}
void operator =(const MyString& s);
~MyString() { delete[] m_str; }
void print() { printf("STRING: %s\n", m_str); }
};
void MyString::operator = (const MyString& s)
{
delete[] m_str; // delete old string
m_str = new char[strlen(s.m_str) + 1]; // allocate memory
strcpy(m_str, s.m_str); // copy new string
}
The above code looks fine, but this contains a hidden error that appears only if a string is assigned to itself. Consider the effect of the code:
MyString s("abc");
s = s;
s.print();
When the assignment operator is called, the argument s is the same as the object to which the member function is applied. Therefore, the addresses m_str and s.m_str are the same pointer, and the delete operator deallocates an address that is immediately used in the subsequent strlen and strcpy function calls. Thus, these operations apply to an illegal address with undefined behavior, and it fails with a crash or garbage output.
This error is an example of a general problem of aliasing in the use of overloaded operators, especially the = operator. The object to which the operator is applied is an alias for the object passed as the argument. Any modifications to the data members also affect the data in the argument object. This type of error is very difficult to track down because it occurs only for one particular special case, and this case may not occur very often. This error is not restricted to operator=, although this is its most common appearance. Similar aliasing errors may also occur in other operators such as +=, or in nonoperator member functions that accept objects of the same type.
The correct idiom to avoid this problem of aliasing is to compare the implicit pointer, this, with the address of the argument object (which must be passed as a reference type). If these addresses are the same, the two objects are identical and appropriate action can be taken for this special case. For example, in the String class the correct action when assigning a string to itself is to make no changes, and the operator= function becomes:
void MyString::operator = (const MyString& s)
{
if (this != &s) { // Correct!
delete[] m_str;
m_str = new char[strlen(s.m_str) + 1];
strcpy(m_str, s.m_str);
}
}
Bitwise copy in object assignment
There is a well-known idiom in the design of good classes that requires four major member functions:
1. Ordinary constructor(s) — Object(), Object(int), etc.
2. Copy constructor — Object(const Object &)
3. Destructor — ~Object()
4. Assignment operator — operator=(const Object &)
This idiom is good style for all classes, but is very important for particular types of classes. The most common error is a failure to supply either a copy constructor or assignment operator in a class that makes use of dynamically allocated memory. This error causes the delete operation in the destructor to be applied to memory that has already been deleted by the destructor of another object. The following naive String class illustrates the error:
class MyString2 {
private:
char* m_str;
public:
MyString2() { m_str = new char[1]; m_str[0] = '\0'; }
MyString2(char* s)
{
m_str = new char[strlen(s) + 1]; strcpy(m_str, s);
}
~MyString2() { delete[] m_str; }
};
void testMystring2()
{
MyString2 s1("abc"), s2("xyz");
s1 = s2; // Dangerous bitwise copy
}
The problem is that the assignment of one string to another defaults to a bitwise copy of the m_str field. Note that you may hear that C++ implements "memberwise" copying instead of bitwise copying, but this occurs only for data members that are themselves classes and does not apply to primitive types — m_str is a pointer which is a primitive type and therefore the str member is bitwise copied.
This means that the above assignment overwrites s1.m_str with s2.m_str, and causes both m_str fields to point to the same address. When s1 and s2 are destroyed, the delete operator in the destructor is given the same address twice, usually leading to a run-time error (double delete). The solution is to declare an overloaded assignment operator for the class, so that bitwise copying does not occur.
Bitwise copy in default copy constructor
Adding the overloaded assignment operator above means that any use of the assignment operator is no longer dangerous. However, the MyString2 class declaration still contains a similar problem because of the absence of a copy constructor.
A String variable that is initialized using another String variable is still dangerous, because the default copy constructor is also bitwise copying. Some examples of declarations that call the copy constructor are below:
MyString2 s3(s1);
MyString2 s4 = s2; // equivalent to String s4(s2);
Note that the = sign in a declaration is not the assignment operator, but is instead a form of initialization. The copy constructor is also called when objects are passed by value (i.e., not as a reference type) as arguments to a function, and also at a return statement where the return type is an object type.
The same problem with the delete operator in the destructor deallocating the same address twice occurs because the C++ compiler generates a default copy constructor that performs bitwise assignment of the m_str field (because it is a primitive type).
After the above declarations the str field of s3 is the same address as that for s1, and this is also true for s4 and s2. The solution is to declare an explicit copy constructor for the MyString2 class that allocates new memory for the m_str field of the newly constructed MyString2 object (rather than just copying the m_str pointer address). The copy constructor looks like:
MyString2::MyString2(const MyString2& s) // Copy constructor
{
m_str = new char[strlen(s.m_str) + 1]; // allocate memory
strcpy(m_str, s.m_str); // copy string
}
In general, this error from bitwise copying can occur in a number of situations, including:
1. A class uses dynamic memory, pointed to by a data member of the class object, and there is a delete operation in the destructor (risk of double-delete).
2. A class contains a file descriptor, such as a FILE* pointer from the standard C library, <stdio.h>, and the file is closed in the destructor (risk of a double-fclose error).
The error due to dynamic allocation can be solved by copying the dynamic memory inside both the copy constructor and assignment operator (as done above for the String class). The similar error, where a file descriptor is "closed" twice, is more difficult to avoid, as copying file descriptors is more difficult. One solution may be to declare the file descriptor as a static data member, and also add another static Boolean data member that indicates whether the file is currently open or closed.
Derived class copy constructor
Since the default constructor for a derived class will automatically call the default constructor for the base class (even if both are user-supplied) we would naturally expect the same effect to occur for the derived copy constructor. Unfortunately, it does not, and this expectation leads to a common C++ pitfall.
The example below illustrates the problem. When the derived copy constructor is called (e.g., in initialization, to pass an object by value or to return an object), the base data members are not copied because the base class copy constructor is never called by the derived copy constructor. Instead, the default constructor for the base class is implicitly called. So the situation is not as bad as it could be since at least we have initialized data members in the base class, but the effect is not what we usually require.
class Base {
int base_data;
public:
Base() { // Base default constructor
base_data = 0;
}
Base(const Base &b) { // Base copy constructor
base_data = b.base_data;
}
};
class Derived : public Base {
int derived_data;
public:
Derived() {
derived_data = 0;
}
Derived(const Derived &d) {
derived_data = d.derived_data;
}
};
The solution is the use of the base class initialization syntax in the function header of the derived copy constructor. The following use of ":Base(d)" causes the base class copy constructor to be called correctly:
Derived(const Derived &d) : Base(d) {
derived_data = d.derived_data;
}
Derived class assignment operator
The above error with derived class copy constructors has an almost identical companion error with the assignment operator. As with the copy constructor, we would hope that the derived class assignment operator would call the base class assignment operator to assign the base class data members. However, this does not occur, and in fact, the base class data members are unaffected by a derived class assignment operator. Consider the following class:
class Base {
int base_data;
public:
void operator=(const Base &b) { // Base assignment
base_data = b.base_data;
}
};
class Derived : public Base {
int derived_data;
public:
void operator=(const Derived &d) { // Derived assignment
derived_data = d.derived_data;
}
};
In this class, the derived assignment operator does not call the base assignment operator. The solution is the placement of an explicit call to the base class assignment operator inside the derived class assignment operator. Unlike in the copy constructor case, there is no neat initialization syntax to use. Instead, there are various ways of explicitly calling the base assignment operator; one example is shown below:
void operator=(const Derived &d) {
(Base&)(*this) = (Base&) d; // Call base assignment
derived_data = d.derived_data;
}
This expression needs some decoding: firstly "*this" is used to get the derived object that we are currently operating upon. This is then cast to a reference to a base object, and this causes the = operator to apply to a base object, and thus the base assignment operator is called. The right-hand-side of the base assignment is the derived object, but truncated to a reference to a base object — hence it is an assignment between both base subobjects of the derived objects.
The above statement is not the only way of calling the base assignment operator. Other possibilities include the use of an explicit call to the "operator=" member function:
((Base&)(*this)).operator= ( (Base&) d);
(*this).Base::operator= ( (Base&) d);
Note that in all cases, the type cast of d to Base& is unnecessary because conversion of a derived reference to a base reference will occur automatically. Howev er, it makes what is occurring a little clearer.
Calling exit from a destructor
The exit library function should never be called from within a destructor. Doing so may cause an infinite loop when a static or global object of that class is destroyed. The exit function invokes the destructors of all static or global objects. If one of these destructors calls exit, the destructors may be called again. This may cause a nonfatal infinite loop or may exhibit a fatal error when the same destructor is called again (e.g., if heap memory is deallocated twice).
It isn't difficult for the implementor to code exit so that it does not invoke destructors the second time it is called, but this is only a partial solution. How should the exit function react when called the second time? Should it invoke the destructor of the objects it hasn't yet destroyed, or abandon destruction of all objects? The result of calling exit twice is undefined. For example, the g++ compiler under UNIX when executed on the following program:
#include <iostream.h>
#include <stdlib.h>
class Obj {
private:
char *name;
public:
Obj(char *s) { name = s;
cout << "constructor: " << name << "\n"; }
~Obj() { cout << "destructor: " << name << "\n";
cout.flush();
exit(0); // DANGEROUS
}
};
Obj a("xyz"); // global objects
Obj a2("abc");
int main()
{
exit(0);
}
produced the following output:
constructor: xyz
constructor: abc
destructor: abc
It appears that g++ abandons all destructors on reentry to exit. This explains why only one of the objects has its destructor called.
Overloading the new and delete operators
The operators new and delete can be overloaded on a class-by-class basis. This allows the programmer to take over the memory allocation for a class, which is occasionally desirable. A simple example of this form of overloading is shown below, where the operators merely call the global new and delete operators to allocate the right number of bytes:
#include <stdlib.h>
#include <stddef.h> // declare size_t
#include <iostream>
class Obj {
int x;
public:
void *operator new(size_t n);
void operator delete(void *p);
};
void *Obj::operator new(size_t n)
{
return ::new char [n];
}
void Obj::operator delete(void *p)
{
::delete [] p;
}
This illustrates some of the pedantic syntactical restrictions, which although not causing a run-time error, make it difficult to get the code to compile. These include:
- operator new must have return type void*.
- operator delete must have return type void.
- The parameter to operator new must have type size_t.
- The parameter to operator delete must have type void*.
- There are inconsistencies between new and delete.
- The operators are called for derived class objects.
- The operators are not called for arrays of class objects (need to also overload "operator new[]" and "operator delete[]").
- The operators are allocators, not constructors and destructors.
The next few sections examine the variety of errors that can arise when overloading new and delete.
Inconsistencies between new and delete
When overloading new and delete, it is important to ensure symmetry between both operators. Naturally, there are any number of ways that programmers can make these two operators do the wrong things, but some of the more common problems are:
- not defining an overloaded delete
- using ::new char[] but not ::delete[]
The first error is a blatant failure to observe an important rule: always overload delete when you overload new. For example, if the delete operator is missing, this is not particularly dangerous, except that the pointer is allocated using ::new[] and deallocated with the global delete operator, probably without the delete[] syntax. Typically the failures will be worse, since the usual purpose of overloading new is to replace the usual memory allocation with some special form, and this will not usually be compatible with the usual deallocator.
A second problem with overloading new and delete is that care must be taken to ensure consistency between the calls to global new and delete (i.e., ::new and ::delete). It would be a dangerous error if the overloaded delete operator did not call ::delete[].
Allocation problems with derived classes
If a class is derived from the Obj class it will still call these overloaded new and delete operators (which are inherited from Obj) for memory allocation. Therefore, the size_t parameter to operator new can have different values that must be considered. For example, if the new operator had been defined using the statement:
return ::new Obj;
This would cause a failure when a derived class object was allocated. The use of the size_t parameter is better than assuming a particular object size. However, consider the problem of derived classes if the purpose of overloading the new and delete operators is to perform some memory management for one particular size of object. For example, an efficiency improvement may be possible by packing small objects into large memory blocks. In this case the allocation will work only if the correct size is received, and will fail for a derived object. Therefore, the code should check whether the size is correct, and call the global new operator if not (i.e., for a derived object):
void *Obj::operator new(size_t n)
{
if (n != sizeof(Obj)) { // handle derived objects
return ::new char[n]; // call global new operator
}
... // advanced allocation for the Obj class
}
A similar test of the size is required in operator delete, although to do so requires the use of a second argument for delete, which is the size of the object being deallocated. The delete operator is defined as follows:
void Obj::operator delete(void *p, size_t n)
{
if (n != sizeof(Obj)) { // handle derived objects
::delete [] p; // call global delete operator
return;
}
... // advanced deallocation for the Obj class
}
Unfortunately, the overloading of new and delete is an area where compiler differences are common and some compilers may not allow the two-argument form of operator delete. In this case the only method of avoiding dangers with derived objects is the redefinition of the new and delete operators in the derived class so that they simply call the global new and delete operators. Always remember that overloaded new and delete operators are inherited!
Overloading array allocation
The second major pitfall when overloading these operators is that they are not called for allocation or deallocation of arrays of objects. Statements such as those below will not invoke the basic new or delete overloaded operators, but will instead call the global operators:
Obj *p = new Obj[10]; // allocate array
delete [] p; // deallocate array
The solution is that you also need to overload the two "array" versions of new and delete that are named "new[]" and "delete[]" to distinguish them from the basic versions. The overloading syntax is similar and straight-forward:
void *operator new[](size_t n);
void operator delete[](void *p);
new is an allocator, not a constructor
An error that can arise when overloading the new and delete operators is one of understanding the purpose of new and delete. new and delete are an allocator and deallocator — not a constructor and destructor. When a dynamic object is allocated using new, the overloaded new is called, and then the constructor is called with the address returned by new (i.e., returned by the allocator). The basic idea is this: new allocates some "raw memory" and the constructor turns that memory into an object. Similarly, when an object is deleted, the reverse procedure occurs: first the destructor turns the object into raw memory, then the overloaded delete operator is called to deallocate that raw memory. The process can be understood using the following class with output statements to show what is happening:
#include <stdlib.h>
#include <stddef.h> // declare size_t
#include <iostream.h>
class Obj {
int x;
public:
Obj() { cout << "Constructor called\n"; }
~Obj() { cout << "Destructor called\n"; }
void *operator new(size_t n);
void operator delete(void *p);
};
void *Obj::operator new(size_t n)
{
cout << "Operator new called\n";
return :: new char [n];
}
void Obj::operator delete(void *p)
{
cout << "Operator delete called\n";
::delete p;
}
int main(void)
{
Obj *a = new Obj;
delete a;
exit(0);
}
The output of this program is:
Operator new called Constructor called Destructor called Operator delete called
This shows that the overloaded new is called before the constructor and the overloaded delete is called after the destructor. Therefore, the overloaded new and delete operators do not apply to an object, only to raw memory.
Some of the possible erroneous actions in new and delete include setting or using a non-static data member, calling a non-static member function, and explicitly calling the destructor. Any attempt to use (non-static) data members in delete is erroneous, since they will have been "destroyed" by the destructor. Any attempt to set (non-static) data members in new is erroneous, although perhaps not always dangerous, because the data members will be reset when the constructor is called on the given object. Naturally, any calls to (non-static) member functions are dangerous since there is nevera"real" object, only raw memory with undefined contents.
Postfix operators in Smart Pointer Classes
There is a potential error when only the prefix version of the overloaded operators ++ or -- are declared. The same overloaded operator is called for both prefix and postfix usages. This creates problems because users of a class with these operators defined may expect different behavior. For example, in the declaration of a "smart" pointer class, the user of a class may assume the following C++ idiom to work correctly:
SmartPtr p1, p2;
while(*p1 != 0)
*p1++ = *p2++; // postfix ++ works?
However, the postfix ++ operator may be actually calling the overloaded prefix ++ operator. The return value of the prefix version is (probably) the new value of the object after increment (to be consistent with prefix usage), rather than the old value before increment (which is the expected behavior because this is the behavior of the built-in postfix ++ operator on primitive types).
The solution is to always declare a postfix version of each operator with appropriate behavior (or at least one that reports an error so as to warn the programmer if it is accidentally called; it can be declared as private so as to cause a compilation error when used).
Nonvirtual destructor
When a class is used as a base class in an inheritance hierarchy there are some errors that can occur if the destructor is not virtual. Ifaclass declaration contains evenasingle virtual function, the destructor should probably also be declared as virtual. Doing so will not impose any extra space overhead since the existence of evenasingle virtual function already imposes this overhead on the class and its objects. Even if a class contains no virtual functions it may be advisable to declare the destructor as virtual if the class is to be used as a base class for other classes (note that this comment applies to other member functions too).
The main problem with having a non-virtual destructor involves the problems when delete is applied to a base class pointer. If the base class pointer actually holds the address of a derived class object the base class destructor will still be invoked unless the destructor is virtual. Any useful work in the derived class destructor is lost: deallocating memory, closing files, etc. The following code illustrates the problem:
#include <iostream.h>
class Base {
int x; // ... member data fields
public:
~Base() { cout << "Base destructor\n"; }
};
class Derived : public Base {
public:
~Derived() { cout << "Derived destructor\n"; }
};
int main()
{
Derived *d = new Derived;
Base *b = d; // conversion to base pointer
delete b; // Which destructor is called?
}
The base class destructor is called because the destructor is not virtual and the pointer variable b is assumed to be pointing at a "real" base class object. Declaring the base class destructor as virtual will cause the true type of the object pointed to by b to be inspected, and the correct destructor called (i.e., the derived class destructor is called, which then also calls the base class destructor automatically).
Because of the problems associated with having a non-virtual destructor, it is often advisable when declaring a library to declare the destructor (and other member functions) as virtual because users of the library are likely to derive further classes from the class in the library. However, the decision should not be taken lightly since adding a virtual function to a class with no other virtual functions imposes a space overhead on every object and may cause reduced run-time efficiency in the use of references or pointers to these objects.
Mistyping double-colon
Another error related to goto labels occurs when the programmer accidentally misses one of the colon characters in the :: token. Consider the following example:
void Derived::fn()
{
Base:fn(); // Typo!
}
The intention is to forward a derived method to the base class version of the same method, but what actually occurs is infinite recursion! The error is a missing second colon character from the "::" token. The sequence "Base:" becomes an unused goto label, and thus the call "fn();" is a recursive call. There should be a compiler warning about an unused goto label.
Templates and primitive types
Template classes and template functions are a means whereby the programmer can write one generic block of source code that should work for many different data types. For example, the programmer can design a Stack class so that it can hold any type of data, and the user of the class can later specify which data type is to be stored. Templates are therefore a very powerful tool, but there are some dangers because of the fact that they are, in a certain sense, just "clever" macros.
The most common problem is that operations on the primitive data types (i.e., integers, characters, floating-point values, and pointers) are not fully "orthogonal." The same operation does not necessarily have the same meaning for all types. For example, the > operation is valid and has the same meaning for integers, characters, and floating-point values but has erroneous meaning for strings declared as char* — it compares the pointer values rather than lexically comparing the two strings. Therefore, the use of this operator in a template may have problems if the template is later instantiated with type char*. Consider the template for a generic max function that returns the maximum of its two arguments, as below:
template <class Type> Type max(Type x, Type y)
{
if (x > y)
return x;
else
return y;
}
This performs correctly for basic types, but when used to test the lexical ordering of two strings it can produce incorrect results. The following code illustrates its usage:
cout << max(1,2) << "\n"; // Integers ok
cout << max('b','a') << "\n"; // Characters ok
cout << max("abcd","defgh") << "\n"; // DANGEROUS!
Because of these problems with templates, the programmer is allowed to explicitly define particular instances of the generic function, so as to handle special cases. A special version of the max function can be declared for type char* as follows:
char* max(char* x, char* y) // specialized char* version
{
if (strcmp(x,y) > 0)
return x;
else
return y;
}
Function overrides inherited virtual method
A common error in the use of virtual functions for derived classes is to accidentally change the signature of the member function — that is, to declare the parameter types in a derived class member function differently from the base class member function. Although the compiler will detect and complain about a virtual member function givenadifferent return type (and the same parameter types), it will allowaprogram using functions with changed parameter types to compile and run. The following code illustrates the strange behavior that can arise:
#include <iostream.h>
class Base {
public:
virtual void print(int x)
{ cout << "Base::print entered\n"; }
};
class Derived : public Base {
public:
void print(long x) // WRONG! Parameter changed
{ cout << "Derived::print entered\n"; }
};
int main()
{
Base * b;
Derived * d = new Derived;
b = d;
b->print(0); // Calls Base::print
}
If Derived::print were declared with an int parameter type, we would expect it to be called rather than Base::print. Howev er, the problem is that the Derived class has no print(int) function and therefore the call using the Base pointer uses the base class function rather than the derived function.
Note that whether or not the Derived::print function declaration is qualified with virtual has no effect on the problem. If the virtual keyword is used this will only affect the member function print(long) in classes derived from the Derived class; it has no effect on any print(int) member functions in any of the classes.
Name of default constructor misspelled
The fact that the C++ compiler creates a default constructor for any class without one can lead to problems. I came across this error when changing an implementation of a "stack" class to use the uppercase letter "S". A substitution of "Stack" for "stack" was performed but one important substitution was accidentally missed, leading to the following class declaration:
class Stack {
private:
int arr[SIZE]; // array holding stack
int sp; // stack pointer
public:
stack() { sp = 0; } // CONSTRUCTOR (or is it?)
....
};
The compiler treats Stack::stack as a simple member function, not a constructor, and instead also creates a default constructor for the class Stack. The stack pointer, sp, remains an uninitialized data member. Fortunately, this error was discovered by a warning that the non-void function Stack::stack does not return a value.
throw inside a destructor
It is bad practice to use the throw statement to raise an exception from within a destructor. The problem is that destructors are also invoked during the stack unwinding caused by handling a thrown exception. Consider the following code:
class Error {}; // exception class
class X {
public:
~X() { throw Error(); } // DANGEROUS!
};
void fn(void)
{
X obj;
if (something)
throw Error();
}
When the function throws the exception, the destructor for obj will be called. If that destructor throws another exception, it seems to be an infinitely looping process because throwing that exception will again lead the destructor to be called. However, Bjarne Stroustrup, the designer of C++, has taken care to avoid this form of infinite looping in the use of exceptions. The requirements for exception handling state that when an exception is thrown from within a destructor that has been called as a result of stack unwinding from an earlier exception, this results in a call to the terminate function (instead of again unwinding the stack leading to an infinite loop). Therefore, a throw statement inside a destructor will often cause the program to terminate rather than returning to earlier exception handling code.