dadhi/FastExpressionCompiler

AccessViolationException and other suspicious behaviour on invoking result of CompileFast()

pmg23 opened this issue · 3 comments

pmg23 commented

Using FastExpressionCompiler v3.3.0, the following code throws NullReferenceException in the constructor of Widget, and the test fails. I think it should succeed, and if I replace CompileFast with Compile it does so.

using System;
using System.Linq.Expressions;
using FastExpressionCompiler;
using NUnit.Framework;

[Test]
public void Ought_to_work()
{
    var result = new Example().Get(); // throws
    Assert.AreEqual("foo", result.Value);
}

private sealed class Example
{
    private readonly S _foo = "foo";

    public S Get()
    {
        Expression<Func<S>> lambda = () => new Widget(null, null, null, _foo, null).Dodgy;
        return lambda.CompileFast()();
    }
}

private sealed class Widget
{
    public readonly S Dodgy;

    public Widget(I a, I b, I c, S dodgy, S e)
    {
        Dodgy = dodgy; // throws
    }
}

private interface I { }

private readonly struct S
{
    public readonly bool HasValue;
    public readonly string Value;

    private S(string value)
    {
        HasValue = value is not null;
        Value = value;
    }

    public static implicit operator S(string s) => new(s);
}

Some further observations:

  1. If I remove the Widget constructor's first parameter (I a), the test throws AccessViolationException instead; if I remove a and b the program crashes; but if I remove a, b and c it's just NullReferenceException (no change).
  2. If I remove the Widget constructor's last parameter (S e), or change its type to I, or change its argument at the call site from null to default, the test succeeds.
  3. If I remove the field S.HasValue, the test succeeds.
  4. If I move the field access .Dodgy outside the lambda, the test succeeds:
  Expression<Func<Widget>> lambda = () => new Widget(null, null, null, _foo, null);
  return lambda.CompileFast()().Dodgy;

In summary, it looks like there is undefined behaviour.

dadhi commented

Thanks for the test code. There is a lot complexity wise going on here, considering that you've got a closure of struct field, and also have an explicit string conversion.
But overall, this sample is small enough to find the issue. I will check later.

pmg23 commented

Great, thanks for looking into it so quickly

dadhi commented

The fix is out with v3.3.1