Goanna Static Analysis by Red Lizard Software

As part of our efforts to expand the scope of Goanna’s C++ checks, we decided to look into copy control, since this backbone of class architecture can also cause plenty of problems. The most common bugs relating to copy control are memory leaks, which are hard to identify and track down, as they will generally not cause the program to crash. Therefore, they are a priority for us to find.

In addition to finding and warning about the most common flaws in copy control functions, we decided to take the opportunity to cover some of the rarer problems, too. Our ultimate aim was to give some kind of useful warning in any case of potential misuse of a class, since copy control is something that a lot of people can have trouble grasping.

While many of our copy control checks warn for convention violations rather than definite bugs, they all combine to ensure that classes follow widely-accepted best practices, to improve the overall readability and robustness of code. As a case study, I’ll demonstrate the construction of a simple class, showing the bugs and suggestions that Goanna points out along the way.

We’ll start with a class with an int pointer. Of course, if we want the pointer to point somewhere, we need to allocate some memory for it, so we’ll do that in a constructor.

1   class MyClass {
2     public:
3       MyClass();  //constructor
4     private:
5       int* xp;
6   };
7
8   //default constructor
9   MyClass::MyClass(){
10     xp = new int[10];
11  }

We all know what the problem with this is:

cop.cc:9: warning: Goanna[COP-dtor] Missing destructor for class `MyClass' whose
function `MyClass::MyClass()' allocates memory

cop.cc:9: warning: Goanna[COP-member-uninit] Not all members initialized in this
constructor

If there is no explicit destructor, the compiler will only call the synthesized destructor, which is a problem for our allocated memory. The synthesized destructor will release the pointer xp, but not the memory allocated to it - this is our responsibility.

The second warning is there because even though we have allocated memory to ‘xp’, we have failed to initialize the values in the array, which should really be done in the constructor.

Because we’re lazy, let’s define an empty destructor to make the warnings go away (because that’s what half of programming is all about). Here’s the updated class:

1   class MyClass {
2     public:
3       MyClass();    //default constructor
4       ~MyClass(){}  //destructor
5     private:
6       int* xp;
7   };
8  
9   //default constructor
10  MyClass::MyClass(){
11    xp = new int[10];
12    for (int i=0; i!=10; ++i){
13      xp[i] = 0;
14    }
15  }

Alas, Goanna still isn’t happy:

cop.cc:4: warning: Goanna[COP-dealloc-dtor] Class field `xp' has memory allocated
in a constructor that is not freed in the destructor

Let’s release the memory in the destructor.

17  //destructor
18  MyClass::~MyClass(){
19    delete[] xp;
20  }

Now our class is free from bugs. So when we run Goanna, we should get the all clear. Right?

cop.cc:18: warning: Goanna[COP-assign-op] Missing assignment operator for class
`MyClass' which uses dynamic memory allocation

cop.cc:18: warning: Goanna[COP-copy-ctor] Missing copy constructor for class
`MyClass' which uses dynamic memory allocation

Wrong! It seems we’re just digging ourselves deeper. The destructor is not the only thing the compiler will synthesize. In fact, for any class, it will synthesize up to 3 functions if they are not explicitly defined:

1. Destructor - there will always be one synthesized to release stack memory
2. Copy constructor - a constructor taking in a reference of the class type
3. Assignment operator - operator=() defined for the class

Even though we have defined a default constructor (with no parameters), the compiler will still synthesize a copy constructor if we have not provided one. The copy constructor is used when instances of a class are copied, for example, passed as parameters or used in containers. Surprisingly, it is also invoked when an instance of the class is initialized at its declaration. For example:

MyClass a;       //default constructor
MyClass b(a);    //copy constructor
MyClass c = a;   //copy constructor
MyClass d; d=a;  //default constructor; assignment operator

It can sometimes be confusing which operators or functions are being invoked, and for such reasons it is important to ensure that all functions are explicitly defined when appropriate. Basically, if a user-defined destructor is required, then an assignment operator and copy constructor will also be required. This is sometimes called the ‘Rule of 3′, suggesting that if one of these three is required, all probably are.

So let’s add our copy constructor and assignment operator to the class, and to avoid any additional warnings, we’d better make sure we allocate the memory required for xp. And while we’re at it, we should get rid of the magic number used for the array size:

1   class MyClass {
2     public:
3       MyClass();                             //default constructor
4       MyClass(const MyClass& other);         //copy constructor
5       void operator=(const MyClass& other);  //assignment operator
6       ~MyClass();                            //destructor
7     private:
8       static const int ARR_INIT_SIZE = 10;
9       int* xp;
10  };
...
18  //copy constructor
19  MyClass::MyClass(const MyClass& other){
20    xp = new int[ARR_INIT_SIZE];
21    for (int i=0; i!=ARR_INIT_SIZE; ++i){
22      xp[i] = other.xp[i];
23    }
24  }
25
26  //assignment operator
27  void MyClass::operator=(const MyClass& other){
28    delete[] xp;  //reallocate the memory in case the size has changed
29    xp = new int[ARR_INIT_SIZE];
30    for (int i=0; i!=ARR_INIT_SIZE; ++i){
31      xp[i] = other.xp[i];
32    }
33  }

The class is starting to get bulky, but at least we can be sure we’re helping to make it safe for any creating, copying and destroying that we might be doing. However, Goanna isn’t quite happy with the class yet:

cop.cc:27: warning: Goanna[COP-assign-op-ret] Assignment operator `MyClass::operator='
does not return a non-const reference to `this'

cop.cc:29: warning: Goanna[COP-assign-op-self] Assignment operator `MyClass::operator='
does not check for self-assignment before allocating memory to a class member

