missing destructor calls when optimization is enabled by maxim1000

It is fixed in VS2010 SP1 Beta according to this:
http://blogs.msdn.com/b/jasonz/archive/2010/12/20/visual-studio-2010-service-pack-1-beta-feedback.aspx

It's been 9 months since the last comment, is there a QFE for this? Is it included in VS 2010 SP1?

We have a fix for the issue and are in the process of evaluating it. If the fix is found to be acceptable we will be preparing a QFE for both Visual Studio 2008 and Visual Studio 2010.

Thanks,

Visual C++ Codegen and Tools Team

My name is Mark Levine and I am a Dev Lead in the Visual C++ Codegen and Tools team. I was in charge of the team in 2008 that did the original investigation of this bug and I made the call to defer addressing it. As previously noted, this was a mistake and the recent discussions and feedback about this decision on this and other channels have been illuminating, to say the least. I will certainly not forget the lessons learned here and will apply them in the future.

Meanwhile, we are actively working on this issue and evaluating solutions. Once we have a suitable fix, we will prepare and make a QFE available for customers using Visual Studio 2008 as quickly as possible (and will also prepare one for Visual Studio 2010, to be available shortly after the VS2010 release). We will post updates here in the coming weeks.

Mark Levine
Dev Lead – Visual C++ Codegen & Tools

I can reproduce this bug in VC8 SP1, with default compiler settings in release build.
Microsoft (R) C/C++ Optimizing Compiler Version 14.00.50727.762
Microsoft (R) Incremental Linker Version 8.00.50727.762

Will there be another service pack for this compiler, too?

As a result of a second example of hitting this bug that was reported on another channel, we took another look at this yesterday. The new example is different in that rather than an infinite loop with a return in the middle as in the 2008 example, yesterday’s example has a loop that exits and then calls a no-return function. This means that the routine still passes our test of whether we can attempt RVO or not. (To do RVO for functions that have exception handling we require that there be only one return. Because the no-return function is called the routine still has only one return even though there are two exits from the loop). This example shows us that the bug shows up in a broader range of cases than we originally thought.

Our original assessment in 2008 was that the style of coding required to hit the bug would be quite rare. Based on that assessment and the apparent suitability of source workarounds, we chose to give higher priority to other compiler issues than this one. Clearly, our assessment was wrong and we erred in not addressing this bug sooner.

Based on yesterday's second look at this issue, we have reevaluated this bug and we now plan to fix it in the next service pack.

Jerry Goodwin
Visual C++ Codegen & Tools

The guarantee that destructors match constructors is fundamental to C++. Choosing not to fix such an error is deliberate negligence. It is said that only a poor craftsman blames his tools, but I don't see how anyone could choose to continue using tools from this company in the face of this. It's simply astonishing.

I am going to be pushing my company to not use the visual studios compiler suite and drop all MSDN licenses because of this incredibly broken compiler optimization. C++'s fundamental underpinning is the automatic calling of destructors of stack objects, member sub-objects, etc. This is the very first thing added to C to make C++, and it is the most important defining quality of C++, aka RAII. Because the compiler bungles such a basic case, and because you prioritize it so lowly makes me question whether or not visual studios is the appropriate place to spend our money for a quality product.

Put another way, if we are unable to ship optimized builds without fear of memory leaks, then we're left with:
1- Don't use RAII, or basically don't program using standard C++.
2- Don't ship optimized builds.
3- Ship broken builds.
4- Don't use visual studios. Aka use gcc or some alternative.

The only reason my company currently uses visual studios, and buys the MSDN licenses, is because it is a superior optimizing compiler. However, we will not use an optimizing compiler if it affects correctness. In our line of work, basically any coding besides games, correctness matters most. Thus, if our choices are the above, 1 is not practical due to time to market costs, 3 is not acceptable, and 4 is preferable to 2, because although visual studios might optimize better when it doesn't break cases, I bet gcc will optimize better vs visual studios with all the optimizations turned off, thus obviating the only reason to use visual studios over gcc.

