c++visual-c++compiler-bug

Сompiler bug? Variable assumed unchanged


Visual Studio version: 17.7.1 (MSVC 19.37.32822)
Freshly created project with default settings and compiler flags.

Minimal reproducible example:

#include <cstdio>

__declspec(noinline) void test2(char** data)
{
    // After moving the pointer:
    // data_1 now points to data[1] = 1
    // data_0 now points to data[0] = 2
    *data += 1;
}

__declspec(noinline) void test(char* data_1)
{
    char* data_0 = data_1;
    test2(&data_1);
    int len = (int)(data_1 - data_0);

    if (*data_1 & 1)
    {
        if (*data_0 & 2)
            printf("good\n");
     }
}

int main()
{
    char data[2];
    data[0] = 2;
    data[1] = 1;
    test(data);
    return 0;
}

No line "good" is printed in Release| x64 configuration.
Debug or Release | x86 produce expected result.

I did some experimenting and looked through resulting assembly in order to reduce the original code to this MRE.
The root cause of the issue seem to be the compiler's assumption that data1 remains unchanged in test2(). Thus, the line data_0 = data_1 can be omitted and data_1 can be used instead of data_0 in the expression (*data_0 & 2).
Each line is necessary, including the casting of (data_1 - data_0) to int, which probably explains the lack of bug reproduction in the x86 build.

Update 1

Problem only occurs in the x64 Release build. Disabling optimizations by changing /O2 to /Od or using #pragma optimize("", off) "fixes" the problem.

Reproducibility by VS version breakdown:

It's possible that the problem has already been fixed or the conditions for its reproduction have changed.

Update 2

One of the MSVC developers confirmed this problem in a private conversation, I've made this issue to track progress: https://developercommunity.visualstudio.com/t/Compiler-optimization-bug-in-VS-2022/10512534


Solution

  • I can confirm that the same behaviour is present here. In addition any attempt to printf values to check the else (fail) conditions results in correct behaviour. Optimiser has been too aggressive. It only fetches *data_1 and applies both tests to it (assuming that data_1==data_0 which of course isn't true).

    Any alteration to print out debugging values seems to result in correct behaviour. I added printf("bad data_0") instead to the inner else clause and predictably that got printed out. I prefer to have all paths annotated.

    FWIW Intel compiler 2023 runs it fine and prints "good".

    Smoking gun - MSC compiler bug excessively aggressive optimisation fails to load *data_0 and applies both tests to *data_1. Disassembly is here (with slight additions which don't affect errant behaviour) :

    --- C:\Users\Martin\source\repos\Toy_bug1\Toy_bug1.cpp ------------------------

    // After moving the pointer:
    // data_1 now points to data[1] = 1
    // data_0 now points to data[0] = 2
    *data += 1;
    00007FF6D10B1070  inc         qword ptr [rcx]  
    }
    00007FF6D10B1073  ret  
    [snip]
    __declspec(noinline) void test(char* data_1)
    {
    00007FF6D10B1080  mov         qword ptr [rsp+8],rcx  
    00007FF6D10B1085  sub         rsp,28h  
    char* data_0 = data_1;
    test2(&data_1);
    00007FF6D10B1089  lea         rcx,[data_1]  
    00007FF6D10B108E  call        test2 (07FF6D10B1070h)  
    int len = (int)(data_1 - data_0);
    
    if (*data_1 & 1)
    00007FF6D10B1093  mov         rax,qword ptr [data_1]  
    00007FF6D10B1098  movzx       ecx,byte ptr [rax]  
    00007FF6D10B109B  test        cl,1  
    00007FF6D10B109E  je          test+3Eh (07FF6D10B10BEh)  
    {
        if (*data_0 & 2)
    00007FF6D10B10A0  test        cl,2  // this test is incorrect!
            printf("good\n");
        else
            printf("bad d0");
    }
    00007FF6D10B10A3  lea         rax,[string "bad d0" (07FF6D10B2258h)]  
    00007FF6D10B10AA  lea         rcx,[string "good\n" (07FF6D10B2250h)]  
    00007FF6D10B10B1  cmove       rcx,rax  
    }
    

    I remain a little mystified about the significance of the crucial line in the MRE which is optimised out of existence but is essential for the fault to show! Namely:

    int len = (int)(data_1 - data_0);

    Without this line present the MSC compiler gets it right in x64 release. Here is the disassembly of the correct code generated in that case:

    //    int len = (int)(data_1 - data_0);
    
    if (*data_1 & 1)
    00007FF6D6D11096  mov         rax,qword ptr [data_1]  
    00007FF6D6D1109B  test        byte ptr [rax],1  
    00007FF6D6D1109E  je          test+3Eh (07FF6D6D110BEh)  
    {
         if (*data_0 & 2)
    00007FF6D6D110A0  test        byte ptr [rdx],2  
    // rax  is data_1
    // rdx  is data_0
    

    Moral of story is beware of doing things that confuse optimising compilers!