/analyze for Visual Studio—the Ugly (part 3), False Positives

/analyze is a powerful Visual C++ feature that can help to find bugs. However it sometimes identifies problems when none exist. Unfortunately these false positives, like those reported here, make /analyze less useful because the serious bugs end up drowned by the cries of ‘wolf’. In this post I describe another frustrating false positive that shows one example of the compiler utterly failing to understand aliasing.

Update: the /analyze in Visual Studio 2012 doesn’t warn on these constructs anymore. But unfortunately they have gone from false positives to false negatives. In the GetInt() example if you comment out some of the initialization then there is no warning, even though a partially uninitialized object is being returned. Better? I’m not sure.

These tests are all done with Visual Studio 2010 SP1, C/C++ Optimizing Compiler Version 16.00.40219.01 for 80×86.

Uninitialized data

We have some code that reads from a file or network stream and initializes various variables. When we need to initialize an integer we call this function:

int GetInt(const char* p)
{
    int result;
    char* pDest = (char*)&result;
    pDest[0] = p[0];
    pDest[1] = p[1];
    pDest[2] = p[2];
    pDest[3] = p[3];
    return result; // warning C6001: Using uninitialized memory ‘result’
}

The compiler is unable or unwilling to tell that we have written to all four bytes of result so it gives a warning when we return it.

One could argue that the compiler is technically correct. The rules around aliasing in the C++ language are a bit esoteric so while I believe that this is legal, I am not certain. Using a union is a way to inform the compiler that aliasing is occurring so I tried that:

union intAlias
{
    int i;
    char c[4];
};

int GetIntFixAttempt(const char* p)
{
    intAlias result;
    char* pDest = result.c;
    pDest[0] = p[0];
    pDest[1] = p[1];
    pDest[2] = p[2];
    pDest[3] = p[3];
    return result.i; // warning C6001: Using uninitialized memory ‘result’
}

No dice. However I was on the right track. I next tried rewriting the code like this, to make it easier for the compiler to follow the trail:

union intAlias
{
    int i;
    char c[4];
};

int GetIntFixed(const char* p)
{
    intAlias result;
    result.c[0] = p[0];
    result.c[1] = p[1];
    result.c[2] = p[2];
    result.c[3] = p[3];
    return result.i; // No warning!
}

Success at last! It’s nice to know that I can find code that will let /analyze realize that I’m not using uninitialized memory, but I’d rather not have to go through such gymnastics. All of the information about how the code will execute is available so there is no theoretical reason why the compiler can’t reason correctly about it, but it fails.

If the compiler wanted to give me a warning about dodgy aliasing then I could respect that, but its current warning is just wrong, and this makes /analyze more difficult to use.