I suggest you seriously rethink your business strategy. No one will put up with a nonstandard C++ compiler which misses the entire point of C++.

Since we're not going to be able to get this fixed in the service pack, and other users are hitting this bug, I wanted to write in more detail about what is going wrong and more specific ways to work around it, since the workarounds will have to be in place for some time.

There is an optimization we do that has a problem in a very specific circumstance. The optimization that has the bug is otherwise a good optimization that works in all but a special case. The normal case where this optimization is a good thing is:

O foo() {
O obj;
// any code containing no return statements
return obj;
}

In the unoptimized case, a copy constructor is called that copies obj into the (unnamed) return object, then obj is destructed. The unnamed return object gets destructed by the caller. What the optimization does is eliminate the copy constructor call and the destructor call for obj and return obj rather than a copy of obj. Obj then gets destructed by the caller. This optimization saves a copy constructor call and a destructor call. You can tell whether you're getting this optimization by putting print statements in your constructor, destructor and copy constructor.

As the previous post indicates, you can certainly work around the problem by turning off all optmizations for the function, but avoiding the specific circumstance where the optimization has a bug is a method that may appeal to some customers. For us to perform the optimization, the function must be returning a user-defined type (a class object). When exception handling (EH) is in use, an additional requirement for the optimization is that there be only one return statement in the function. Exception handling is "in use" if the function was compiled with /EHa or /EHs, regardless of whether there are any exception handling constructs (e.g. try/catch) actually used in the function. The specific circumstances to trigger the bug are that we're doing the optimization, EH is in use, and there is are two code paths leading from the point of a constructor call where one path leads to the return statement and the other to a separate point where the destructor for that object would be called. An example is easier to understand:

O foo() {     // O is a user-defined class type
    // point 4
    for (int i = 0; i < 10; i++) {
        O obj(); // point 1
        if (i == 9)
            return obj; //point 2
    } // point 3
}

Assume that the above is compiled with /EHsc. It has just one return statement, and the bug is triggered because there is a code path leading from the constructor call at point 1 to the return statement at point 2 and another code path leading from point 1 to point 3. There is an implicit destructor call at point 3 where obj goes out of scope and is destructed before the loop goes back to the top. It's this implicit destructor that we mess up due to this bug. So in the example above, we will fail to call the destructor for the object for all iterations of the loop except the last one where we return.

The simplest workaround in the above case would be to move the declaration of obj outside the loop to point 4. This changes the semantics of the program; the object is constructed once instead of multiple times. This workaround won't do if you need to pass the value of i to the constructor. Another way to work around the problem is to force a second return statement into the code some way or other. Here's one way:

O foo() {    
    for (int i = 0; i < 10; i++) {
        O obj(i);     // need to use i in constructor call
        if (i == 9)
            return obj;
        else if (i > 9)
            return obj; // will never get executed
    }
}

The presence of the second never-executed return statement causes the specific optimization to be disabled but allows other optimizations to the function. Another way to force an extra return statement would be to wrap the body of the function in an always-true if statement and put a never-executed return in the else of the always-true if.

One thing that does not work as a workaround is to have an explicit throw statement as a second way out of the routine. This doesn't count as a second return statement.

I've taken the time to write this up because we value our customers and regret that our code sometimes has bugs that can't be immediately fixed.

Jerry Goodwin

Unfortunately, the fix for this issue is very involved and will not be in the next Service Pack; we hope to have it fixed in the next major product release.

We are still working on solutions for this issue, and expect a fix to be in our next Service Pack release. In the meantime, you can work around this issue by disabling global optimization in the offending function (in this case, aaa()). For example:

#pragma optimize("g", off)
A aaa(){
    // aaa's code goes here
}
#pragma optimize("", on)

will turn off global optimizations for aaa, and use your original optimizations for the rest of the code.

Thank you,
Stephen

Thanks for your feedback.

We are escalating 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.

Thank you,
Visual Studio Product Team