From 9b2872611e04d017710c9747e7859c3b726578d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Tue, 4 Aug 2026 00:08:56 +0200 Subject: [PATCH] Fix InvalidProgramException when NewArray result is ignored This should also fix more subtle problems with i.e. by-reference targets of the array assignment, since NewArray propagated ParentFlags Fix is to stop propagating parent flags + handle IgnoreResult. Same logic as my fix in 62bde5605 Alternative is to propagate IgnoreResult and then don't create the array. However, array creation asserts that integer is positive, so this is invalid optimization, plus the case is quite patological and IMHO not worth optimizing for. AI use: I let AI generate the tests (before I applied the fix) + for code review of the fix. --- .../FastExpressionCompiler.cs | 15 +- .../ConstructorCallTests.cs | 182 +++++++++++++++++- 2 files changed, 189 insertions(+), 8 deletions(-) diff --git a/src/FastExpressionCompiler/FastExpressionCompiler.cs b/src/FastExpressionCompiler/FastExpressionCompiler.cs index 2b15a206..46712ff4 100644 --- a/src/FastExpressionCompiler/FastExpressionCompiler.cs +++ b/src/FastExpressionCompiler/FastExpressionCompiler.cs @@ -3700,10 +3700,12 @@ private static bool EmitNewArrayBounds(NewArrayExpression expr, ILGenerator il, { var bounds = expr.GetArgExprs(); var boundCount = bounds.GetArgCount(); + var flags = ParentFlags.CtorCall; + for (var i = 0; i < boundCount; i++) + if (!TryEmit(bounds.GetArgument(i), il, ref context, flags)) + return false; if (boundCount == 1) { - if (!TryEmit(bounds.GetArgument(0), il, ref context, parent)) - return false; var elemType = expr.Type.GetElementType(); if (elemType == null) return false; @@ -3711,11 +3713,9 @@ private static bool EmitNewArrayBounds(NewArrayExpression expr, ILGenerator il, } else { - for (var i = 0; i < boundCount; i++) - if (!TryEmit(bounds.GetArgument(i), il, ref context, parent)) - return false; il.Demit(OpCodes.Newobj, expr.Type.GetTypeInfo().DeclaredConstructors.GetFirst()); } + il.EmitPopIfIgnoreResult(parent); return true; } @@ -3743,17 +3743,18 @@ private static bool EmitNewArrayInit(NewArrayExpression expr, ILGenerator il, re if (isElemOfValueType) // loading element address for later copying of value into it. { il.Demit(OpCodes.Ldelema, elemType); - if (!TryEmit(elems.GetArgument(i), il, ref context, parent)) + if (!TryEmit(elems.GetArgument(i), il, ref context, ParentFlags.Empty)) return false; il.Demit(OpCodes.Stobj, elemType); // store element of value type by array element address } else { - if (!TryEmit(elems.GetArgument(i), il, ref context, parent)) + if (!TryEmit(elems.GetArgument(i), il, ref context, ParentFlags.Empty)) return false; il.Demit(OpCodes.Stelem_Ref); } } + il.EmitPopIfIgnoreResult(parent); return true; } diff --git a/test/FastExpressionCompiler.UnitTests/ConstructorCallTests.cs b/test/FastExpressionCompiler.UnitTests/ConstructorCallTests.cs index 5b03214d..44d62fda 100644 --- a/test/FastExpressionCompiler.UnitTests/ConstructorCallTests.cs +++ b/test/FastExpressionCompiler.UnitTests/ConstructorCallTests.cs @@ -1,11 +1,16 @@ using System; using System.Reflection; +using System.Reflection.Emit; +using System.Linq; + #if LIGHT_EXPRESSION +using Expression = FastExpressionCompiler.LightExpression.Expression; using static FastExpressionCompiler.LightExpression.Expression; namespace FastExpressionCompiler.LightExpression.UnitTests; #else +using Expression = System.Linq.Expressions.Expression; using static System.Linq.Expressions.Expression; namespace FastExpressionCompiler.UnitTests; #endif @@ -51,7 +56,14 @@ public int Run() Constructor_nested_in_block_member_access(); Constructor_conversions_and_boxing(); - return 32; + NewArrayBounds_ignored_result(); + NewArrayInit_ignored_result(); + NewArrayInit_in_different_parent_contexts(); + NewArrayInit_with_call_and_nested_init_in_different_parent_contexts(); + NewArrayBounds_with_call_argument_in_different_parent_contexts(); + Multidimensional_NewArrayBounds_in_different_parent_contexts(); + + return 38; } public void Constructor_conversions_and_boxing() @@ -431,4 +443,172 @@ public TestClass(int a, string b) public static int StaticAdd(TestClass c, int value) => c.A + value; } + + public void NewArrayBounds_ignored_result() + { + var array = NewArrayBounds(typeof(int), Constant(1)); + var lambda = Lambda(array); + + lambda.CompileSys()(); + var fast = lambda.CompileFast(true, CompilerFlags.EnableDelegateDebugInfo | CompilerFlags.ThrowOnNotSupportedExpression); + fast(); + fast.AssertOpCodes( + OpCodes.Ldc_I4_1, + OpCodes.Newarr, + OpCodes.Pop, + OpCodes.Ret); + } + + public void NewArrayInit_ignored_result() + { + var array = NewArrayInit(typeof(int), Constant(1), Constant(2)); + var lambda = Lambda(array); + + lambda.CompileSys()(); + var fast = lambda.CompileFast(true, CompilerFlags.EnableDelegateDebugInfo | CompilerFlags.ThrowOnNotSupportedExpression); + fast(); + fast.AssertOpCodes( + OpCodes.Ldc_I4_2, + OpCodes.Newarr, + OpCodes.Dup, + OpCodes.Ldc_I4_0, + OpCodes.Ldelema, + OpCodes.Ldc_I4_1, + OpCodes.Stobj, + OpCodes.Dup, + OpCodes.Ldc_I4_1, + OpCodes.Ldelema, + OpCodes.Ldc_I4_2, + OpCodes.Stobj, + OpCodes.Pop, + OpCodes.Ret); + } + + + private static readonly MethodInfo ArrayFingerprintMethod = + typeof(ConstructorCallTests).GetMethod(nameof(ArrayFingerprint), BindingFlags.Static | BindingFlags.NonPublic); + + private static readonly MethodInfo CreateArrayMethod = + typeof(ConstructorCallTests).GetMethod(nameof(CreateArray), BindingFlags.Static | BindingFlags.NonPublic); + + private static readonly MethodInfo PositiveLengthMethod = + typeof(ConstructorCallTests).GetMethod(nameof(PositiveLength), BindingFlags.Static | BindingFlags.NonPublic); + + private static readonly ConstructorInfo ArrayConsumerConstructor = + typeof(ArrayConsumer).GetConstructor([ typeof(Array) ]); + + public void NewArrayInit_in_different_parent_contexts() => + AssertNewArrayInDifferentParentContexts(value => + NewArrayInit(typeof(int), value, Add(value, Constant(1)))); + + public void NewArrayInit_with_call_and_nested_init_in_different_parent_contexts() => + AssertNewArrayInDifferentParentContexts(value => + NewArrayInit(typeof(int[]), + NewArrayInit(typeof(int), value, Add(value, Constant(1))), + Call(CreateArrayMethod, value))); + + public void NewArrayBounds_with_call_argument_in_different_parent_contexts() => + AssertNewArrayInDifferentParentContexts(value => + NewArrayBounds(typeof(int), Call(PositiveLengthMethod, value))); + + public void Multidimensional_NewArrayBounds_in_different_parent_contexts() => + AssertNewArrayInDifferentParentContexts(value => + NewArrayBounds(typeof(int), value, Call(PositiveLengthMethod, value))); + + private static void AssertNewArrayInDifferentParentContexts( + Func newArray) where TArray : class + { + // Lambda result and block result. + AssertArrayResult(value => newArray(value)); + AssertArrayResult(value => Block(Constant(0), newArray(value))); + // ignored result + AssertArrayResult(value => Block(newArray(value), newArray(value))); + + // Assignment RHS, method argument, constructor argument, and coalesce operand. + AssertScalarResult(value => + { + var array = Variable(typeof(TArray), "array"); + return Block([ array ], + Assign(array, newArray(value)), + Call(ArrayFingerprintMethod, array)); + }); + AssertScalarResult(value => Call(ArrayFingerprintMethod, newArray(value))); + AssertScalarResult(value => Field( + New(ArrayConsumerConstructor, newArray(value)), nameof(ArrayConsumer.Value))); + AssertScalarResult(value => Call(ArrayFingerprintMethod, + Coalesce(newArray(value), Default(typeof(TArray))))); + + // Array length, member access, instance call, index read, and index assignment target. + if (typeof(TArray).GetArrayRank() == 1) + AssertScalarResult(value => ArrayLength(newArray(value))); + AssertScalarResult(value => Property(newArray(value), nameof(Array.Length))); + AssertScalarResult(value => Call(newArray(value), + typeof(Array).GetMethod(nameof(Array.GetLength)), Constant(0))); + AssertScalarResult(value => ReadFirstArrayItem(newArray(value))); + AssertScalarResult(value => AssignFirstArrayItem(newArray(value))); + } + + private static void AssertArrayResult(Func body) where TArray : class + { + var value = Parameter(typeof(int), "value"); + var lambda = Lambda>(body(value), value); + + var expected = lambda.CompileSys()(2); + var fast = lambda.CompileFast(true); + Asserts.IsNotNull(fast); + var actual = fast(2); + + Asserts.AreEqual( + ArrayFingerprint((Array)(object)expected), + ArrayFingerprint((Array)(object)actual)); + } + + private static void AssertScalarResult(Func body) + { + var value = Parameter(typeof(int), "value"); + var lambda = Lambda>(body(value), value); + + var expected = lambda.CompileSys()(2); + var fast = lambda.CompileFast(true); + Asserts.IsNotNull(fast); + Asserts.AreEqual(expected, fast(2)); + } + + private static Expression ReadFirstArrayItem(Expression array) + { + var item = ArrayAccess(array, Enumerable.Repeat(Constant(0), array.Type.GetArrayRank())); + return item.Type.IsArray ? Call(ArrayFingerprintMethod, item) : item; + } + + private static Expression AssignFirstArrayItem(Expression array) + { + var item = ArrayAccess(array, Enumerable.Repeat(Constant(0), array.Type.GetArrayRank())); + var itemType = item.Type; + Expression value = itemType.IsArray + ? NewArrayInit(itemType.GetElementType(), Constant(42)) + : Constant(42, itemType); + var assignment = Assign(item, value); + return itemType.IsArray ? Call(ArrayFingerprintMethod, assignment) : assignment; + } + + private static int ArrayFingerprint(Array array) + { + var result = array.Rank; + for (var dimension = 0; dimension < array.Rank; dimension++) + result = unchecked(result * 31 + array.GetLength(dimension)); + foreach (var item in array) + result = unchecked(result * 31 + + (item is Array nested ? ArrayFingerprint(nested) : item?.GetHashCode() ?? 0)); + return result; + } + + private static int[] CreateArray(int value) => [ value, value + 1 ]; + + private static int PositiveLength(int value) => Math.Abs(value) + 1; + + private sealed class ArrayConsumer(Array array) + { + public readonly int Value = ArrayFingerprint(array); + } + }