icsharpcode / ILSpy

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

Different IL produced based on where one line of c# code is located #776

Closed wklingler closed 7 years ago

wklingler commented 7 years ago

I'm not sure if you have run across this or not but here is the issue. This function:

public Polygon CalcOuterRect(Polygon fp)
{
            PointD orient = _orientation.NormalizeXY();
            bool bSkipTransform = orient.X== 1.0 && orient.Y== 0.0; // skip if it would be the identity matrix
            double eave0 = _eaveHeight0 + BaseElev;
            double eave1 = _eaveHeight1 + BaseElev;
            Mat4 t = null;

            // [FIXIT] Does this if block need to be here? We're just preserving it from the previous change.
            // I suspect that this was a fix applied to the incorrect place. See GetEaveHeight(), I suspect
            // that's what they were really trying to fix; the 0 elevation check in particular.
            if (_roofType == RoofType.Shed && _eaveHeight0== 0 && _eaveHeight1==0)
            {
                eave0 += 1;
                eave1 += 1;
            }

            BoundingCube bc;
            if (bSkipTransform)
            {
                bc = fp.GetBoundingCube();
            }
            else
            {
                Polygon fpCopy = new Polygon(fp);
                PointD orientation = new PointD(orient.X, orient.Y);
                PointD leftOrientation = orientation.RotateXY90();
                t = new Mat4();
                //otherToSketch.Set3x3Col(orientation, leftOrientation, PointD.Z_Vector); // this is otherToSketch
                t.Set3x3Row(orientation, leftOrientation, PointD.Z_Vector); // this is sketchToOtherF
                //Mat4 sketchToOther = new Mat4(otherToSketch);

                fpCopy.Transform(t);
                bc = fpCopy.GetBoundingCube();
            }
            var lo = bc.lo;
            var hi = bc.hi;

            PointD BL = new PointD(lo.X, hi.Y, eave0);
            PointD FL = new PointD(hi.X, hi.Y, eave0);
            PointD FR = new PointD(hi.X, lo.Y, eave1);
            PointD BR = new PointD(lo.X, lo.Y, eave1);

            Polygon outer = new Polygon(BL, FL, FR, BR);

            if (!bSkipTransform)
            {
                t.Transpose(); // inverse of a rotation transform
                outer.Transform(t);
            }

            return outer;
        }

becomes this in ILSpy:

public Polygon CalcOuterRect(Polygon fp)
{
    PointD orient = this._orientation.NormalizeXY();
    object arg_92_0 = orient.X == 1.0 && orient.Y == 0.0;
    double eave0 = this._eaveHeight0 + this.BaseElev;
    double eave = this._eaveHeight1 + this.BaseElev;
    Mat4 t = null;
    if (this._roofType == RoofType.Shed && this._eaveHeight0 == 0.0 && this._eaveHeight1 == 0.0)
    {
        eave0 += 1.0;
        eave += 1.0;
    }
    object expr_92 = arg_92_0;
    BoundingCube bc;
    if (expr_92 != null)
    {
        bc = fp.GetBoundingCube();
    }
    else
    {
        Polygon arg_DF_0 = new Polygon(fp);
        PointD orientation = new PointD(orient.X, orient.Y, 0.0);
        PointD leftOrientation = orientation.RotateXY90();
        t = new Mat4();
        t.Set3x3Row(orientation, leftOrientation, PointD.Z_Vector);
        arg_DF_0.Transform(t);
        bc = arg_DF_0.GetBoundingCube();
    }
    PointD lo = bc.lo;
    PointD hi = bc.hi;
    PointD arg_15B_0 = new PointD(lo.X, hi.Y, eave0);
    PointD FL = new PointD(hi.X, hi.Y, eave0);
    PointD FR = new PointD(hi.X, lo.Y, eave);
    PointD BR = new PointD(lo.X, lo.Y, eave);
    Polygon outer = new Polygon(arg_15B_0, FL, FR, BR);
    if (expr_92 == null)
    {
        t.Transpose();
        outer.Transform(t);
    }
    return outer;
}

The issue is that when the optimizer is on then the if statement if(expr_92 != null) changes the entire flow of the function because expr_92 will never be null and therefore the else block is never entered. The workaround that I found is if I actually move

bool bSkipTransform = orient.X== 1.0 && orient.Y== 0.0; // skip if it would be the identity matrix

to right before the if statement so it looks like this:

bool bSkipTransform = orient.X== 1.0 && orient.Y== 0.0; // skip if it would be the identity matrix
if (bSkipTransform)

It looks like there might be some optimization that the C# compiler is doing and understands how to resolve the IL code to make it work properly because if I turn the optimizer off for the project then arg_92_0 is actually set to a bool and everything works fine. I am using JSIL to transpile my c# code into javascript code and they use the IL produced from c#. Because this is happening, it makes the functionality of that function not work right. The only reason this worries me is that this problem could happen in a ton of other locations just depending on how the developer likes to format their code. Do you have ideas or suggestions for this or is this something I'm just going to have to live with? Thanks!

dgrunwald commented 7 years ago

Unfortunately this is not easy to fix with the old decompiler engine. Type inference there is basically guesswork, as we don't extract the necessary information from the IL code.

This is fixed in the newdecompiler branch. But that's a completely new decompiler engine, so it might be tricky to port JSIL to that.

dgrunwald commented 7 years ago

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