Find bugs for (almost) free

Want to find bugs in your code for (almost) free? Try static code analysis — a useful technique, though often maligned by developers for noise and “false positives”. If you need an appeal to authority to be convinced, see how John Carmack (of Doom and Quake fame) swears by it!

If you use C++ and a modern version of Visual Studio, you already have a pretty powerful code analysis engine available to you, the /analyze compiler switch. It started life many years back as “PREfast,” a technology developed and used internally at Microsoft. Over the past several years, it has been steadily improved and nicely integrated right into the Visual Studio IDE (Analyze -> Run Code Analysis On Solution). Overall it is quite good at finding very common — and severe — bugs plaguing native code developers.

Null pointer dereferences? Check.

#include <cstdlib>
#include <new>

struct Data
{
    int x;
};

Data * MallocData()
{
    // !!! Raises Warning C6011: Dereferencing NULL pointer 'd'.
    Data* d = (Data*)malloc(sizeof(Data));
    d->x = 1234;
    return d;
}

Data * NewData()
{
    // !!! Raises Warning C6011: Dereferencing NULL pointer. 'd' contains the same NULL value as 'new(1*4, nothrow)' did.
    Data* d = new (std::nothrow) Data();
    d->x = 1234;
    return d;
}

Buffer overruns? Check.

wchar_t * RunOver()
{
    // !!! Raises Warning C6200: Index '10' is out of valid index range '0' to '9' for non-stack buffer 'buffer'.
    wchar_t * buffer = new wchar_t[10];
    buffer[10] = L'\0';
    return buffer;
}

Other common data type misuses? Check.

#include <Windows.h>
#include <iostream>

using namespace std;

void Init()
{
    // !!! Raises Warning C6217: Implicit cast between semantically different integer types : testing HRESULT with 'not'.
    //     Consider using SUCCEEDED or FAILED macro instead.
    if (!CoInitialize(nullptr))
    {
        wcerr << L"Failed!" << endl;
        // ...
    }
    // ...
}

It can even find race conditions if you use concurrency annotations:

#include <Windows.h>

class MaybeThreadSafeCounter
{
public:
    MaybeThreadSafeCounter()
        : cs_(), count_(0)
    {
        InitializeCriticalSection(&cs_);
    }

    ~MaybeThreadSafeCounter()
    {
        DeleteCriticalSection(&cs_);
    }

    int Increment()
    {
        EnterCriticalSection(&cs_);
        int value = ++count_;
        LeaveCriticalSection(&cs_);
        return value;
    }

    int Decrement()
    {
        // !!! Raises Warning C26130: Missing annotation _Requires_lock_held_(this->cs_) or _No_competing_thread_ at function
        //     'MaybeThreadSafeCounter::Decrement'.Otherwise it could be a race condition.
        return --count_;
    }

private:
    CRITICAL_SECTION cs_;
    _Guarded_by_(cs_) int count_;
};

Okay, so static analysis is not completely free. You have to run the tool and sift through the issues. In some cases you need to annotate your intent, or even clean up “harmless” coding patterns that confuse the analyzer; as Carmack says, “Anything that isn’t crystal clear to a static analysis tool probably isn’t clear to your fellow programmers, either.” For that matter, enabling and acting on compiler warnings isn’t free either but it would be hard to find a modern day programmer who would argue against it.

Give /analyze a try and prepare to be shocked by what it finds lurking in your codebase.