Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems with casting under certain conditions #30

Closed
daveaglick opened this issue Dec 9, 2014 · 3 comments
Closed

Problems with casting under certain conditions #30

daveaglick opened this issue Dec 9, 2014 · 3 comments

Comments

@daveaglick
Copy link
Collaborator

It appears as though inline casts are getting eaten, but only under certain conditions. I started out noticing this for enums. For example, consider the following test:

[Fact]
public void TestEnumCast()
{
    Expression<Func<TestEnum, bool>> expected = x => (int)x == 10;
    Func<TestEnum, bool> compiled = x => (int)x == 10;
    Test(expected, compiled);
}

This works fine. However, this test does not:

[Fact]
public void TestEnumCast()
{
    Expression<Func<TestEnum, int>> expected = x => (int)x % 10;
    Func<TestEnum, int> compiled = x => (int)x % 10;
    Test(expected, compiled);
}

It fails with System.InvalidOperationException : The binary operator Modulo is not defined for the types 'DelegateDecompiler.Tests.EnumTests+TestEnum' and 'System.Int32'.

This also doesn't work:

[Fact]
public void TestEnumCast()
{
    Expression<Func<TestEnum, bool>> expected = x => new int[]{ (int)x }[0] == 10;
    Func<TestEnum, bool> compiled = x => new int[]{ (int)x }[0] == 10;
    Test(expected, compiled);
}

The error this time is System.InvalidOperationException : An expression of type 'DelegateDecompiler.Tests.EnumTests+TestEnum' cannot be used to initialize an array of type 'System.Int32'

I figured out this wasn't just an enum problem because this test also fails:

[Fact]
public void TestEnumCast()
{
    Expression<Func<int, bool>> expected = x => new short[]{ (short)x }[0] == (short)10;
    Func<int, bool> compiled = x => new short[]{ (short)x }[0] == (short)10;
    Test(expected, compiled);
}

System.InvalidOperationException : The binary operator Equal is not defined for the types 'System.Int16' and 'System.Int32'.

When stepping through the Processor.Process() method, it doesn't appear as though any IL was generated for the casts in these cases. I assume the issue either lies with Mono.Reflection or with the .NET method MethodBase.GetMethodBody() which Mono.Reflection uses. Unfortunately that makes it difficult to figure out how to fix. And since the exceptions are being thrown in the Processor during decompilation, it's not like the expression tree can be walked to add the casts back in (because there is no expression tree yet).

@daveaglick
Copy link
Collaborator Author

I've done a little more digging. I think the problem is that the decompiled IL won't contain a cast by design for certain compatible types. For example, if I put the following into LINQPad (I just picked an arbitrary enum):

StringComparison s = StringComparison.CurrentCultureIgnoreCase;
int x = (int)s % 10;

It generates the following IL:

IL_0001:  ldc.i4.1    
IL_0002:  stloc.0     // s
IL_0003:  ldloc.0     // s
IL_0004:  ldc.i4.s    0A 
IL_0006:  rem         
IL_0007:  stloc.1     // x

You can see there's no attempt to convert the enum to an int (which makes sense - enum is obviously a compiler construct). This looks almost identical to the IL generated by the following:

int i = 1;
int x = i % 10;

Which generates:

IL_0001:  ldc.i4.1    
IL_0002:  stloc.0     // i
IL_0003:  ldloc.0     // i
IL_0004:  ldc.i4.s    0A 
IL_0006:  rem         
IL_0007:  stloc.1     // x

So I think the problem is that while the IL we get back from Mono.Reflection is correct, the Processor class is not inferring casts when it should because they sometimes get lost in the journey from MethodInfo to IL as part of the optimization.

The solution would be to check for these cases and insert Expression.Convert() calls into the expression tree generated by the Processor when needed. I'll take a look and see if I can find any OSS decompiler code that already implements a check like this.

@hazzik
Copy link
Owner

hazzik commented Dec 9, 2014

We already have AdjustType method which should do this. I'll check later.

@daveaglick
Copy link
Collaborator Author

Great - thanks. Hadn't noticed the AdjustType method. Just let me know if there's anything else I can do to help with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants