Perplexing STL bug in Release in Visual Studio.

Question

I have what I'm pretty sure is a bug in the optimizer in Visual studio 2005. The problem is with an STL map.

Here's the relevant code:

MyMapIterator myIt = m_myMap.find(otherID);

if (myIt != m_myMap.end() && myIt->second.userStatus == STATUS_A)
{
 //Prints stuff.  No side-effects whatsoever.
}
else if (myIt != m_myMap.end() && myIt->second.userStatus == STATUS_B && myIt->second.foo == FOO_A)
{
 //Prints stuff.  No side-effects whatsoever.
}
else if (myIt != m_myMap.end() && myIt->second.userStatus == STATUS_B && myIt->second.foo == FOO_B && /*other meaningless conditions */)
{
 //Prints stuff.  No side-effects whatsoever.
}

Works flawlessly in debug, used to crash in release, and disabling global optimizations "fixed it" Now, nothing works. I get a:

Microsoft Visual Studio C Runtime Library has detected a fatal error in [...]

Press Break to debug the program or Continue to terminate the program.

This happens on the first MyMapIterator::operator-> of the last else if

The map is empty, I know that find should have returned end(), The first two comparisons to that effect work. But somehow, the third time 'myIt != m_myMap.end()' returns true, and the right side of the && is executed.

Various other places fail like this with a variant of 'myIt != m_myMap.end()' returning true in that same file, but this is, to me, the one that rules out most other possibilities. I used to think it was a buffer overflow that was stomping on my map, but take a look back at the code. I am positive that no other thread is stomping on it, and this is 100% reproductible.

So, what do I do from here. This is not performance sensitive in the slightest. I just need it to work as it should. Any option is acceptable. Yes I know I could surround the whole thing with the iterator equality check and it isn't the nicest code. The point is, it should still work, and if that fails, anything else can.

EDIT

The last else-if doesn't generate any jump !

    if (myIt != m_myMap.end() && myIt->second.userStatus == STATUS_A)
009270BE  mov         ecx,dword ptr [this] 
009270C4  add         ecx,0Ch 
009270C7  lea         eax,[ebp-90h] 
009270CD  call        std::_Tree<std::_Tmap_traits<unsigned __int64,lux::Foo,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,lux::Foo> >,0> >::end (8A21E0h) 
009270D2  mov         esi,eax 
009270D4  lea         edi,[myIt] 
009270D7  call        std::_Tree<std::_Tmap_traits<unsigned __int64,lux::Foo,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,lux::Foo> >,0> >::const_iterator::operator!= (8A2220h) 
009270DC  movzx       ecx,al 
009270DF  test        ecx,ecx 
009270E1  je          lux::Bar::DoStuff+0E4h (927154h) 
009270E3  lea         esi,[myIt] 
009270E6  call        std::_Tree<std::_Tmap_traits<unsigned __int64,lux::Foo,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,lux::Foo> >,0> >::iterator::operator-> (8A21F0h) 
009270EB  cmp         dword ptr [eax+8],1 
009270EF  jne         lux::Bar::DoStuff+0E4h (927154h) 
    {
         Stuff
    }
    else if (myIt != m_myMap.end() && myIt->second.userStatus == STATUS_B)
00927154  mov         ecx,dword ptr [this] 
0092715A  add         ecx,0Ch 
0092715D  lea         eax,[ebp-98h] 
00927163  call        std::_Tree<std::_Tmap_traits<unsigned __int64,lux::Foo,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,lux::Foo> >,0> >::end (8A21E0h) 
00927168  mov         esi,eax 
0092716A  lea         edi,[myIt] 
0092716D  call        std::_Tree<std::_Tmap_traits<unsigned __int64,lux::Foo,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,lux::Foo> >,0> >::const_iterator::operator!= (8A2220h) 
00927172  movzx       edx,al 
00927175  test        edx,edx 
00927177  je          lux::Bar::DoStuff+17Ah (9271EAh) 
00927179  lea         esi,[myIt] 
0092717C  call        std::_Tree<std::_Tmap_traits<unsigned __int64,lux::Foo,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,lux::Foo> >,0> >::iterator::operator-> (8A21F0h) 
00927181  cmp         dword ptr [eax+8],2 
00927185  jne         lux::Bar::DoStuff+17Ah (9271EAh) 
    {
            //Stuff
     }
    else if (myIt != m_myMap.end() && myIt->second.userStatus == STATUS_C)
