Dangerous error Visual c++ 2005

Question

I bumped into a very serious error using visual studio 2005, running a C++ Win32 Console application. The problem will show when running the code below (simplified), using the following project properties: C++|optimization|optimization|/O2 (or /O1, or /Ox), C++|optimization|Whole program optimization|/GL, linker|optimization|/ltcg

    #include "stdafx.h"
    #include <iostream>
    using namespace std;
    const int MAXVAL=10;
    class MyClass
    {
    private:
      int p;
      bool isGood;
    public:
      int SetUp(int val);
    };
    int MyClass::SetUp(int val)
    {
      isGood = true;
      if (MAXVAL<val)
      {
        int wait;
        cerr<<"ERROR, "<<MAXVAL<<"<"<<val<<endl;
        cin>>wait;
        //exit(1);      //for x64 uncomment, for win32 leave commented
      }
      if (isGood) p=4;
      return 1;
    }
    int _tmain(int argc, _TCHAR* argv[])
    {
      int wait=0, setupVal1=10, setupVal2=12;
      MyClass classInstance1;
      MyClass classInstance2;
      if (MAXVAL>=setupVal1) classInstance1.SetUp(setupVal1);
      if (MAXVAL>setupVal2) classInstance2.SetUp(setupVal2);
      cerr<<"exit, enter value to terminate\n";
      cin>>wait;
      return 0;
    }

The output shows that value 10 is smaller then value 10! I already found out that changing setting /O2 to /Od solves the problem (setting /Og, which is part of /O2, causes the problem), but that really slows down the execution time. Also changing the code a bit can solve it but hey, I can never be sure that the code is reliable. I am using Visual studio 2005 professional (Version 8.0.50727.867), os windows 7. My questions are: can someone try to reproduce this error using Visual Studio 2005, (I already tried VS 2010, no problem), and if so, what happens here? Can I assume that newer versions have solved this problem (I consider buying VS 2012) Thank you

Answer 1

You can reduce your example significantly and still get the same problem! You don't need two instances, and you don't need any of the other local or member variables. Also, you can hardcode MAXVAL.

Quick summary of what "solves" the problem:

• making MAXVAL a nonconst int
• setting setupVal2 to a value less than 10
• surprisingly, changing the condition 10<val to val>10 !!!

Here's my minimal version to reproduce the problem:

#include "stdafx.h"
#include <iostream>
using namespace std;

class MyClass
{
public:
    int SetUp(int val);
};

int MyClass::SetUp(int val)
{
    if (10<val)
        cout<<10<<"<"<<val<<endl;
    return 1;
}

int _tmain(int argc, _TCHAR* argv[])
{
    int setupVal1=10, setupVal2=12;
    MyClass classInstance;

    classInstance.SetUp(setupVal1);
    classInstance.SetUp(setupVal2);

    cin.get();
    return 0;
}

The problem, as witnessed by the disassembly, is that the compiler thinks 10<val is always true and therefore omits the check.

_TEXT   SEGMENT
?SetUp@MyClass@@QAEHH@Z PROC                ; MyClass::SetUp
; _val$ = ecx

; 16   :    if (10<val)
; 17   :        cout<<10<<"<"<<val<<endl;

    mov eax, DWORD PTR __imp_?endl@std@@YAAAV?$basic_ostream@DU?$char_traits@D@std@@@1@AAV21@@Z
    push    eax
    push    ecx
    mov ecx, DWORD PTR __imp_?cout@std@@3V?$basic_ostream@DU?$char_traits@D@std@@@1@A
    push    10                  ; 0000000aH
    call    DWORD PTR __imp_??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@H@Z
    push    eax
    call    ??$?6U?$char_traits@D@std@@@std@@YAAAV?$basic_ostream@DU?$char_traits@D@std@@@0@AAV10@PBD@Z ; std::operator<<<std::char_traits<char> >
    add esp, 4
    mov ecx, eax
    call    DWORD PTR __imp_??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@H@Z
    mov ecx, eax
    call    DWORD PTR __imp_??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@P6AAAV01@AAV01@@Z@Z

; 18   :    return 1;

    mov eax, 1

; 19   : }