tags:

views:

1235

answers:

5

The following code gives different output when running the release inside Visual Studio, and running the release outside Visual Studio. I'm using Visual Studio 2008 and targeting .NET 3.5. I've also tried .NET 3.5 SP1.

When running outside Visual Studio, the JIT should kick in. Either (a) there's something subtle going on with C# that I'm missing or (b) the JIT is actually in error. I'm doubtful that the JIT can go wrong, but I'm running out of other possiblities...

Output when running inside Visual Studio:

    0 0,
    0 1,
    1 0,
    1 1,

Output when running release outside of Visual Studio:

    0 2,
    0 2,
    1 2,
    1 2,

What is the reason?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Test
{
    struct IntVec
    {
        public int x;
        public int y;
    }

    interface IDoSomething
    {
        void Do(IntVec o);
    }

    class DoSomething : IDoSomething
    {
        public void Do(IntVec o)
        {
            Console.WriteLine(o.x.ToString() + " " + o.y.ToString()+",");
        }
    }

    class Program
    {
        static void Test(IDoSomething oDoesSomething)
        {
            IntVec oVec = new IntVec();
            for (oVec.x = 0; oVec.x < 2; oVec.x++)
            {
                for (oVec.y = 0; oVec.y < 2; oVec.y++)
                {
                    oDoesSomething.Do(oVec);
                }
            }
        }

        static void Main(string[] args)
        {
            Test(new DoSomething());
            Console.ReadLine();
        }
    }
}
+6  A: 

