Home Dashboard Directory Help
Search

Miscompile with RVO in x64 compiler by Erik Olofsson


Status: 

Closed
 as Fixed Help for as Fixed


1
0
Sign in
to vote
Type: Bug
ID: 735523
Opened: 4/4/2012 4:24:25 PM
Access Restriction: Public
Moderator Decision: Sent to Engineering Team for consideration
0
Workaround(s)
view
0
User(s) can reproduce this bug

Description

Code is miscompiled generating "and dword ptr" instead of "and qword ptr".

If you std::move the result so RVO cannot be done the code is correctly generated.
Details
Sign in to post a comment.
Posted by Microsoft on 6/6/2012 at 10:50 AM
Unfortuantely, the fix didn't make it in time for the RC release. I do see that it is included in early builds of the final product (RTM).

At this time there are no planned public releases of Visual Studio 2012 until the final release.

thanks,
ian Bearman
Principal Software Developer
VC++ Code Generation and Optimization
Posted by Erik Olofsson on 6/5/2012 at 8:59 AM
I'm still getting this in Visual Studio 2012 RC. Is the fix supposed to be in there? If not is it possible to get access to a fixed compiler version so I can verify the fix before RTM?
Posted by Microsoft on 5/15/2012 at 11:16 AM
Again, thank you for reporting this. I thought I'd give you an update. We've checked in a fix for failing to emit the REX prefix. Here's a snippet of disasm from the fs_LaunchExecutable that shows, at the end of the snippet, the corrected emission of the REX prefix.

?fs_LaunchExecutable@CTest@@SA?AU1@AEBV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z (public: static struct CTest __cdecl CTest::fs_LaunchExecutable(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const &)):
0000000000000000: 48 89 4C 24 08     mov         qword ptr [rsp+8],rcx
0000000000000005: 53                 push        rbx
0000000000000006: 48 83 EC 30        sub         rsp,30h
000000000000000A: 48 C7 44 24 28 FE mov         qword ptr [rsp+28h],0FFFFFFFFFFFFFFFEh
                    FF FF FF
0000000000000013: 48 8B D9         mov         rbx,rcx
0000000000000016: 45 33 C0         xor         r8d,r8d
0000000000000019: 44 89 44 24 20     mov         dword ptr [rsp+20h],r8d

< 000000000000001E: 83 21 01         and         dword ptr [rcx],1
> 000000000000001E: 48 83 21 01        and         qword ptr [rcx],1
Posted by Microsoft on 4/11/2012 at 10:17 AM
Okay, thanks for staying on this. Based on your feedback i took the additional step of disassembling the binary and lo and behold it looks like there's an encoding problem in the compiler. The listing file generated by the compiler (via /Fa) has this:

and    QWORD PTR [rcx], 1

But the disasembly of the final binary (done via link.exe /dump /disasm) contains this:

and    dword ptr [rcx],1

The compiler thinks it's emitting a QWORD load/store, but it is in fact emitting a DWORD load/store.

I or someone from my team will post back once we know more.

ian Bearman
Principal Developer
VC++ Code Generation and Optimization

Posted by Erik Olofsson on 4/11/2012 at 1:36 AM
I'm overwriting the stack just to demonstrate the problem. Without that the stack will be 0 for the member variable in this example, so it would not show any difference in outcome depending on DWORD or QWORD and.

Are you compiling with the Visual Studio 11 beta compiler?

This is the code I get generated:

    __declspec(noinline) static CTest fs_LaunchExecutable
        (
            std::string const &_Executable
        )
    {
000007F746FD1240 mov         qword ptr [rsp+8],rcx
000007F746FD1245 push        rbx
000007F746FD1246 sub         rsp,30h
000007F746FD124A mov         qword ptr [rsp+28h],0FFFFFFFFFFFFFFFEh
000007F746FD1253 mov         rbx,rcx
000007F746FD1256 xor         r8d,r8d
000007F746FD1259 mov         dword ptr [rsp+20h],r8d
        CTest Ret; // m_Environment is initialized with "and         dword ptr [rcx],1" instead of the correct "and         qword ptr [rcx],1"
000007F746FD125E and         dword ptr [rcx],1
000007F746FD1261 add         rcx,8
000007F746FD1265 mov         qword ptr [rcx+18h],0Fh
000007F746FD126D mov         qword ptr [rcx+10h],r8
000007F746FD1271 mov         byte ptr [rcx],r8b
000007F746FD1274 mov         dword ptr [rsp+20h],1
        Ret.m_Target = _Executable;
000007F746FD127C cmp         rcx,rdx
000007F746FD127F je         CTest::fs_LaunchExecutable+4Ah (07F746FD128Ah)
000007F746FD1281 or         r9,0FFFFFFFFFFFFFFFFh
000007F746FD1285 call        std::basic_string<char,std::char_traits<char>,std::allocator<char> >::assign (07F746FD1450h)
        return Ret;
000007F746FD128A mov         rax,rbx
    }