009271EA  mov         ecx,dword ptr [this] 
009271F0  add         ecx,0Ch 
009271F3  lea         eax,[ebp-0A0h] 
009271F9  call        std::_Tree<std::_Tmap_traits<unsigned __int64,lux::Foo,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,lux::Foo> >,0> >::end (8A21E0h) 
009271FE  mov         esi,eax 
00927200  lea         edi,[myIt] 
00927203  call        std::_Tree<std::_Tmap_traits<unsigned __int64,lux::Foo,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,lux::Foo> >,0> >::const_iterator::operator!= (8A2220h) 
00927208  lea         esi,[myIt] 
0092720B  call        std::_Tree<std::_Tmap_traits<unsigned __int64,lux::Foo,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,lux::Foo> >,0> >::iterator::operator-> (8A21F0h) 
    {
            //Stuff in the condition and after

Answer 1

It seems likely that you have some other condition in code you've not shown that is causing the map to be populated with some invalid data.

Try dumping out the contents of the map before your call to find(), and have a look for any possible uninitialized values in the rest of the code.

Answer 2

obviously anything can have bug, even the VC compiler. so if there is a bug in this section of code, you could wrap it in a #pragma to turn optimisations off. IF it still crashes after that, then something else in your code is trashing your map (and then you need to turn on DrWatson to get dumps so you can inspect the crash in windbg to try and find out what's happened to it).

To turn off optimisation use:

#pragma optimize("g", off)

to turn it back on, either change off to on or use the special case

#pragma optimize("", on)

which resets your optimisation settings to the project defaults.

Answer 3

The optimizer really messes up your debugging info. There's a very good chance that the line that's actually failing on a different, nearby line. Even if the debugger is telling you that the exception is occurring on the second half of the && operator, it probably isn't.

To diagnose the real problem, you may wish the break up the last if statement into something like:

else if (myIt != m_myMap.end())
{
    printf("TEST 1\n");
    if (myIt->second.userStatus == STATUS_B && myIt->second->foo == FOO_B && /*other meaningless conditions */)
    {
        //Prints stuff.  No side-effects whatsoever.
    }
}

printf("TEST 2\n");

If you get an output of "TEST 1", then there's something wrong with your iterator. If you get an output of "TEST 2" and then a crash, then obviously the error is occurring in the subsequent code. If neither "TEST 1" nor "TEST 2" prints and it still crashes there, then something is really wrong.

Answer 4

Yes I know I could surround the whole thing with the iterator equality check and it isn't the nicest code. The point is, it should still work, and if that fails, anything else can.

Of course, with that logic you have no options... Anything you do is a workaround.

I think my first approach would be to wrap the stuff in a block that's verified that myIt is valid:

if (myIt != m_myMap.end())
{
    if (myIt->second.userStatus == STATUS_A) 
    {
        //Prints stuff.  No side-effects whatsoever.
    }
    else if (myIt->second.userStatus == STATUS_B && myIt->second->foo == FOO_A)
    {        
        //Prints stuff.  No side-effects whatsoever.
    }
    else if (myIt->second.userStatus == STATUS_B && myIt->second->foo == FOO_B && /*other meaningless conditions */)
    {
        //Prints stuff.  No side-effects whatsoever.
    }
}

Answer 5

I suspect you have code elsewhere that has a buffer over run in it.

This other code is managing to wreck the memory that the posted code is using.

In debug mode the extra memory padding will change the behavior.

Changing other random code will also change the behaviour as it will shift the relative position of things.

Basically you'll need to review a lot of code or use a tool like BoundsChecker to find the error.

Concentrate on arrays or any raw pointer maths. Also any use of deleted pointers.

Bugs like this can hide for a long time if they write to place that doesn't cause a crash. For the same reason they often mysteriously disappear.

Answer 6

I think you're making an assumption that's probably false: that the crash happens after having successfully checked myIt != m_myMap.end() && myIt->second.userStatus in the same iteration. I'd bet that the crash immediately follows the first time these values are examined. It's the optimizer that makes it look like you've cleared the first two if conditions.

Try putting a completely different code block into the loop and see if it crashes there (my bet is that it would -- I don't think you'll see the output from cerr immediately preceding a crash.)

// New code block
MyMapIterator yourIt = m_myMap.find(otherID);
if (yourIt != m_myMap.end() && yourIt->second.userStatus == STATUS_A || 
     yourIt->second.userStatus == STATUS_B)
{
        cerr << "Hey, I can print something! " << std::ios::hex << this << endl;
}

// Original code
MyMapIterator myIt = m_myMap.find(otherID);
if (myIt != m_myMap.end() && myIt->second.userStatus == STATUS_A)
{
        //Prints stuff.  No side-effects whatsoever.
}
else if (myIt != m_myMap.end() && myIt->second.userStatus == STATUS_B && myIt->second->foo == FOO_A)
{
        //Prints stuff.  No side-effects whatsoever.
}
else if (myIt != m_myMap.end() && myIt->second.userStatus == STATUS_B && myIt->second->foo == FOO_B && /*other meaningless conditions */)
{
        //Prints stuff.  No side-effects whatsoever.
}

Answer 7

if (myIt != m_myMap.end() && myIt->second.userStatus == STATUS_B && myIt->second->foo == FOO_A)

Why are you accessing userStatus with . and foo with -> ?

Answer 8

Well guys. After much pain and tears. To fix it at least temporarily I had to rework the code a bit to the form suggested by most people, which is how it should have been in the first case:

if (myIt != m_myMap.end())
{
    if (myIt->second.userStatus == STATUS_A) 
    {
        //Prints stuff.  No side-effects whatsoever.
    }
    else if (myIt->second.userStatus == STATUS_B && myIt->second->foo == FOO_A)
    {        
        //Prints stuff.  No side-effects whatsoever.
    }
    else if (myIt->second.userStatus == STATUS_B && myIt->second->foo == FOO_B && /*other meaningless conditions */)
    {
        //Prints stuff.  No side-effects whatsoever.
    }
}

However, the bug was still present. This gem fixed it:

if (myIt != m_myMap.end())
if (myIt != m_myMap.end())
{
    if (myIt->second.userStatus == STATUS_A) 
    {
        //Prints stuff.  No side-effects whatsoever.
    }
    else if (myIt->second.userStatus == STATUS_B && myIt->second->foo == FOO_A)
    {        
        //Prints stuff.  No side-effects whatsoever.
    }
    else if (myIt->second.userStatus == STATUS_B && myIt->second->foo == FOO_B && /*other meaningless conditions */)
    {
        //Prints stuff.  No side-effects whatsoever.
    }
}

Yes. Doubling the if resulted in a jump instruction being emitted after the test.