icsharpcode / ILSpy

.NET Decompiler with support for PDB generation, ReadyToRun, Metadata (&more) - cross-platform!
21.45k stars 3.35k forks source link

Incorrect decompilation of bgt.un.s on signed operands #830

Closed KrisVandermotten closed 7 years ago

KrisVandermotten commented 7 years ago

Compile the following C# code with Visual Studio 2017 Update 3, optimizations on:

public enum Example { Zero, One, Two, Three, Four, Five, Six, Seven }

public static bool IsGood(Example example)
{
    switch (example)
    {
        case Example.Two:
        case Example.Three:
        case Example.Four:
        case Example.Five:
            return true;
    }

    return false;
}

The method ÌsGood is compiled to:

.method public hidebysig static bool IsGood ( valuetype Test/Example example ) cil managed 
{
    .maxstack 8

    ldarg.0
    ldc.i4.2
    sub
    ldc.i4.3
    bgt.un.s IL_0008

    ldc.i4.1
    ret

    IL_0008:
    ldc.i4.0
    ret
}

Notice the (clever) use of bgt.un.s on signed operands.

ILSpy version 2.4.0.1963 decompiles this method to

public static bool IsGood(Test.Example example)
{
    return example - Test.Example.Two <= 3;
}

This is incorrect. It would return true when passed Example.One, whereas the original code would return false.

There are of course many ways to decompile this correctly, one of them being the original source code. But this is a more reasonable one:

public static bool IsGood(Test.Example example)
{
    return (uint)(example - Test.Example.Two) <= 3;
}
dgrunwald commented 7 years ago

Comparisons ignoring signedness is a known issue in the old decompiler engine.

On your code, the newdecompiler branch produces:

public static bool IsGood(Example example)
{
    if ((uint)(example - 2) <= 3u)
    {
        return true;
    }
    return false;
}
dgrunwald commented 7 years ago

This was fixed in the new decompiler engine (which just landed on the master branch).