VC6 optimizer bug with _com_raise_error (sample code)

[manfred]

/*
hi all,

the following minimal win32 app causes an access violation when
compiled with /O2 on VC6 (i think SP3). it runs fine with any
other optimizer setting. there is no 'critical' code anywhere,
so i wonder if someone can help me with this.

to build the app, generate an empty win32 application and add the
code as the only source file. compile and start the release version.

note that even subtle changes (reordering, removing non-executed
code) will make the access violation disappear.

the AV is raised after _com_raise_error and before the catch handler
is entered. the objects in func() are unwound before the AV.

[carl daniels]
carl, maybe this is what you also experienced. no other way out than
abandon _com_error??

thanks all,

-manfred

demo app code:
*/

#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#include <comdef.h> // for _com_raise_error
//#include "SEHframe.h"

struct structA
{
structA() { x = 0; y = 0; }
~structA() { }
double x, y;
};

struct structB
{
structB(char* p) : m_ptr(0) { }
~structB()
{
if(m_ptr != NULL)
++*m_ptr; // never executed
}
char* m_ptr;
};

void func(structB, structA, structA, int, int, int)
{
structA a, b, c;
_com_raise_error(E_FAIL); // causes access violation
with /O2
}

int APIENTRY WinMain(HINSTANCE, HINSTANCE, LPSTR, int)
{
structB p(0);

try {
// WalkSEHFrames("pre func");
func(0, structA(), structA(), 0, 0, 0);
}
catch(const _com_error& ce) {
ce;
::MessageBox(0, "made it into catch block", "/O2 bug",
MB_OK);
}
return 0;
}

[Carl Daniel [MVP]]

"manfred" <mzi...@snafu.de> wrote in message
news:c4i2quk8hp6c11sjb...@4ax.com...

