151

Consider:

using System;

public class Test
{
    enum State : sbyte { OK = 0, BUG = -1 }

    static void Main(string[] args)
    {
        var s = new State[1, 1];
        s[0, 0] = State.BUG;
        State a = s[0, 0];
        Console.WriteLine(a == s[0, 0]); // False
    }
}

How can this be explained? It occurs in debug builds in Visual Studio 2015 when running in the x86 JIT. A release build or running in the x64 JIT prints True as expected.

To reproduce from the command line:

csc Test.cs /platform:x86 /debug

(/debug:pdbonly, /debug:portable and /debug:full also reproduce.)

Peter Mortensen
  • 28,342
  • 21
  • 95
  • 123
shingo
  • 6,928
  • 4
  • 13
  • 29

2 Answers2

163

You found a code generation bug in the .NET 4 x86 jitter. It is a very unusual one, it only fails when the code is not optimized. The machine code looks like this:

        State a = s[0, 0];
013F04A9  push        0                            ; index 2 = 0
013F04AB  mov         ecx,dword ptr [ebp-40h]      ; s[] reference
013F04AE  xor         edx,edx                      ; index 1 = 0
013F04B0  call        013F0058                     ; eax = s[0, 0]
013F04B5  mov         dword ptr [ebp-4Ch],eax      ; $temp1 = eax 
013F04B8  movsx       eax,byte ptr [ebp-4Ch]       ; convert sbyte to int
013F04BC  mov         dword ptr [ebp-44h],eax      ; a = s[0, 0]
        Console.WriteLine(a == s[0, 0]); // False
013F04BF  mov         eax,dword ptr [ebp-44h]      ; a
013F04C2  mov         dword ptr [ebp-50h],eax      ; $temp2 = a
013F04C5  push        0                            ; index 2 = 0
013F04C7  mov         ecx,dword ptr [ebp-40h]      ; s[] reference 
013F04CA  xor         edx,edx                      ; index 1 = 0
013F04CC  call        013F0058                     ; eax = s[0, 0]
013F04D1  mov         dword ptr [ebp-54h],eax      ; $temp3 = eax 
                                               ; <=== Bug here!
013F04D4  mov         eax,dword ptr [ebp-50h]      ; a == s[0, 0] 
013F04D7  cmp         eax,dword ptr [ebp-54h]  
013F04DA  sete        cl  
013F04DD  movzx       ecx,cl  
013F04E0  call        731C28F4  

A plodding affair with lots of temporaries and code duplication, that's normal for unoptimized code. The instruction at 013F04B8 is notable, that is where the necessary conversion from sbyte to a 32-bit integer occurs. The array getter helper function returned 0x0000000FF, equal to State.BUG, and that needs to be converted to -1 (0xFFFFFFFF) before the value can be compared. The MOVSX instruction is a Sign eXtension instruction.

Same thing happens again at 013F04CC, but this time there is no MOVSX instruction to make the same conversion. That's where the chips fall down, the CMP instruction compares 0xFFFFFFFF with 0x000000FF and that is false. So this is an error of omission, the code generator failed to emit MOVSX again to perform the same sbyte to int conversion.

What is particularly unusual about this bug is that this works correctly when you enable the optimizer, it now knows to use MOVSX in both cases.

The probable reason that this bug went undetected for so long is the usage of sbyte as the base type of the enum. Quite rare to do. Using a multi-dimensional array is instrumental as well, the combination is fatal.

Otherwise a pretty critical bug I'd say. How widespread it might be is hard to guess, I only have the 4.6.1 x86 jitter to test. The x64 and the 3.5 x86 jitter generate very different code and avoid this bug. The temporary workaround to keep going is to remove sbyte as the enum base type and let it be the default, int, so no sign extension is necessary.

You can file the bug at connect.microsoft.com, linking to this Q+A should be enough to tell them everything they need to know. Let me know if you don't want to take the time and I'll take care of it.

Hans Passant
  • 873,011
  • 131
  • 1,552
  • 2,371
  • 33
    Good, solid data with the exact cause of such a strange issue, always a pleasure to read, +1. – Lasse V. Karlsen Apr 13 '16 at 09:12
  • 11
    Please post a link to the connect.microsoft.com article so we can vote for it. – Hans Passant Apr 13 '16 at 11:13
  • I assume using `byte` instead of `sbyte` should be fine too and might be preferable if the real code is used with say an ORM where you don't want your flags in the database to take up extra space. – Voo Apr 13 '16 at 14:57
  • I considered recommending BUG = 0xff but decided not to since we can't see why -1 is important. – Hans Passant Apr 13 '16 at 15:02
  • 6
    I'd post the bug [on dotnet/coreclr](https://github.com/dotnet/coreclr/issues) rather than connect, you'll get straight to the JIT devs. – Lucas Trzesniewski Apr 13 '16 at 18:41
  • 8
    I'm a dev on the JIT team at Microsoft. I've reproduced the bug and have opened an issue for it internally (shipping x86 JIT is not yet in the open on github). In terms of timing of when this would be fixed, I anticipate that we will have this fix included in the next major release of the tools. If this bug is having business impact, and you need a fix earlier, please file the connect (connect.microsoft.com) issue so we can look impact and what alternatives we have to getting a fix to you faster. – Russell C. Hadley Apr 14 '16 at 20:21
  • @RussellC.Hadley - here is another one: http://stackoverflow.com/questions/41212637/ryujit-bug-with-ushort-and-equals-override-64bit – Hans Passant Dec 18 '16 at 22:41
8

Let's consider OP's declaration:

enum State : sbyte { OK = 0, BUG = -1 }

Since the bug only occurs when BUG is negative (from -128 to -1) and State is an enum of signed byte I started to suppose that there were a cast issue somewhere.

If you run this:

Console.WriteLine((sbyte)s[0, 0]);
Console.WriteLine((sbyte)State.BUG);
Console.WriteLine(s[0, 0]);
unchecked
{
    Console.WriteLine((byte) State.BUG);
}

it will output :

255

-1

BUG

255

For a reason that I ignore(as of now) s[0, 0] is cast to a byte before evaluation and that's why it claims that a == s[0,0] is false.

Community
  • 1
  • 1
Thomas Ayoub
  • 27,208
  • 15
  • 85
  • 130