icsharpcode / ILSpy

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

Comparisons performed with incorrect signed-ness #338

Closed dgrunwald closed 7 years ago

dgrunwald commented 12 years ago

Compile this code in an unchecked context:

public static bool Test1(int x,int y){
    return (uint)x>(uint)y;
}
public static bool Test2(uint x,int y){
    return x>y;
}

Decompiling with ILSpy 2.0 produces:

    public static bool Test1(int x, int y)
    {
        return x > y;
    }
    public static bool Test2(uint x, int y)
    {
        return (ulong)x > (ulong)((long)y);
    }

This is incorrect: Test1 was comparing unsigned ints, the decompiled code is comparing signed ints. Test2 was comparing signed longs, the decompiled code is uncomparing unsigned longs.

qwertie commented 10 years ago

I noticed this bug when decompiling List<T>. Code is

IL_0001: ldarg.0
IL_0002: ldfld int32 class System.Collections.Generic.List`1<!T>::_size
IL_0007: blt.un.s IL_000e

but the C# version says

public T this[int index]
{
    [__DynamicallyInvokable, TargetedPatchingOptOut("Performance critical to inline across NGen image boundaries")]
    get
    {
        if (index >= this._size)
        {
            ThrowHelper.ThrowArgumentOutOfRangeException();
        }
        return this._items[index];
    }

Originally this caused me to make an incorrect statement while discussing List<T> on a web page, but later I thought... wait a minute, surely list[-1] throws ArgumentOutOfRange and not IndexOutOfRange!

dgrunwald commented 7 years ago

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