Incorrect compilation of lamba expressions using method parameters

Hello,

yesterday we have ran into a runtime error in our code that was caused by incorrect compilation of the code that uses combination of LINQ expressions and lambda functions that use parameters from defined method. In the attachment you’ll find code snippet that produces this exact error.

LambdaTestCase.zip (1.3 MB)

In the code snippet you can find method Zkopiruj_pripojene_spisy in the file _CSpis_kniha_spisu.Impl.pas which contains the problematic code and our way how to fix it temporarily. If you build the provided solution once with problematic code and a second time with the fix and compare these two dll files in e.g. ILSpy or .NetPeek you will see this difference:

public static void Zkopiruj_pripojene_spisy(IOWManager OWManager, OWObject Obj_odkud, OWObject Obj_kam)
{
	<>c__DisplayClass0 <>c__DisplayClass = new <>c__DisplayClass0();
	<>c__DisplayClass.OWManager = OWManager;
	<>c__DisplayClass.source = Obj_odkud;
	IConditionableQuery<_ICSpis> conditionableQuery = <>c__DisplayClass.OWManager.Query<_ICSpis>();
	ParameterExpression parameterExpression = Expression.Parameter(typeof(_ICSpis), "s");
	LabelTarget target = Expression.Label();
	ParameterExpression parameterExpression2 = Expression.Variable(typeof(bool), "Result");
	ParameterExpression parameterExpression3 = Expression.Variable(typeof(<>c__DisplayClass1), "OX$<>8__locals1");
	ParameterExpression parameterExpression4 = Expression.Parameter(typeof(_ICSpis), "ks");
	List<long> kolekceSpisuIDOdkud = (from s in conditionableQuery.Where(Expression.Lambda<Func<_ICSpis, bool>>(Expression.Block(new ParameterExpression[1] { parameterExpression2 } as IEnumerable<ParameterExpression>, Expression.Block(new ParameterExpression[1] { parameterExpression3 } as IEnumerable<ParameterExpression>, Expression.Assign(parameterExpression3, Expression.New((ConstructorInfo)MethodBase.GetMethodFromHandle((RuntimeMethodHandle)/*OpCode not supported: LdMemberToken*/, typeof(<>c__DisplayClass1).TypeHandle), new Expression[0])), Expression.Assign(Expression.Field(parameterExpression3, FieldInfo.GetFieldFromHandle((RuntimeFieldHandle)/*OpCode not supported: LdMemberToken*/, typeof(<>c__DisplayClass1).TypeHandle)), Expression.Constant(<>c__DisplayClass, typeof(<>c__DisplayClass0))), Expression.Assign(Expression.Field(parameterExpression3, FieldInfo.GetFieldFromHandle((RuntimeFieldHandle)/*OpCode not supported: LdMemberToken*/, typeof(<>c__DisplayClass1).TypeHandle)), parameterExpression), Expression.Block(Expression.Assign(parameterExpression2, Expression.AndAlso(Expression.GreaterThan(Expression.Property(Expression.Field(parameterExpression3, FieldInfo.GetFieldFromHandle((RuntimeFieldHandle)/*OpCode not supported: LdMemberToken*/, typeof(<>c__DisplayClass1).TypeHandle)), (MethodInfo)MethodBase.GetMethodFromHandle((RuntimeMethodHandle)/*OpCode not supported: LdMemberToken*/, typeof(IOWObject).TypeHandle)), Expression.Constant(0L, typeof(long))), Expression.Call((MethodInfo)MethodBase.GetMethodFromHandle((RuntimeMethodHandle)/*OpCode not supported: LdMemberToken*/, typeof(QueryExtension).TypeHandle), Expression.Call((MethodInfo)MethodBase.GetMethodFromHandle((RuntimeMethodHandle)/*OpCode not supported: LdMemberToken*/, typeof(OWManagerExtensions).TypeHandle), Expression.Field(Expression.Field(parameterExpression3, FieldInfo.GetFieldFromHandle((RuntimeFieldHandle)/*OpCode not supported: LdMemberToken*/, typeof(<>c__DisplayClass1).TypeHandle)), FieldInfo.GetFieldFromHandle((RuntimeFieldHandle)/*OpCode not supported: LdMemberToken*/, typeof(<>c__DisplayClass0).TypeHandle))), Expression.Quote(Expression.Lambda<Func<_ICSpis, bool>>(Expression.AndAlso(Expression.Equal(parameterExpression4, Expression.Field(Expression.Field(parameterExpression3, FieldInfo.GetFieldFromHandle((RuntimeFieldHandle)/*OpCode not supported: LdMemberToken*/, typeof(<>c__DisplayClass1).TypeHandle)), FieldInfo.GetFieldFromHandle((RuntimeFieldHandle)/*OpCode not supported: LdMemberToken*/, typeof(<>c__DisplayClass0).TypeHandle))), Expression.Equal(Expression.Property(parameterExpression4, (MethodInfo)MethodBase.GetMethodFromHandle((RuntimeMethodHandle)/*OpCode not supported: LdMemberToken*/, typeof(IOWObject).TypeHandle)), Expression.Property(Expression.Field(parameterExpression3, FieldInfo.GetFieldFromHandle((RuntimeFieldHandle)/*OpCode not supported: LdMemberToken*/, typeof(<>c__DisplayClass1).TypeHandle)), (MethodInfo)MethodBase.GetMethodFromHandle((RuntimeMethodHandle)/*OpCode not supported: LdMemberToken*/, typeof(IOWObject).TypeHandle)))), new ParameterExpression[1] { parameterExpression4 }))))), Expression.Return(target))), Expression.Label(target), parameterExpression2), new ParameterExpression[1] { parameterExpression }))
		select s.ID).ToList();
}

