Another Bug in Your Bug (Finder): __offsetof

Last month I posted about a crashing bug in Visual Studio 2012’s static code analysis feature.  The irony was delicious. Yesterday I found another bug in the same feature. This one isn’t quite as dramatic, but I found it amusing, and the techniques needed to fully explore it are instructive.

This new bug is in the 64-bit version of /analyze that comes with VS 2012 Professional, which somehow manages to have the wrong type for __offsetof(), leading to /analyze-only compile errors.

The main bug is here

The repro is quite simple:

template <typename T>
T MyMax(const T& a, const T& b)
{
    return a > b ? a : b;
}

int main(int argc, char* argv[])
{
    MyMax(sizeof(void*), __alignof(int));
    return 0;
}

On all 32-bit builds, and on 64-bit builds without /analyze this code compiles fine, but on 64-bit /analyze builds it fails with this error:

error C2782: ‘T MyMax(const T &,const T &)’ : template parameter ‘T’ is ambiguous

Normally sizeof() and __alignof() both return size_t which allows MyMax to work, but in 64-bit /analyze builds one of them is returning the wrong type. That’s the first problem.

Getting this code to compile is easy enough – just cast the two parameters to size_t and you’re done. But doing that without fully understanding the problem makes me feel unclean, so I dug in deeper.

The secondary bug is here

The second problem is that the error message is unhelpful. It could tell us what the two ambiguous types are, but it doesn’t, and that makes the bug harder to understand. So we need to engage in some trickery. The obvious thing to do is to use typeid() to print the types, with code like this:

printf("%s\n", typeid(sizeof(void*)).name());
printf("%s\n", typeid(__alignof(int)).name());

However this doesn’t help. Both statements print unsigned __int64. That’s because the code-gen phase of the compiler is working fine (which is why the code compiles without /analyze), but the analysis phase is broken (note that knowing that there are two phases can be useful for diagnosing other esoteric /analyze problems).

Now the problem is to figure out a way to convince VC++ to print the types as seen by the analysis phase.

It turns out that when VC++ displays a warning about code inside of a template function it tells you what the types are. We can leverage this to find what we need. We just need to call MyMax with matched parameters (once with sizeof() and once with __alignof()) with an intentional bug added to MyMax. I chose an out-of-range write to an array. Here’s the test code:

template <typename T>
T MyMax(const T& a, const T& b)
{
    char buffer[100];
    buffer[100] = 0; // Oops!
    return a > b ? a : b;
}
 
int main(int argc, char* argv[])
{
    MyMax(__alignof(int), __alignof(int));
    MyMax(sizeof(void*), sizeof(void*));
    return 0;
}

offsetof.cpp(10): warning C6201: Index ‘100’ is out of valid index range
    OffsetOf.cpp(19) : see template ‘T MyMax<unsigned int>
    with [ T=unsigned int ]
offsetof.cpp(10): warning C6386: Buffer overrun while writing to ‘buffer’

offsetof.cpp(10): warning C6201: Index ‘100’ is out of valid index range
    OffsetOf.cpp(20) : see template ‘T MyMax<unsigned __int64>
    with [ T=unsigned __int64 ]
offsetof.cpp(10): warning C6386: Buffer overrun while writing to ‘buffer’

From this we can tell that the bug is in __alignof() which returns type unsigned int. This seems to indicate some something that was forgotten during the port of /analyze from 32-bit because unsigned int is the correct type on 32-bit builds.

Now I can hack around the bug using a cast to size_t and keep a clear conscience. How blissful.

Previous bug update

Meanwhile the previous bug (use-after-free crash in the annotation parser) is still unfixed. On the five build machines which I have running /analyze I get 10-20 compiler crashes a day (averaging about one per build). I’ve adjusted my scripts to ignore them so I don’t have to babysit the unreliability too often. Microsoft is aware of the problem, but I have heard no ETA on a fix.

Case Aside

Working on Linux has me thinking about case-sensitivity more than usual so I was amused by the inconsistent casing of the source-file name in the warning messages. The file is actually “OffsetOf.cpp” but the compiler lower-cases it in some places. I’ve never noticed that before.

About these ads

About brucedawson

I'm a programmer, working for Google, focusing on optimization and reliability. Nothing's more fun than making code run 10x faster. Unless it's eliminating large numbers of bugs. I also unicycle. And play (ice) hockey. And juggle.
This entry was posted in Code analysis, Code Reliability, Programming, Visual Studio and tagged , , , , , . Bookmark the permalink.

2 Responses to Another Bug in Your Bug (Finder): __offsetof

  1. Hello,
    I suggest to try PVS-Studio. This analyzer recently started working with Vs2012. I wonder if you can find errors in it. Download: http://www.viva64.com/en/pvs-studio-download/

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s