The first warning is there because it is the convention that an assignment operator will return a reference to the target of the assignment. Such conventions exist so that all objects can be treated like primitive types, and to give the programmer more freedom to write intuitive code. For example:

MyClass a,b,c;
(a = b) = c;
(a = c).f();

The problem causing the second warning, is that self-assignment is generally perfectly legal code. For example:

MyClass c;
c = c;

However, calling a class’ assignment operator on itself will cause problems if dynamic memory allocation takes place. In our assignment operator, we free the memory allocated to ‘xp’, before allocating a fresh store. If the class instance called the assignment operator on itself, then the memory that being copied from in the 4th line of the operator will also be fresh. This means that self-assignment will basically lead to the object being populated with uninitialized data, which is almost certainly not the intention of the programmer. To handle this, we simply need to ensure memory manipulation only takes place if ‘this’ and the parameter refer to different instances of the class.

Our assignment operator must be altered to fix these two problems:

25  //assignment operator
26  MyClass& MyClass::operator=(const MyClass& other){
27    if (this != &other){  //check for self-assignment
28      delete[] xp;  //reallocate the memory in case the size has changed
29      xp = new int[ARR_INIT_SIZE];
30      for (int i=0; i!=ARR_INIT_SIZE; ++i){
31        xp[i] = other.xp[i];
32      }
33    }
34    return *this;  //return a reference to 'this'
35  }

After these changes, Goanna will not give any more warnings, and the class will be very robust. Hopefully this has provided you with some insight to some of our copy control checks, and given you a quick revision on some of the important things to keep in mind when creating classes. We’re still trying to expand our range of C++ checks further, to include checks on the proper use of iterators, containers and exception handling, and many other constructs.

Here is our completed class:

1   class MyClass {
2     public:
3       MyClass();                                 //constructor
4       MyClass(const MyClass& other);             //copy constructor
5       MyClass& operator=(const MyClass& other);  //assignment operator
6       ~MyClass();                                //destructor
7     private:
8       static const int ARR_INIT_SIZE = 10;
9       int* xp;
10  };
11 
12  //default constructor
13  MyClass::MyClass(){
14    xp = new int[ARR_INIT_SIZE];
15    for (int i=0; i!=ARR_INIT_SIZE; ++i){
16      xp[i] = 0;
17    }
18  }
19 
20  //copy constructor
21  MyClass::MyClass(const MyClass& other){
22    xp = new int[ARR_INIT_SIZE];
23    for (int i=0; i!=ARR_INIT_SIZE; ++i){
24      xp[i] = other.xp[i];
25    }
26  }
27 
28  //assignment operator
29  MyClass& MyClass::operator=(const MyClass& other){
30    if (this != &other){  //check for self-assignment
31      delete[] xp;  //reallocate the memory in case the size has changed
32      xp = new int[ARR_INIT_SIZE];
33      for (int i=0; i!=ARR_INIT_SIZE; ++i){
34        xp[i] = other.xp[i];
35      }
36    }
37    return *this;  //return a reference to 'this'
38  }
39 
40  //destructor
41  MyClass::~MyClass(){
42    delete[] xp;
43  }

I used to be a keen windsurfer and once attended a training camp/holiday in Greece. We covered advanced maneuvers such as 360’s, forward rolls and duck gybes etc. The instructor always said, “Doesn’t matter if you pull it off, just so long as it looks good!”

The reason for this story is that at Red Lizards we have a mantra of “Bringing higher quality software to market faster”, and so the flip side of this coin is that we can’t ship a release until it is of sufficiently high quality. Double edged sword perhaps, yet this is the business we chose.

(more…)

So what’s different about Goanna and why did we approach the static analysis challenge with model checking?

In case you weren’t aware; Goanna is the first static analysis tool to implement model checking after 4 years research at NICTA (National ICT Australia), which is Australia’s leading ICT government research organisation. 

The result  is that the key attributes for Goanna’s differences are as follows;
(more…)

Internally we are encouraging new team members to watch the so called Sean Tapes. There is nothing dubious about these tapes, unless you call Static Analysis and OCaml “dubious”. They are actually just screen captures, in which Sean explains how the Goanna tool performs static analysis for C/C++ code.

We are happy to share Sean’s prized voice-over with you all. Here’s the first video on the use of Goanna.

The other day we met with a new prospective alpha partner for our upcoming MSVS release (Visual Studio support is scheduled for July 2009). This customer has a lot of smarter people creating impressive code bases to challenging requirements.

Having presented how Goanna is different in that we fit within the IDE, as opposed to sitting with/behind the central build, I asked the question as to when and how they run peer reviews.

(more…)

A static analysis tool for Java is FindBugs. I found this interesting Google Video, in which Bill Pugh talks about static analysis. While he talks mostly about bugs in Java it is still quite interesting in general. His most interesting lesson is “Smart people make dumb mistakes”. I’d like to add, “Really smart people are glad to find their dumb mistakes, and are not afraid to get help finding them“. (more…)

We are frequently asked whether Goanna is a software model checker. After all, we analyse software and use a model checker to do so. However, while Goanna uses a model checker in the background for part of its analysis, it is not a software model checker. In the typical meaning of the word. (more…)

The gcc compiler supports several “attributes” that you can tag a function with. The semantics of those attributes is described informally and, indeed, gcc doesn’t check that they attributes are observed. In other words, the attributes are just decorative, and really have no semantics at all.

(more…)

Often I am asked: How much does a bug cost? I would love to say $1023.76. Each and every time. Consistently. Use this as your metrics for calculating risks and you will be fine.

Well, the truth is costs are somewhere between nothing and your business. Now, let me explain why. (more…)