Excellent STL bug in release in Visual Studio
I have what I'm pretty sure is a bug in the optimizer in Visual studio 2005. The problem is in the 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 debugging, used to crash in release and disable global optimizations "fixed". Now nothing works. I get:
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 in the first MyMapIterator :: operator-> of the last one if
The map is empty, I know find should have returned end (). The first two comparisons work with this effect. But somehow, the third time "myIt! = M_myMap.end ()" returns true, and the right side of && is executed.
Various other places fail just like the myIt! = M_myMap.end () variant returning true in the same file, but for me this is the one that precludes most of the other possibilities. I used to think it was a buffer overflow that is stomping on my map, but take a look at the code. I'm sure no other thread stomps on it and this is 100% reproducible.
So what should I do next. It is not performance sensitive. I just need it to work as it should. Either option is acceptable. Yes, I know I could surround the whole thing with an iterator equality check, and this is not the nicest code. The point is that it still has to work, and if that fails, everything 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
a source to share
Well guys. After intense pain and tears. To fix this, at least temporarily, I had to rework the code a bit in the form most people suggested, as 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 error was still present. This stone has been fixed:
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 if, which issues a jump command after the test.
a source to share
I suspect you have code elsewhere that has mileage in it.
This other code is capable of destroying the memory that the allocated code is using.
In debug mode, the additional memory addition will change the behavior.
Changing other random code will also change the behavior as it will shift the relative state of things.
Basically you will need to go through a lot of code or use a tool like BoundsChecker to find the error.
Focus on arrays or any raw pointer math. Also any use of remote pointers.
Errors like this can hide for a long time if they write in place, which does not crash. For the same reason, they often disappear mysteriously.
a source to share
It seems likely that you have a different condition in the code that you didn't specify, which causes the map to be filled with some invalid data.
Try dumping the contents of the map prior to your call before find()
and see all possible uninitialized values in the rest of the code.
a source to share
obviously everything can have a bug, even the VC compiler. so if there is a bug in this section of code, you can wrap it in # pragma to disable optimization. If it still crashes after that, then something else in your code is destroying your map (and then you need to enable DrWatson to get dumps so you can check the crash in windbg to try and figure out what happened to it).
To disable optimization use:
#pragma optimize("g", off)
to enable it, either change to enable or use special case
#pragma optimize("", on)
which resets the optimization settings to the project defaults.
a source to share
The optimizer has really messed up your debug information. There's a very good chance there is a line that is actually failing on another nearby line. Even if the debugger tells you that the exception occurs in the second half of the statement &&
, it probably isn't.
To diagnose a real problem, you may wish to break 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 the output "TEST 1", then something is wrong with your iterator. If you get the result "TEST 2" and then fail, then obviously the error occurs in the following code. If neither "TEST 1" nor "TEST 2" prints and it crashes anyway, then something is really wrong.
a source to share
I think I ran into this same problem.
Only it seems that in the x64 version (full optimization). My STL map is confirmed to be empty (size () == 0), but this line of code doesn't detect an empty map and instead goes into a for loop:
typedef std::map<std::string,std::string> KeyValueMap;
printf( "size: %d\n", ExtraHeaders.size() ); // prints "size: 0"
for( KeyValueMap::iterator it= ExtraHeaders.begin(); it != ExtraHeaders.end(); it++ )
{
// this line is executed
printf( "%s\t%s\n", (*it).first.c_str(), (*it).second.c_str() );
}
Looks like a compiler error.
a source to share
Yes, I know that I could work around it all by checking the iterator equality and this is not the nicest code. The point is that it still has to work, and if that fails, everything else can.
Of course, with this logic, you have no parameters ... Everything you do is a workaround.
I think my first approach would be to wrap stuff in a block that validated it myIt
:
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.
}
}
a source to share
I think you are making an assumption, which is probably false: that the crash occurs after being successfully checked myIt != m_myMap.end() && myIt->second.userStatus
in the same iteration. I would argue that a crash follows immediately the first time these values are considered. This is an optimizer that makes it look like you cleared the first two conditions if
.
Try putting a completely different block of code in the loop and see if it fails (my bet is what it would be - I don't think you will see the output from cerr
immediately preceding the failure.)
// 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.
}
a source to share