VS.

public static void Zkopiruj_pripojene_spisy(IOWManager OWManager, OWObject Obj_odkud, OWObject Obj_kam)
{
	List<long> kolekceSpisuIDOdkud = (from s in OWManager.Query<_ICSpis>()
		where s.ID > 0L && OWManager.Query<_ICSpis>().Any((_ICSpis ks) => ks == Obj_odkud && ks.ID == s.ID)
		select s.ID).ToList();
}

Our main problem with this is that there is a creation of Block objects that are not supported in our code. (Rightfully so)

Thanks in advance.

Any chance this reproduces in a test case w/o external dependencies?

Logged as bugs://E26969.

Apologies, I did not notice that the .elements file was created with .dll references using absolute paths, not relative ones. Here’s a fixed example:

LambdaTestCase_Fix.zip (1.3 MB)

No worries, I had already fixed that. I was asking if this reproduces standalone in a simple project without dependencies, as that reduces the complexity of debugging this by like 100x for the compiler team :wink:

Alright, I tried my best to create a test case entirely independent of any dependencies. Hope it helps. :slight_smile:

LambdaTestCase_NoDeps.zip (711.4 KB)

Hi!
I’m checking the ILCode comparing the correct code vs the incorrect one, but to be honest, I’m not sure about what’s wrong between the generated from:

    // Compiles incorrectly

    var kolekceSpisuIDOdkud := OWManager.Query<IOWBaseEntity>.Where(s -> (s.ID > 0) AND OWManager.Query<IOWBaseEntity>.Any(ks -> ks.ID = s.ID)).ToList();
    
    writeLn("******************************");

    // Compiles correctly

    var manager := OWManager;
    var kolekceSpisuIDOdkud2 := manager.Query<IOWBaseEntity>.Where(s -> (s.ID > 0) AND manager.Query<IOWBaseEntity>.Any(ks -> ks.ID = s.ID)).ToList();

Could you please explain more specific about the issue?

Hi, of course.

If you compile (or build to be precise) the incorrect code and then have a look at it in ILSpy. You will see that creation of Expression.Block happens inside Where method. This instantiation of block expressions causes runtime errors in our libraries where we don’t implement behavior for this.

If you compare it with the “correct” exaple, then IL Spy will produce this:

As you can see, there is no calling of Expression.Block. (Not even in clean IL code, I checked).

So, to sum this up: If we create a local variable manager from the method parameter OWManager inside the method Zkopiruj_pripojene_spisy and use it inside lambda functions, then we do not get any runtime errors in our libraries. However, if we do not create a local variable and use the parameter OWManager directly inside the lambda functions, then the runtime error occurs because of the usage of block expression.

Our version of Elements is 12.0.0.2931

Edit 1
Even in non-elements projects an expression with block around it cannot be created. That’s the reason why we also don’t implement it in our libraries.

Can you make this a test case an .exe where we can see the actual problem (e.g. wring result, crash) happening runtime? Just because IL looks different than from, say, similar C# code, does not necessarily mean the generated code is bad, and it is very hard to diagnose and fix an issue on that level.

It would be good to see what actually fails, in other words.

thanx,
marc

Sorry for this test case “ping pong”, will do better next time. Here’s a test case with runtime example. I’ve added console app project named “RuntimeExample”. Set this as startup project and you will be able to replicate the exception with incorrectly compiled code. If you you the commented code with local variable, then no exception will be thrown.

LambdaTestCase_Runtime.zip (745.2 KB)

Thanks

Ok, I’m trying to better understand your issue. I have analyzed the compiled dll with peverify and the IL looks fine.
The runtime error you said is due to:

        private T BuildCondition(Expression expr)
        {
            switch (expr.NodeType)
            {
                case ExpressionType.AndAlso:
                case ExpressionType.And:
                case ExpressionType.Or:
                case ExpressionType.OrElse:
                    return default(T);
                case ExpressionType.Not:
                    return default(T);

                case ExpressionType.NotEqual:
                case ExpressionType.Equal:
                case ExpressionType.GreaterThan:
                case ExpressionType.GreaterThanOrEqual:
                case ExpressionType.LessThan:
                case ExpressionType.LessThanOrEqual:
                    return default(T);

                case ExpressionType.MemberAccess:
                    if (expr.Type == typeof(bool))
                        return default(T);
                    break;

                case ExpressionType.Call:
                    if (expr.Type == typeof(bool))
                        return default(T);
                    break;
      }
      throw new ArgumentException($"This type of expression in condition is not implemented! ({expr})", nameof(expr));
        }

The node is hitting here is NodeType.Block, there is no case for .Block, then raise the exception. My question is why is this wrong or you consider bad code?

Well, to implement Block expression is quite complicated. Not even Microsoft implements it. Example bellow:

For us it’s just puzzling that not creating a local variable leads to such a complicated code that is not evaluable for us.

May I ask about the current status of this? It’s really an unpleasant situation for us. Thanks

We are checking the issue right now, give us a couple of days and let you know how is going.

bugs://E26969 was closed as fixed.

bugs://E26969 was reopened.

bugs://E26969 was closed as fixed.

Hi, the issue has been fixed today.

2 Likes

Confirming that the issue has been fixed, thank you. :blush:

1 Like

Happy to hear!