Optimization bug with recursive functions

Optimization bug with recursive functions by DJones69

Closed

as Fixed

Type:	Bug
ID:	732925
Opened:	3/23/2012 11:57:09 AM
Access Restriction:	Public
Moderator Decision:	Sent to Engineering Team for consideration

Workaround(s)

view

User(s) can reproduce this bug

Greetings,

We ran across an optimization bug in our program when using x64 and the following optimization flags: /O2 /Ob1 /Oi /Ot. Our function is recursive and manipulates a global object. It seems similar to bug ID 678200. However, my example function requires somewhat irrelevant operations, otherwise the bug will disappear. For example, if the std::vector is removed, then correct assembly is generated. This is similar behavior to an earlier bug that I submitted where register allocation was not working properly.

In my example, I analyzed the assembly and noticed that it produces this when it compares the variables at the bottom of the DoSomething function:

    if( InstanceState.SavedData != InstanceState.Instance->Data )
000000013F26112C mov         eax,dword ptr [Instance+8 (13F263660h)]
000000013F261132 cmp         eax,eax
000000013F261134 je         DoSomething+0E2h (13F261142h)

The cmp is clearly wrong as the data has changed in the global variable. ResetData should be called in the outermost DoSomething call.

Some final notes:

This code also doesn't work in VS 2008 and 2011 Beta.
Win32 will produce correct assembly.
/Ob0 will produce correct assembly (presumably because the std::vector functions aren't inlined now).

Details (expand)

Visual Studio 2010

#include <vector>
#include <stdlib.h>
#include <stdio.h>
#include <stdint.h>

class TestClass
{
public:
    TestClass()
        : CurrentPointer(0x0),
         Data(0)
    {

    }

    char* CurrentPointer;
    int Data;

    void SetData(int MinSize);
    void ResetData();
};

class TestClassState
{
public:

    TestClassState()
        : Instance(NULL),
         CurrentPointer(NULL),
         SavedData(0)
    {

    }

    TestClass* Instance;
    char* CurrentPointer;
    int SavedData;
};

TestClass Instance;

void DoSomething()
{
    TestClassState InstanceState;
    InstanceState.Instance = &Instance;
    InstanceState.CurrentPointer = Instance.CurrentPointer;
    InstanceState.SavedData = Instance.Data;

    int bytes = 16;
    char* Result = (char *)((uintptr_t)Instance.CurrentPointer & ~7);
    Instance.CurrentPointer = Result + bytes;

    if( Instance.CurrentPointer > 0x0 )
    {
        Instance.SetData( bytes + 8 );
    }

    std::vector<int*> Foo;
    int* blah = 0x0;
    Foo.push_back(blah);

    static int depth = 1;
    if (depth == 1)
    {
        --depth;
        DoSomething();
    }

    for (unsigned int i = 0; i < Foo.size(); ++i)
    {
        int* a = Foo[i];
        free(a);
    }

    if( InstanceState.SavedData != InstanceState.Instance->Data )
    {
        InstanceState.Instance->ResetData();
    }

    InstanceState.Instance->CurrentPointer = InstanceState.CurrentPointer;
}

void TestClass::SetData( int MinSize )
{
    Data = MinSize;
}

void TestClass::ResetData( )
{
    Data = 0;
}

int main()
{
    DoSomething();

    printf("Data: %d\n", Instance.Data);
}

English

Windows 7

English

Data: 24

Data: 0

0 attachments

Posted by Microsoft on 4/24/2012 at 4:54 PM

DJones69,
First of all, thanks for reporting this bug.

The fix will be included in the VS 2011 release. Unfortunately, it does not meet the bar to be backported to 2010.

The bug could potentially affect any code gen target since it occurs in target-independent code.

Andrew Spencer, Visual C++

Posted by DJones69 on 4/3/2012 at 11:10 AM

I noticed that this has been marked as fixed. When will we see the fix applied? Will it only affect 2011, or will there be a hotfix for 2010, too? Does this error affect Win32, too?

Posted by MS-Moderator10 [Feedback Moderator] on 3/26/2012 at 1:07 AM

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 DJones69 on 3/23/2012 at 1:02 PM

One other note: changing SetData and ResetData to be inlined alleviates the problem. It's as if the compiler fails to recognize that those functions are mutating data.

Posted by MS-Moderator01 on 3/23/2012 at 12:51 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)