From 488b429f7fbdd89ba42e71c4f9f62ccdc18ac563 Mon Sep 17 00:00:00 2001 From: jogibear9988 Date: Wed, 4 Nov 2026 23:17:34 +0100 Subject: [PATCH] fix: duplicate only requested final field addresses Assignments through nested struct fields can leave an extra managed address on the evaluation stack. On master, the regression compiles strictly but invoking the delegate throws InvalidProgramException in both expression APIs. Compute the duplication destination once in the existing member emitter: duplicate the receiver or the final writable address when requested, and retain one address for intermediate struct owners. No new emitter, traversal, context metadata or allocation is introduced. Focused Standard/Light regressions pass on .NET 8, .NET 10 and .NET Framework 4.7.2. Full solution and legacy suites pass. Modern full runners retain only the reproduced upstream nullable MultiplyChecked parity failure. Working benchmark controls retain allocation counts and emitted IL. --- .../FastExpressionCompiler.cs | 7 +- .../Program.cs | 2 + .../Program.cs | 2 + ...NestedStructFieldAddressRegressionTests.cs | 131 ++++++++++++++++++ 4 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 test/FastExpressionCompiler.UnitTests/NestedStructFieldAddressRegressionTests.cs diff --git a/src/FastExpressionCompiler/FastExpressionCompiler.cs b/src/FastExpressionCompiler/FastExpressionCompiler.cs index feb94970b..b0e957416 100644 --- a/src/FastExpressionCompiler/FastExpressionCompiler.cs +++ b/src/FastExpressionCompiler/FastExpressionCompiler.cs @@ -5729,8 +5729,9 @@ public static bool TryEmitMemberGet(MemberExpression expr, ILGenerator il, ref C // we don't need to duplicate the instance if we are working with the field address to save to it directly, // so the field address should be Dupped instead for loading and then storing by-ref for assignment (see below). // (don't forget to Pop if the assignment should be skipped for nullable with `null` value) - if ((parent & ParentFlags.DupIt) != 0 & - (!isByAddress | (parent & ParentFlags.AssignmentLeftValue) == 0)) + var dupIt = (parent & ParentFlags.DupIt) != 0; + var dupAddress = dupIt && isByAddress && (parent & ParentFlags.AssignmentLeftValue) != 0; + if (dupIt && !dupAddress) il.Demit(OpCodes.Dup); context.LastEmitIsAddress = isByAddress; @@ -5743,7 +5744,7 @@ public static bool TryEmitMemberGet(MemberExpression expr, ILGenerator il, ref C else { il.Demit(OpCodes.Ldflda, field); - if ((parent & ParentFlags.AssignmentLeftValue) != 0) + if (dupAddress) il.Demit(OpCodes.Dup); } } diff --git a/test/FastExpressionCompiler.TestsRunner.Net472/Program.cs b/test/FastExpressionCompiler.TestsRunner.Net472/Program.cs index 34674fd9f..7bdfc33bb 100644 --- a/test/FastExpressionCompiler.TestsRunner.Net472/Program.cs +++ b/test/FastExpressionCompiler.TestsRunner.Net472/Program.cs @@ -34,6 +34,7 @@ static void RunLightExpressionTests(object state) var justLightTestsStopwatch = Stopwatch.StartNew(); var t = (LightExpression.TestRun)state; + t.Run(new LightExpression.UnitTests.NestedStructFieldAddressRegressionTests()); t.Run(new LightExpression.IssueTests.Issue183_NullableDecimal()); t.Run(new LightExpression.IssueTests.Issue398_Optimize_Switch_with_OpCodes_Switch()); t.Run(new LightExpression.IssueTests.Issue431_Add_structural_equality_comparison_to_LightExpression()); @@ -59,6 +60,7 @@ static void RunLightExpressionTests(object state) var fecTestsStopwatch = Stopwatch.StartNew(); var fecTests = new TestRun(); + fecTests.Run(new NestedStructFieldAddressRegressionTests()); fecTests.Run(new Issue183_NullableDecimal()); fecTests.Run(new Issue468_Optimize_the_delegate_access_to_the_Closure_object_for_the_modern_NET()); fecTests.Run(new Issue472_TryInterpret_and_Reduce_primitive_arithmetic_and_logical_expressions_during_the_compilation()); diff --git a/test/FastExpressionCompiler.TestsRunner/Program.cs b/test/FastExpressionCompiler.TestsRunner/Program.cs index 036aee7c7..ca07a0cad 100644 --- a/test/FastExpressionCompiler.TestsRunner/Program.cs +++ b/test/FastExpressionCompiler.TestsRunner/Program.cs @@ -64,6 +64,7 @@ public static void Main(string[] args) { var ltStopwatch = Stopwatch.StartNew(); #if NET8_0_OR_GREATER + lt.Run(new LightExpression.UnitTests.NestedStructFieldAddressRegressionTests()); lt.Run(new LightExpression.IssueTests.Issue490_Regression_in_compiling_lambdas_with_ref_struct_parameters()); #endif @@ -85,6 +86,7 @@ public static void Main(string[] args) }); var stStopwatch = Stopwatch.StartNew(); + st.Run(new NestedStructFieldAddressRegressionTests()); st.Run(new Issue499_InvalidProgramException_for_Sorting_and_comparison_function()); st.Run(new Issue561_NonBooleanEqualityOperator()); st.Run(new ReductionProgressRegressionTests()); diff --git a/test/FastExpressionCompiler.UnitTests/NestedStructFieldAddressRegressionTests.cs b/test/FastExpressionCompiler.UnitTests/NestedStructFieldAddressRegressionTests.cs new file mode 100644 index 000000000..fd228f8b4 --- /dev/null +++ b/test/FastExpressionCompiler.UnitTests/NestedStructFieldAddressRegressionTests.cs @@ -0,0 +1,131 @@ +using System; +using System.Runtime.CompilerServices; +using S = System.Linq.Expressions.Expression; +#if LIGHT_EXPRESSION +namespace FastExpressionCompiler.LightExpression.UnitTests; +#else +namespace FastExpressionCompiler.UnitTests; +#endif + +public struct NestedStructFieldAddressRegressionTests : ITestX +{ + public struct Counter { public int Value; } + private static int OwnerReads; + private static int RightReads; + public static StrongBox Owner(StrongBox box) { ++OwnerReads; return box; } + public static int Right(int value) { ++RightReads; return value; } + public static void Increment(ref int value) => ++value; + + private static readonly CompilerFlags[] Flags = + { + CompilerFlags.Default, + CompilerFlags.NoInvocationLambdaInlining, + CompilerFlags.EnableDelegateDebugInfo | CompilerFlags.ThrowOnNotSupportedExpression + }; + + public void Run(TestRun t) + { + Nested_struct_field_assignment_uses_one_owner_address(t); + Compound_and_increment_results_preserve_the_final_address(t); + Nested_struct_field_ref_argument_mutates_original_storage(t); + } + + private static System.Linq.Expressions.MemberExpression Field(System.Linq.Expressions.Expression box) => + S.Field(S.Field(box, nameof(StrongBox.Value)), nameof(Counter.Value)); + + private static System.Linq.Expressions.MethodCallExpression OwnerCall(System.Linq.Expressions.Expression box) => + S.Call(typeof(NestedStructFieldAddressRegressionTests).GetMethod(nameof(Owner)), box); + + public void Nested_struct_field_assignment_uses_one_owner_address(TestContext t) + { + var box = S.Parameter(typeof(StrongBox)); + var value = S.Parameter(typeof(int)); + var source = S.Lambda, int, int>>(S.Assign(Field(OwnerCall(box)), + S.Call(typeof(NestedStructFieldAddressRegressionTests).GetMethod(nameof(Right)), value)), box, value); + Verify(t, source, 5, 12, 12, 12, 1); + + // A void delegate cannot leave an extra intermediate owner address at ret. + var action = S.Lambda, int>>(source.Body, box, value); + var expected = new StrongBox(new Counter { Value = 5 }); + OwnerReads = RightReads = 0; action.Compile()(expected, 17); + t.AreEqual(17, expected.Value.Value); t.AreEqual(1, OwnerReads); t.AreEqual(1, RightReads); + foreach (var flags in Flags) + { + var fast = action.FromSysExpression().CompileFast(true, flags); + if (!t.IsNotNull(fast)) continue; + var actual = new StrongBox(new Counter { Value = 5 }); + OwnerReads = RightReads = 0; fast(actual, 17); + t.AreEqual(17, actual.Value.Value); t.AreEqual(1, OwnerReads); t.AreEqual(1, RightReads); + } + } + + public void Compound_and_increment_results_preserve_the_final_address(TestContext t) + { + var box = S.Parameter(typeof(StrongBox)); + var value = S.Parameter(typeof(int)); + var field = Field(OwnerCall(box)); + var right = S.Call(typeof(NestedStructFieldAddressRegressionTests).GetMethod(nameof(Right)), value); + // System's compound-expression reduction mutates a copy of this nested struct. + // Keep that measured control, and prove writable storage with an explicit + // once-only owner/value expansion using ordinary field assignment. + Verify(t, S.Lambda, int, int>>(S.AddAssign(field, right), box, value), + 5, 7, 12, 12, 1, CompoundControl(box, value, add: true, post: false)); + Verify(t, S.Lambda, int, int>>(S.PostIncrementAssign(field), box, value), + 5, 0, 5, 6, 0, CompoundControl(box, value, add: false, post: true)); + Verify(t, S.Lambda, int, int>>(S.PreIncrementAssign(field), box, value), + 5, 0, 6, 6, 0, CompoundControl(box, value, add: false, post: false)); + } + + private static System.Linq.Expressions.Expression, int, int>> CompoundControl( + System.Linq.Expressions.ParameterExpression box, System.Linq.Expressions.ParameterExpression value, bool add, bool post) + { + var owner = S.Variable(typeof(StrongBox)); + var previous = S.Variable(typeof(int)); + System.Linq.Expressions.Expression increment = add + ? S.Call(typeof(NestedStructFieldAddressRegressionTests).GetMethod(nameof(Right)), value) + : S.Constant(1); + return S.Lambda, int, int>>(S.Block(new[] { owner, previous }, + S.Assign(owner, OwnerCall(box)), + S.Assign(previous, Field(owner)), + S.Assign(Field(owner), S.Add(previous, increment)), + post ? (System.Linq.Expressions.Expression)previous : Field(owner)), box, value); + } + + public void Nested_struct_field_ref_argument_mutates_original_storage(TestContext t) + { + var box = S.Parameter(typeof(StrongBox)); + var value = S.Parameter(typeof(int)); + var source = S.Lambda, int, int>>(S.Block( + S.Call(typeof(NestedStructFieldAddressRegressionTests).GetMethod(nameof(Increment)), Field(OwnerCall(box))), + Field(box)), box, value); + Verify(t, source, 5, 0, 6, 6, 0); + } + + private static void Verify(TestContext t, + System.Linq.Expressions.Expression, int, int>> source, + int initial, int argument, int result, int stored, int rightReads, + System.Linq.Expressions.Expression, int, int>> systemBehaviorControl = null) + { + var expected = new StrongBox(new Counter { Value = initial }); + OwnerReads = RightReads = 0; + t.AreEqual(result, source.Compile()(expected, argument)); + t.AreEqual(systemBehaviorControl == null ? stored : initial, expected.Value.Value); + t.AreEqual(1, OwnerReads); t.AreEqual(rightReads, RightReads); + if (systemBehaviorControl != null) + { + expected.Value = new Counter { Value = initial }; + OwnerReads = RightReads = 0; + t.AreEqual(result, systemBehaviorControl.Compile()(expected, argument)); + t.AreEqual(stored, expected.Value.Value); t.AreEqual(1, OwnerReads); t.AreEqual(rightReads, RightReads); + } + foreach (var flags in Flags) + { + var fast = source.FromSysExpression().CompileFast(true, flags); + if (!t.IsNotNull(fast)) continue; + var actual = new StrongBox(new Counter { Value = initial }); + OwnerReads = RightReads = 0; + t.AreEqual(result, fast(actual, argument)); + t.AreEqual(stored, actual.Value.Value); t.AreEqual(1, OwnerReads); t.AreEqual(rightReads, RightReads); + } + } +}