I copied your code into a new Console App.

  • Debug Build
    • Correct output with both debugger and no debugger
  • Switched to Release Build
    • Again, correct output both times
  • Created a new x86 configuration (I'm on running X64 Windows 2008 and was using 'Any CPU')
  • Debug Build
    • Got the correct output both F5 and CTRL+F5
  • Release Build
    • Correct output with Debugger attached
    • No debugger - Got the incorrect output

So it is the x86 JIT incorrectly generating the code. Have deleted my original text about reordering of loops etc. A few other answers on here have confirmed that the JIT is unwinding the loop incorrectly when on x86.

To fix the problem you can change the declaration of IntVec to a class and it works in all flavours.

Think this needs to go on MS Connect....

-1 to Microsoft!

Andras Zoltan
Interesting idea, but surely this isn't "optimisation" but a very major bug in the compiler if this is the case? Would have been found by now wouldn't it?
David M
I agree with you. Reordering loops like this could cause untold issues. Actually this seems even less likely, because the for loops can't ever reach 2.
Andras Zoltan
Looks like one of these nasty Heisenbugs :P
arul
Any CPU won't work if the OP (or anyone using his application) has a 32-bit x86 machine. The problem is that the x86 JIT with optimizations enabled generates bad code.
Nick Guerrera
Yeah, you're right... I'll edit and remove that statement.
Andras Zoltan
+25  A: 

I believe this is in a genuine JIT compilation bug. I would report it to Microsoft and see what they say. Interestingly, I found that the x64 JIT does not have the same problem.

Here is my reading of the x86 JIT.

// save context
00000000  push        ebp  
00000001  mov         ebp,esp 
00000003  push        edi  
00000004  push        esi  
00000005  push        ebx  

// put oDoesSomething pointer in ebx
00000006  mov         ebx,ecx 

// zero out edi, this will store y
00000008  xor         edi,edi 

// zero out esi, this will store x
0000000a  xor         esi,esi 

// NOTE: the inner loop is unrolled here.
// set y to 2
0000000c  mov         edi,2 

// call oDoesSomething.DoSomething(x, y) -- y is always 2!?!
00000011  push        edi  
00000012  push        esi  
00000013  mov         ecx,ebx 
00000015  call        dword ptr ds:[002F0010h] 

// call oDoesSomething.DoSomething(x, y) -- y is always 2?!?!
0000001b  push        edi  
0000001c  push        esi  
0000001d  mov         ecx,ebx 
0000001f  call        dword ptr ds:[002F0010h] 

// increment x
00000025  inc         esi  

// loop back to 0000000C if x < 2
00000026  cmp         esi,2 
00000029  jl          0000000C 

// restore context and return
0000002b  pop         ebx  
0000002c  pop         esi  
0000002d  pop         edi  
0000002e  pop         ebp  
0000002f  ret     

This looks like an optimization gone bad to me...

Nick Guerrera
+59  A: 

It is a JIT optimizer bug. It is unrolling the inner loop but not updating the oVec.y value properly:

      for (oVec.x = 0; oVec.x < 2; oVec.x++) {
0000000a  xor         esi,esi                         ; oVec.x = 0
        for (oVec.y = 0; oVec.y < 2; oVec.y++) {
0000000c  mov         edi,2                           ; oVec.y = 2, WRONG!
          oDoesSomething.Do(oVec);
00000011  push        edi  
00000012  push        esi  
00000013  mov         ecx,ebx 
00000015  call        dword ptr ds:[00170210h]        ; first unrolled call
0000001b  push        edi                             ; WRONG! does not increment oVec.y
0000001c  push        esi  
0000001d  mov         ecx,ebx 
0000001f  call        dword ptr ds:[00170210h]        ; second unrolled call
      for (oVec.x = 0; oVec.x < 2; oVec.x++) {
00000025  inc         esi  
00000026  cmp         esi,2 
00000029  jl          0000000C 

The bug disappears when you let oVec.y increment to 4, that's too many calls to unroll.

One workaround is this:

  for (int x = 0; x < 2; x++) {
    for (int y = 0; y < 2; y++) {
      oDoesSomething.Do(new IntVec(x, y));
    }
  }
Hans Passant
+1 This is definitely a bug. I posted more or less the same answer as an edit to my answer while this came in. It seems that answering questions here is often a race...
Nick Guerrera
+1, definitely a bug - I might have identified the conditions for the error (not saying that nobugz found it because of me, though!), but this (and yours, Nick, so +1 for you too) shows that the JIT is the culprit. interesting that the optimisation is either removed or different when IntVec is declared as a class. Even if you explicitly initialise the struct fields to 0 first before the loop the same behaviour is seen.Nasty!
Andras Zoltan
A: 

Good luck with MS Connect. Not only does that site bomb out on dump uploads and what not, they just stare at it for months. There were serious JIT bugs in transition from 2.0 to updated 2.0.xxxx with the 3.5 bits and it took roughly 5 months to fix them.

Having said that, I haven't checked this instance but I stopped bothering as the JIT is crap anyway. Now, what on earth will a new version of CLR bring with 4.0 I am scared to even think about.

But, I hope someone picks up that structs are almost always immutable and dangerous in loops unless you know what you are doing TM.

rama-jka toti
wow - now that is one of the most negative comments I've ever read! Its easy for us developers to vent our frustration at those that supply our tools - but remember they're developers too! I'm not saying MS Connect is faultless; but then show me anything that is.Equally - I think the investigations here show that the struct isn't the problem, it's the JIT.
Andras Zoltan
Than you should go out more :)
rama-jka toti
+1 I think this is a helpful warning before pointing someone at MS Connect. MS Connect is a corporate bug tracking system with slow response times, and often incorrect closes without any comment after multiple months. It's a dip in cold water compared to Stack Overflow :)
Andomar
I agree that it's usually a black hole for suggestions (except some rare ones that become extremely popular), but of the 3-5 bugs that I reported for Visual Studio they fixed one within less than two weeks. Of course being Microsoft they didn't _release_ the fixed version, but still......
romkyns
+3  A: 

The reason why this bug doesn't reproduce when you run the program with an attached debugger, is because by default VS will force the JIT not to use optimizations when compiling the CIL (since that would make the task of actually debugging the code to be much harder .. and actually impossible at times).