From 67e22d7057a4e62ae5236fd7cfff42e255b4a95e Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 29 Aug 2026 10:18:05 +0200 Subject: [PATCH] Flush pending side effects before materializing an operator receiver The receiver slot is appended to the current block, but the expression stack was only flushed inside the per-parameter loop - which the increment operators, taking no parameters, never enter. A side effect still pending on the stack was then emitted after the receiver read it should precede, so the increment applied to a stale value of the field it targets. Assisted-by: Claude:claude-opus-5:Claude Code --- .../CompoundAssignmentOperatorEdgeCases.cs | 21 ++++++++++++ .../CompoundAssignmentOperatorEdgeCases.il | 34 +++++++++++++++++++ ICSharpCode.Decompiler/IL/ILReader.cs | 7 ++++ 3 files changed, 62 insertions(+) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.cs b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.cs index 7540202bc..47f8f8d8b 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.cs @@ -89,6 +89,8 @@ public class EdgeCases public static ShadowTarget Shared; + public ShadowTarget mutableField; + // A statement-level "x++" considers the instance operator first; the only form that always // binds the static operator is a postfix increment whose result is used, so the result goes // to a discard. @@ -97,6 +99,25 @@ public class EdgeCases _ = Shared++; } + public ShadowTarget ReassignField() + { + mutableField = new ShadowTarget(); + return null; + } + + public static void UseTwo(object o, ShadowTarget t) + { + } + + // The receiver read must not be hoisted above side effects already pending on the + // expression stack: ReassignField() replaces the field the increment then applies to. + public void IncrementAfterPendingSideEffect() + { + ShadowTarget o = ReassignField(); + mutableField++; + UseTwo(o, mutableField); + } + // A pre-increment whose result is used prefers the instance operator too, so the increment // becomes a statement of its own before the uses of the new value. public static void ValueUsedPreIncrement() diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.il b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.il index 6e2ec1b15..3a2317461 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.il +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.il @@ -188,6 +188,40 @@ IL_0011: ret } + .field public class ShadowTarget mutableField + + .method public hidebysig instance class ShadowTarget ReassignField() cil managed + { + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: newobj instance void ShadowTarget::.ctor() + IL_0006: stfld class ShadowTarget EdgeCases::mutableField + IL_000b: ldnull + IL_000c: ret + } + + .method public hidebysig static void UseTwo(object o, class ShadowTarget t) cil managed + { + .maxstack 8 + IL_0000: ret + } + + // The receiver read must not be hoisted above side effects already pending on the + // expression stack: ReassignField() replaces the field the increment then applies to. + .method public hidebysig instance void IncrementAfterPendingSideEffect() cil managed + { + .maxstack 3 + IL_0000: ldarg.0 + IL_0001: call instance class ShadowTarget EdgeCases::ReassignField() + IL_0006: ldarg.0 + IL_0007: ldfld class ShadowTarget EdgeCases::mutableField + IL_000c: callvirt instance void ShadowTarget::op_IncrementAssignment() + IL_0011: ldarg.0 + IL_0012: ldfld class ShadowTarget EdgeCases::mutableField + IL_0017: call void EdgeCases::UseTwo(object, class ShadowTarget) + IL_001c: ret + } + // A pre-increment whose result is used: a prefix increment would bind the instance operator, // so the increment has to become a statement of its own before the uses of the new value. .method public hidebysig static void ValueUsedPreIncrement() cil managed diff --git a/ICSharpCode.Decompiler/IL/ILReader.cs b/ICSharpCode.Decompiler/IL/ILReader.cs index bd73725c1..8d3065a1b 100644 --- a/ICSharpCode.Decompiler/IL/ILReader.cs +++ b/ICSharpCode.Decompiler/IL/ILReader.cs @@ -1840,6 +1840,13 @@ namespace ICSharpCode.Decompiler.IL // address, so it already denotes a variable, and copying it into another one would // make the operator mutate the copy. bool materializeReceiver = IsNonStaticOperatorCall() && expectedStackType == StackType.O; + if (materializeReceiver) + { + // The receiver slot is appended to the current block, so everything still + // pending on the expression stack has to be flushed first - the parameter + // loop below does that too, but the increment operators take no parameters. + FlushExpressionStack(); + } for (int i = method.Parameters.Count - 1; i >= 0; i--) { if (requiresLdObjIfRef || materializeReceiver)