vs2005 compiler is fatally flawed: generates bad code

In this very simple excerpted code, the compiler fails
to do its job. If you want it recreated, I propose
someone there try to recreate it. I've already got
it happening here. It's as simple as it gets. From
the type of failure, it looks like it's going to
happen a lot.

r12 is assigned the address of mc.decodePriority only
if gDecodeMaxPriority >= 3, yet it, r12, is still used
as the store address when it is not >= 3. r12 is
pointing to nowhereland in the code below, when, for
example, gDecodeMaxPriority is 0.

       if (mc.inWaveMode != 2) { // not if in CopyFile mode
0009637C add         r3, r8, #1, 18
00096380 ldr         r3, [r3, #0xD58]
00096384 cmp         r3, #2
00096388 beq         |DecodeThread + 0x3244 ( 963c4h )|
          if (gDecodeMaxPriority >= 3) {
0009638C ldr         r3, [pc, #0xDA0]
00096390 ldr         r3, parmPtr
00096394 cmp         r3, #3
             mc.decodePriority = THREAD_PRIORITY_TIME_CRITICAL;
00096398 addcs       r12, r8, #3, 20

After the addcs (carry is not set), the registers are:

R0 = 0x00000000 R1 = 0x00000002 R2 = 0x000e844c
R3 = 0x00000000 R4 = 0x00000009 R5 = 0x00000001
R6 = 0x00000020 R7 = 0x00000000 R8 = 0x00102920
R9 = 0x00000000 R10= 0x0013fa58 R11= 0x2a63fe94
R12= 0x03fbdc84 Sp = 0x2a63f454 Lr = 0x000960a4
Pc = 0x0009639c Ps = 0x8000001f

mc is a large structure variable. It's base is in
r8. gDecodeMaxPriority is 0. The assign below,
at 0x963B0

mc.decodePriority = THREAD_PRIORITY_ABOVE_NORMAL;

results in a GPF in release mode

First-chance exception at 0x000963b0 in yourstinkinapp.exe: 0xC0000005:
Access violation writing location 0x03fbe200.

since r12 is never assigned the correct base, since
that was only done in the conditional addcs at 096394, above.

0009639C strcs       r7, [r12, #0x57C]
          if (gDecodeMaxPriority >= 3) {
000963A0 bcs         |DecodeThread + 0x3234 ( 963b4h )|
          }
          else if (gDecodeMaxPriority == 2) {
000963A4 cmp         r3, #2
             mc.decodePriority = THREAD_PRIORITY_HIGHEST;
000963A8 streq       r5, [r12, #0x57C]
          }
          else { // 1 or 0
             mc.decodePriority = THREAD_PRIORITY_ABOVE_NORMAL;
000963AC movne       r3, #2
000963B0 strne       r3, [r12, #0x57C]
          }
          SetThreadPriority(mc.decodeThreadH, mc.decodePriority);
000963B4 add         r3, r8, #3, 20
000963B8 ldr         r1, [r3, #0x57C]
000963BC ldr         r0, [r3, #0x59C]
000963C0 bl          000BBC38
       }

(cl switches used)

/Ox /Os /I ... /D "NDEBUG" /D "_WIN32_WCE=0x501" /D "UNDER_CE" /D "WIN32_PLATFORM_PSPC"
/D "WINCE" /D "_WINDOWS" /D "ARM" /D "_ARM_" /D "_UNICODE" /D "UNICODE"
/FD /MD /GS- /Gy /fp:fast /J /GR /Fo"Windows Mobile 5.0 Pocket PC SDK (ARMV4I)\Release/"
/Fd"Windows Mobile 5.0 Pocket PC SDK (ARMV4I)\Release/vc80.pdb" /W3 /c /Zi /TP

Microsoft Visual Studio 2005
Version 8.0.50727.42 (RTM.050727-4200)
Microsoft .NET Framework
Version 2.0.50727

Installed Edition: Professional
...
Microsoft Visual C++ 2005 77626-009-0000007-41373
...

Also happens using
/O1
/O2

========================

Here is the working generated code, using /Od
As you can see, r12 is assigned in each case.
Which, for this, is good. Better to do it
just once, but then that once has to be done
for all cases, not just the way the optimizer
does it.

       if (mc.inWaveMode != 2) { // not if in CopyFile mode
00094C28 ldr         r3, [pc, #0xC5C]
00094C2C add         r12, r3, #1, 18
00094C30 ldr         r3, [r12, #0xD58]
00094C34 cmp         r3, #2
00094C38 beq         |DecodeThread + 0x5b68 ( 94cb0h )|
          if (gDecodeMaxPriority >= 3) {
00094C3C ldr         r3, [pc, #0xCD8]
00094C40 ldr         r3, parmPtr
00094C44 cmp         r3, #3
00094C48 bcc         |DecodeThread + 0x5b18 ( 94c60h )|
             mc.decodePriority = THREAD_PRIORITY_TIME_CRITICAL;
00094C4C ldr         r2, [pc, #0xC38]
00094C50 mov         r3, #0
00094C54 add         r12, r2, #3, 20
00094C58 str         r3, [r12, #0x57C]
00094C5C b           |DecodeThread + 0x5b4c ( 94c94h )|
          }
          else if (gDecodeMaxPriority == 2) {
00094C60 ldr         r3, [pc, #0xCB4]
00094C64 ldr         r3, parmPtr
00094C68 cmp         r3, #2
00094C6C bne         |DecodeThread + 0x5b3c ( 94c84h )|
             mc.decodePriority = THREAD_PRIORITY_HIGHEST;
00094C70 ldr         r2, [pc, #0xC14]
00094C74 mov         r3, #1
00094C78 add         r12, r2, #3, 20
00094C7C str         r3, [r12, #0x57C]
          }
          else { // 1 or 0
00094C80 b           |DecodeThread + 0x5b4c ( 94c94h )|
             mc.decodePriority = THREAD_PRIORITY_ABOVE_NORMAL;
00094C84 ldr         r2, [pc, #0xC00]
00094C88 mov         r3, #2
00094C8C add         r12, r2, #3, 20
00094C90 str         r3, [r12, #0x57C]
          }
          SetThreadPriority(mc.decodeThreadH, mc.decodePriority);
00094C94 ldr         r3, [pc, #0xBF0]
00094C98 add         r1, r3, #3, 20
00094C9C ldr         r1, [r1, #0x57C]
00094CA0 ldr         r3, [pc, #0xBE4]
00094CA4 add         r0, r3, #3, 20
00094CA8 ldr         r0, [r0, #0x59C]
00094CAC bl          0015FDDC
       }

(Ugh!) Hate to spoil your thinking about this compiler,
but I can't even use it when it makes this type of code.
Not ready for prime time. Not even close.

=========================================

Jump ahead a little over a year and:

SP1 does the code like this:

if (gDecodeMaxPriority >= 3) {
00066B94 bcs         |DecodeThread + 0x3604 ( 66bb0h )|
             }
             else if (gDecodeMaxPriority == 2) {
00066B98 cmp         r3, #2
                mc.decodePriority = THREAD_PRIORITY_HIGHEST;
00066B9C addeq       r12, r8, #3, 20    <--- the needed op
00066BA0 streq       r4, [r12, #0xE38]
             }
             else { // 1 or 0
                mc.decodePriority = THREAD_PRIORITY_ABOVE_NORMAL;
00066BA4 movne       r3, #2
00066BA8 addne       r12, r8, #3, 20    <--- the needed op
00066BAC strne       r3, [r12, #0xE38]
             }

Answer this question

vs2005 compiler is fatally flawed: generates bad code

Dika
SH3 compiler in EVC3 was terrible - I have spent many hours moving code to and fro and trying to get rid of INTERNAL COMPILER ERRORs.
SH4/EVC4 had many problems too (with : operator, "for{ switch {continue}}" and others).
This is absolutely ok for embedded development.
YOU CAN NOT TRUST COMPILER.

What was really terrible is EVC4 debugger because it worked in EVC3 all right and did not work at all in EVC4 if you tried to debug > 10 dlls workspace.

I strive to see whether it is fixed in 8.0, but I don't have time to port our code base at the moment. Hope to do it by the Christmas.
Milind Lele

40th, I have a response from the compiler team, can you look at the suggestions below and let us know if these "fix" the issue (temporarily, we're still looking at long term solutions):

Q: are there any realistic/reasonable workarounds for this

A: Well… a workaround (assumed that the client investigated his problem and found that his problem corresponds to *this* codegen bug) is to declare volatile the local variable whose address is mis-calculated (or calculated only in one if/else branch)

Q: is there some way for customers to know that they’ll hit this, etc

A: in some cases with small if/else branches when in both of them (or just above it, or just after it) access the same variable. We have seen this bug only with local variables till now, and I don’t expect it in other cases.

Q: Do you know how often you’d expect people to hit this bug

A: I don’t know for sure. Seems, though, that we didn’t hit this bug for years so, I guess, it’s not so common to hit this.

It should repro with some register allocator pressure, functions with lots of code (and/or lots of inlining), and, as said before, small conditional branches that access the same local variable.

Thanks,

Jeff

sonaht

40th Floor wrote:
First, using volatile very likely will "cure" THIS particular problem, but I can't trust the compiler. It's that simple.

I take it this is the first compiler bug you've come across Well newsflash: every compiler has them. Here's the list of compilers I personally "can't trust": gcc 2.95.3, gcc 3.2, evc 3, evc 4, VS2002, VS2003 (haven't really run into any compiler bugs yet in VS2005). Some are worse than others *cough* eVC3 *cough* but we can't ship our desktop products with optimizations turned on because they will just plain not work (VS2003). Include orders are sometimes shuffled to avoid INTERNAL COMPILER ERRORs on perfectly legal code and that goes for gcc as well.

So if you don't trust the compiler you're on to something.

40th Floor wrote:
Also, the response that it only happens when locals are involved is plain wrong. Neither of the used variables are locals. This fact, by the way, is apparent to anyone that actually looked at the disassembly posted.

No, it isn't, and I'm living proof of it, but of course I'm not on the compiler team. Why is it so obvious that mc is not a local variable In fact given that name I think anyone with a C/C++ background would be certain/hope that it was a local variable.

swoozie
The disgusting part is that it won't be fixed. It hasn't, and it won't. Not much to that. Nice use of paragraphs there, but more than I can read at one sitting.
Marianne
40th Floor wrote:

Why is it obvious r8 is not anywhere close to the stack frame. I know, now you want to know what that means.

Actually I have some idea what that means, having implemented the missing runtime library functions to use C++ exceptions on Pocket PC 2002 with code compiled with eVC4.10. But in retrospect, I guess it wasn't too hard to spot, no. I guess gDecodeMaxPriority being 2 isn't too good either.

40th Floor wrote:

Now, if, as you say, there a lots of compiler problems, where are the fixes

Depends on what compiler we're talking about, but it's certainly true that Microsoft have an appalling record when it comes to fixing compiler bugs. However, given the tremendous improvments in the VS2005 compiler compared to the older ones, I think it's safe to say that there has been an enormous change of attitude. Let's just hope that means we'll get fixes as well. But at the end of the day this is an economical issue. They will provide compiler fixes if they think it'll make them money.

You're free to use whatever compiler you want to, but if you're developing for Windows Mobile or SmartPhone or whatever they're called today, I know what my choice would be, having researched my options extensively before embarking on the project mentioned above.
Olman Q

So, I'm not going to attempt to address your post right now, but I will reiterate that we are looking at this, and we're trying to figure out what needs to happen. You should expect a response within the next couple of days with more details. Thanks,

Jeff Abraham
Visual Studio
twinkieracer
Why is it obvious r8 is not anywhere close to the stack frame. I know, now you want to know what that means. This really is obvious to anyone that knows what's going on here. Volatile is not an answer. It's not acceptable. Dismissing this as not a problem is what I expected. Hearing from the peanut gallery, that too. haha Now, if, as you say, there a lots of compiler problems, where are the fixes When was the last time you saw any fixes for vs compilers How many years ago aha That's a sorry state of affairs. Terrible. Disgusting, even.
Jason P
It has already been fixed. The unknown is when they will release it.
Ansu

Sorry for the lack of response, I pinged the owners of the servicing of this component some time ago and haven't seen a response (RPC_E_TIMEOUT). It's just been raised again, and someone should get back shortly. Thanks,

Jeff Abraham
ta5artir
First, using volatile very likely will "cure" THIS particular problem, but I can't trust the compiler. It's that simple. It will happen again. It probably is happening again, somewhere else, perhaps all over the place. It was fortunate that r12 was pointing to nowhere; if it had been valid, this would have taken a long time to notice (I would have, by virtue of the thread not running at the right priority, but how long would it have taken ). Where else is this happening What if r12 was pointing to your next paycheck amount, or how much radiation to fire, or (endless list) They'd all "work", but it seems my definition of "work" and yours (compiler person) are different.

This EXACT same code is correct in evc4, so the claim that this hasn't been happening for years sounds made up. You know, excuses. haha It only happens in vs2005. This code is nearly five years old. More, even.

And did you not write,

I've pinged the CE compiler team, and this bug was found and fixed after VS 2005 RTM'd. There's currently a discussion going on about how to get a fix out to customers.

so it's a know bug, fixed even, but ... but what What's the point of the excuses , or the lame "workaround" suggestions, if there's supposedly already a fix If he thinks it's rare, he's not fooling anyone but himself. This can happen easily, just by the type of code it generates I can see that. Or is, "doesn't happen 99% of the time" good enought there now

Also, the response that it only happens when locals are involved is plain wrong. Neither of the used variables are locals. This fact, by the way, is apparent to anyone that actually looked at the disassembly posted.

This is disgusting, really.
JA9999
Compiler bugs are indeed disgusting because they're hard to narrow down (I can tell you spent a lot of time doing so), and it's a big letdown due the expectation that a compiler produces correct code. The compiler team knows this and treat pretty much all code generation bugs as priority 1 (and this would qualify), meaning that they must fix before they ship.  Since they already shipped, then they have to strategize on rereleasing a compiler into the public -- supporting multiple versions of a compiler is a headache, especially if a fix for a bug ripples throughout and changes codegen everywhere. You should prepare yourself for the possible outcome that fixing this bug for a QFE (versus a Service Pack release) poses more risks and costs than not fixing it. (The risk accessment usually comes from actual empirical measurements of where the bug is manifesting, with and without negative consequences.)

The quality bar is not percentage based. It's not even release date based (the quality bar is supposed to be reached well in advance of RTM). It's severity based.  Oh, and 99% correct is actually a very bad quality bar for any compiler. :)

What you have here, 40th, is very likely to be an isolated case because if it were pervasive, it would have reared its ugly head more often and earlier. I only speak this out of my own (and rather biased) perception of the quality of the ARM compiler.  My advice is that the sky is not falling here.  The volatile workaround (not cure) is reasonable: it reduces register pressure and forces the compiler to not optimize access to a particular variable.

You also have the option of using #pragma optimize("",off) around the problematic function. Optimization is one of those "features" that average customers expect to improve with each iteration of the compiler, but you're free to pass on this feature.

Optimizing compilers are complex and, if I may emphasize here, are extremely sensitive to the various interdependencies underneath the hood. e.g. instruction scheduling, register coloring, ARM instruction choices, etc. Contrary to your direct experiences, the VS 2005 version is more likely to produce correct code than its precedessors, by virtue of the fact that a backlog of bugs have been fixed and more of Microsoft's test cases pass regression testing. If the compiler team states that the bug was always there, they're probably right, and you're only seeing it for the first time because the manifestation of the bug is dependent on some other aspect of code generation that masked it prior to VS2005.

If a buggy compiler drives you crazy, try the Gnu compiler... :)

Brian
Jason Lau
Is there a pre-release/RTM for this fix We are running into this same problem and would like a fix as soon as possible, even if it means testing out a beta copy of this fix.

Thanks in advance.
Georgios D

Supposedly this is fixed already and will be available with SP1.

Dave
Messex

I've pinged the CE compiler team, and this bug was found and fixed after VS 2005 RTM'd. There's currently a discussion going on about how to get a fix out to customers. Can you please open a bug on the MSDN feedback center so that this gets higher visibility and we can keep everyone updated Thanks for the bug report,

Jeff Abraham
Visual Studio
vs2005 compiler is fatally flawed: generates bad code

Answer this question