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

Superhighway

Is VS2005 even supported now? It's 8 years old. – Spoilsport

Tried it on my VS2005, indeed the output is ERROR, 10<10 – Florescence

Have you considered downloading the express versions of Visual Studio? the compiler included is the same for each release year – Embank

Express versions don't include openMP – Superhighway

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   : }

Ster

So the problem is reproducible. I think this has far-reaching consequences; the behaviour and output of many C++ win32 console applications compiled with Visual C++ 2005 (using the above setting, which are standard) is undefined!!! I don't trust this compiler anymore. – Superhighway

Pretty sure you aren't the first one who found this bug, but I can't find it on any bug tracker... but I haven't been looking too hard. – Ster

Have you filed a bug report for this on Microsoft Connect? – Avant

@Avant I don't have a Connect account (usually dev'ing for Linux, but was interested in this question so installed VS2005 Express). If you have one, could you file this and link the report here? Would be much appreciated. – Ster

@us2012: Ok, I've tried submitting it, but for some strange reason the Connect website currently acts up for me (surprise, lol). I'll try again next week. Though I think you're right; it's hard to believe no one stumbled across this before. The fact that changing setupVal2 "resolves" the error makes me think that maybe it has to do with the initialization of setupVal1 & 2 on one line. Could you try declaring them in separate declarations? – Avant

@Avant This occurred to me as well, however, putting them in separate lines does not solve this issue. – Ster

filed bug report to Microsoft here – Superhighway