As you can see it's clearly generating:
000007F746FD125E and         dword ptr [rcx],1

Also I don't see how it could be inlined into main as I have explicitly disabled inline of the fs_LaunchExecutable function.

This is the full command line Visual Studio is generating for the compile:
cl /c /Zi /nologo- /W3 /WX- /O2 /Oi /GL /D WIN32 /D NDEBUG /D _CONSOLE /D _UNICODE /D UNICODE /Gm- /EHsc /MD /GS /Gy /fp:precise /Zc:wchar_t /Zc:forScope /Yu"stdafx.h" /Fp"x64\Release\TestCodegen.pch" /Fo"x64\Release\\" /Fd"x64\Release\vc110.pdb" /Gd /TP /errorReport:prompt TestCodegen.cpp

And the compiler version (x86 cross compiler):
Microsoft (R) C/C++ Optimizing Compiler Version 17.00.50214.1 for x64
Posted by Microsoft on 4/10/2012 at 3:50 PM
Thanks for taking the time to file this report. Unfortunately, I’m not able to reproduce the issue you are seeing regarding DWORD vs. QWORD. I compiled the code you provide using the x64 compiler with -O2 optimization level.

Here is what I see for the code in CTestLink::f_Assign:

        static void f_Assign(CLinkPointer &_Dest, CTestLink *_pSrc)
    {
        size_t Src = ((size_t)_Dest.f_Get()) & size_t(1);
        size_t Combined = Src | (size_t)_pSrc;
        _Dest.f_Set((CTestLink *)Combined);
    }

and    QWORD PTR [rcx], 1
    or    QWORD PTR [rcx], rdx
    ret    0


Here is what I see for the code in CTestTree::CTestTree where CTestLink::f_Assign* is inlined:

    CTestTree::CTestTree()
    {
        CTestLink::f_Assign(m_TestLink, nullptr);
    }

    and    QWORD PTR [rcx], 1
    mov    rax, rcx
    ret    0

This is then inlined into CTest::CTest:

and    QWORD PTR [rcx], 1
    mov    QWORD PTR [rcx+32], 15
    mov    QWORD PTR [rcx+24], 0
    mov    BYTE PTR [rcx+8], 0
    mov    rax, rcx
    ret    0

Which is then inlined into CTest::fs_LaunchExecutable

    static CTest fs_LaunchExecutable
        (
            std::string const &_Executable
        )
    {
        CTest Ret; // m_Environment is initialized with "and         dword ptr [rcx],1" instead of the correct "and         qword ptr [rcx],1"
        Ret.m_Target = _Executable;
        return Ret;
    }

    …
    and    QWORD PTR [rcx], 1
    mov    rbx, rcx
    add    rcx, 8
    mov    QWORD PTR [rcx+24], 15
    mov    QWORD PTR [rcx+16], 0
    mov    BYTE PTR [rcx], 0
    …


And finally this code inlined into main:
    …
and    QWORD PTR Test$[rsp], 1
    …



*Note that m_TestLink is an uninitialized member variable at this point and its value is undefined. I think you may be attempting to write to it using ::fg_CorreuptMemory, but that is strongly dependent on stack layout which is different between debug and optimized builds.


Please let me know if there’s something else I need to do to generate the failure (different compiler optimizations, etc)

thanks,
ian Bearman
Principal Developer
VC++ Code Generation and Optimization
Posted by MS-Moderator10 [Feedback Moderator] on 4/4/2012 at 11:49 PM
Thanks for your feedback.

We are rerouting this issue to the appropriate group within the Visual Studio Product Team for triage and resolution. These specialized experts will follow-up with your issue.
Posted by MS-Moderator01 on 4/4/2012 at 4:49 PM
Thank you for your feedback, we are currently reviewing the issue you have submitted. If this issue is urgent, please contact support directly(http://support.microsoft.com)
Sign in to post a workaround.
File Name Submitted By Submitted On File Size  
TestCodegen.zip (restricted) 4/4/2012 -
TestCodegen.zip 4/4/2012 4 KB