> /*
> hi all,
>
> the following minimal win32 app causes an access violation when
> compiled with /O2 on VC6 (i think SP3). it runs fine with any
> other optimizer setting. there is no 'critical' code anywhere,
> so i wonder if someone can help me with this.

I compiled it (VC6/SP5) from the command prompt with /GX /O2 as the only
command-line arguments and indeed, it does produce an access violation.

Compiled with VC7 (VS.NET), it shows the message box as expected, so
apparently it's a bug that was fixed.

>
> to build the app, generate an empty win32 application and add the
> code as the only source file. compile and start the release version.
>
> note that even subtle changes (reordering, removing non-executed
> code) will make the access violation disappear.
>
> the AV is raised after _com_raise_error and before the catch handler
> is entered. the objects in func() are unwound before the AV.
>
> [carl daniels]
> carl, maybe this is what you also experienced. no other way out than
> abandon _com_error??

It might be - your code is very different than the code I was working on at
the time, but the symptoms appear to be the same.

-cd

[Doug Harrison [MVP]]

manfred wrote:

>the following minimal win32 app causes an access violation when
>compiled with /O2 on VC6 (i think SP3). it runs fine with any
>other optimizer setting. there is no 'critical' code anywhere,
>so i wonder if someone can help me with this.

I see the same thing with VC6, SP4. However, it works properly with
/O1, which many people consider the better project-wide optimization
option. With /O1, you reap the benefits of smaller code, without
subjecting yourself to the vagaries of /O2. I would recommend using
/O2 only on those isolated bits of code which have been proven to
benefit from it.

--
Doug Harrison
Microsoft MVP - Visual C++

[Larry Brasfield]

In article <tni3quc5443mv727e...@4ax.com>,
Doug Harrison [MVP] (d...@mvps.org) says...

I will second that recommendation. For what it's
worth, space optimization is what Microsoft uses
internally for its products based on the idea that
smaller code is generally faster that "fastest"
code due to cache effects. This fact also means
that the space optimizer is much better wrung out
than the speed optimizer because code generation
problems tend to be discovered and fixed early
in the compiler release cycle.

--
-Larry Brasfield
(address munged, s/sn/h/ to reply)

[Bobby Mattappally [MS]]

Your code will work as expected if you add the /EHa compiler option.
The default (/EHsc) does not handle asynchrnous exceptions.

The vc6.0 compiler does not handle "exception-specifications", so the
compiler doesn't know that the
_com_raise_error function can throw an exception and hence the optimizer is
free to optimize away the exception handling code
around the try block.

Vc7.0 compiler understands exception specifications better enough to
understand that _com_raise_error can throw exception and hence won't
optimize away the exception handling code.

Hope this helps.

Thank you,
Bobby Mattappally
Microsoft VC++/C# Team

This posting is provided "AS IS" with no warranties, and confers no rights.

--------------------
>From: manfred <mzi...@snafu.de>
>Newsgroups: microsoft.public.vc.language
>Subject: VC6 optimizer bug with _com_raise_error (sample code)

[Carl Daniel [MVP]]

"Bobby Mattappally [MS]" <bob...@online.microsoft.com> wrote in message
news:bqO#gCnbCHA.1676@cpmsftngxa08...

> Your code will work as expected if you add the /EHa compiler option.
> The default (/EHsc) does not handle asynchrnous exceptions.

That's interesting, but it's hard to see why it's relevant - there's no
asynchronous exception occurring here, is there? Or does _com_raise_error
actually raise an SEH exception which eventually becomes a _com_error?

-cd

[Bobby Mattappally [MS]]

That's interesting, but it's hard to see why it's relevant - there's no
>asynchronous exception occurring here, is there? Or does _com_raise_error
>actually raise an SEH exception which eventually becomes a _com_error?
>

The problem here is that the compiler was optimizing away the exception
handling code.
When you enable EHa, the compiler knows that an exception can happen
anywhere in any function and hence won't optimize away
the exception handling code and hence the result.
Hope this helps.

Thank you,
Bobby Mattappally
Microsoft VC++/C# Team

This posting is provided "AS IS" with no warranties, and confers no rights.

--------------------
>From: "Carl Daniel [MVP]" <cpda...@pacbell.net>
>References: <c4i2quk8hp6c11sjb...@4ax.com>
<bqO#gCnbCHA.1676@cpmsftngxa08>
>Subject: Re: VC6 optimizer bug with _com_raise_error (sample code)
>Date: Mon, 7 Oct 2002 19:53:43 -0700

[Carl Daniel [MVP]]

"Bobby Mattappally [MS]" <bob...@online.microsoft.com> wrote in message

news:wTt8qsnbCHA.2764@cpmsftngxa08...

> That's interesting, but it's hard to see why it's relevant - there's no
> >asynchronous exception occurring here, is there? Or does
_com_raise_error
> >actually raise an SEH exception which eventually becomes a _com_error?
> >
>
> The problem here is that the compiler was optimizing away the exception
> handling code.
> When you enable EHa, the compiler knows that an exception can happen
> anywhere in any function and hence won't optimize away
> the exception handling code and hence the result.
> Hope this helps.

Ok, using /EHa is a work-around - the code should've worked as-was.

-cd

[manfred]

On Tue, 08 Oct 2002 02:17:03 GMT, bob...@online.microsoft.com (Bobby
Mattappally [MS]) wrote:

>Your code will work as expected if you add the /EHa compiler option.
>The default (/EHsc) does not handle asynchrnous exceptions.
>
>The vc6.0 compiler does not handle "exception-specifications", so the
>compiler doesn't know that the
>_com_raise_error function can throw an exception and hence the optimizer is
>free to optimize away the exception handling code
>around the try block.

i can assure you that nothing has been optimized away. take func() and
either remove one of the dummy <int> args, or add one more. this
should have little effect on the code created, right? but it will
change the stack layout slightly.

then, compile with /O2, and the app will run very nicely. exception
thrown and caught as expected. nothing has been optimized away.

the original bug happend in a 700+ KLOC app, and it took me 4 full
days to reduce it to the 30 lines you see. during this process it was
indeed hard to keep the AV from disappearing - usually it does _not_
show up. i'm therefore not very surprised that it disappears if a
compiler option is changed, but i guess the problem is only hidden,
not really removed.

thanks for your reply,

-manfred

[manfred]

On Mon, 7 Oct 2002 12:02:19 -0700, Larry Brasfield
<larry_b...@snotmail.com> wrote:

>I will second that recommendation. For what it's
>worth, space optimization is what Microsoft uses
>internally for its products based on the idea that
>smaller code is generally faster that "fastest"
>code due to cache effects. This fact also means
>that the space optimizer is much better wrung out
>than the speed optimizer because code generation
>problems tend to be discovered and fixed early
>in the compiler release cycle.

aha. thanks for this recommendation that sounds very reasonable. i
wonder, however, why /O2 is the 'default' when creating a new
project...

in fact, we don't care too much about optimization, but just wanted to
compile a release version, and /O2 had been set by the app wizard.

thanks,

-manfred

[Tomas Restrepo [MVP]]

Manfred,

>
> aha. thanks for this recommendation that sounds very reasonable. i
> wonder, however, why /O2 is the 'default' when creating a new
> project...

Ahh, that's a completely different story. To be honest, the IDE defaults for
the compiler settings have never been quite the best, in some cases even
conflicting with what you'd like to have. It's a pitty, really, but not one
we've gotten changed :)

Best advice I can offer is: Learn what the compiler and linker switches are,
and tweak the default project settings into what you want....

--
Tomas Restrepo
tom...@mvps.org

[Bobby Mattappally [MS]]

Does you code still fail with /EHa?

Thank you,
Bobby Mattappally
Microsoft VC++/C# Team

This posting is provided "AS IS" with no warranties, and confers no rights.

--------------------
>From: manfred <mzi...@snafu.de>
>Newsgroups: microsoft.public.vc.language

>Subject: Re: VC6 optimizer bug with _com_raise_error (sample code)
>Date: Tue, 08 Oct 2002 10:05:40 +0200
>Organization: [Posted via] Inter.net Germany GmbH
>Lines: 30

>Xref: cpmsftngxa06 microsoft.public.vc.language:174139
>X-Tomcat-NG: microsoft.public.vc.language

[manfred]

On Tue, 08 Oct 2002 17:29:54 GMT, bob...@online.microsoft.com (Bobby
Mattappally [MS]) wrote:

>Does you code still fail with /EHa?
>
>Thank you,
>Bobby Mattappally
>Microsoft VC++/C# Team

bobby,

thanks for your reply.

i'm not sure i understand your question. if you want to know whether i
still get an assert violation with /EHa, the answer is no. if you want
to know whether the app is now working properly, i don't know; i would
assume no, it's just working by coincidence. do you think that adding
another dummy integer argument really fixes the problem? well, it also
makes the AV go away...

my problem is not so much getting rid of the assert violation; as i
said, any change to the code will remove it. the problem is that i
have a large app and want to know whether i can rely on /O2, or should
use /O2 with /EHa, or should skip /O2 in favour of /O1, or whatever -
i just need to rely on the compiler. so what i'll do, i'll follow
recommendations, switch to /O1 and probably be happy ever after.

i want to mention, however, that this is an app, built with default
IDE settings. there are thousands of these apps out there. are you
saying that all these apps have a problem, as they are not using /EHa?

to get some confidence in /O2 (with or without /EHa) i would like to
look at the exception frames, but i only get so far with matt pietreks
code from the 'depths of win32 SEH' article. i guess its too much
asking for the CxxThrowException sources, but can you give some
information about the tables the compiler sets up (_s_FuncInfo,
_s_TryBlockMapEntry)?

thanks,

-manfred

[Visual C++ Team]

Hi Manfred,

Your behavior is being caused by the dynamic 8-byte stack alignment that is
being performed at the beginning of function "func". (The third instruction
'and esp, -8). The 8-byte alignment is occurring because struct A contains a
double, and double accesses are much faster when made to an 8-byte-aligned
location. The dynamic alignment requires that the unwind-dtor actions obtain
the actual stored callers frame pointer, when reporting parameter's
addresses to the runtime, and this is not occurring in this case (due to a
compiler error). Note that -EHa works only because it causes main to have an
additional 4-byte element in its frame, and so accidentally aligns the stack
pointer incoming to func.

O2 implies Oy: "frame pointer omission". A workaround is to disable "Frame
Pointer Omission", using -Oy- on the command line, after O2. This will cause
the compiler to not attempt to dynamically align func's frame.

We will review whether this can be fixed for VC7.1.

--
Eric Christoffersen
Paul Leathers
Visual C++ Team

This posting is provided "AS IS" with no warranties, and confers no rights.

"Carl Daniel [MVP]" <cpda...@pacbell.net> wrote in message
news:eYFFfhgbCHA.384@tkmsftngp11...

[manfred]

hi, eric and paul,

many thanks for this detailed explanation, and for the time to review
this problem.

just one final question: as recommended by the group i'll probably
switch to /O1. i just saw that /O1 also implies /Oy. would you
recommend /Oy- also when using /O1?

-manfred

[Carl Daniel [MVP]]

"Visual C++ Team" <vc...@microsoft.com> wrote in message
news:O#4ome8bCHA.508@tkmsftngp12...

> Hi Manfred,
>
> Your behavior is being caused by the dynamic 8-byte stack alignment that
is
> being performed at the beginning of function "func". (The third
instruction
> 'and esp, -8). The 8-byte alignment is occurring because struct A contains
a
> double, and double accesses are much faster when made to an 8-byte-aligned
> location. The dynamic alignment requires that the unwind-dtor actions
obtain
> the actual stored callers frame pointer, when reporting parameter's
> addresses to the runtime, and this is not occurring in this case (due to a
> compiler error). Note that -EHa works only because it causes main to have
an
> additional 4-byte element in its frame, and so accidentally aligns the
stack
> pointer incoming to func.
>
> O2 implies Oy: "frame pointer omission". A workaround is to disable "Frame
> Pointer Omission", using -Oy- on the command line, after O2. This will
cause
> the compiler to not attempt to dynamically align func's frame.
>
> We will review whether this can be fixed for VC7.1.

Interesting.

VC7 and later seem to compile this code correctly. But looks may be
deceiving. I haven't been able to find any combination of options & code
permutations that cause VC7 to ever generate the dynamic stack alignment
code.
In most cases, VC7 optimizes func(), structA and structB in their entirety.
Even splitting the test code up into 3 modules so that the optimizer can't
optimize them away, the code still works when compiled with VC7.
Annoyingly, with the code split up, VC6 compiles it correctly too.

Perhaps the real problem is still present in VC7+ but simlpy harder to
elicit.

-cd

[Visual C++ Team]

This error can occur with O1 or O2. You must still use -Oy- to force the
compiler to retrieve the correct frame pointer in destructor actions if the
current frame has been dynamically aligned.

It is generally a good idea to use O2, not O1, for individual functions that
are performance-critical.

--
Eric Christoffersen
Paul Leathers
Visual C++ Team
This posting is provided "AS IS" with no warranties, and confers no rights.

"manfred" <mzi...@snafu.de> wrote in message

news:o939qus6n3to0non6...@4ax.com...

[Visual C++ Team]

We will, indeed, pick up a fix for this compiler issue in VC 7.1, so no
workaround needs to be applied once that product is available.

Thanks.

--
Eric Christoffersen
Paul Leathers
Visual C++ Team
This posting is provided "AS IS" with no warranties, and confers no rights.

"manfred" <mzi...@snafu.de> wrote in message

news:o939qus6n3to0non6...@4ax.com...

[Carl Daniel [MVP]]

"Visual C++ Team" <vc...@microsoft.com> wrote in message
news:#DwAsiUcCHA.2492@tkmsftngp12...

> We will, indeed, pick up a fix for this compiler issue in VC 7.1, so no
> workaround needs to be applied once that product is available.
>
> Thanks.
>
> --
> Eric Christoffersen
> Paul Leathers
> Visual C++ Team
> This posting is provided "AS IS" with no warranties, and confers no
rights.

Excellent!

-cd