Browse Source

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
pull/3972/head
Siegfried Pammer 3 weeks ago
parent
commit
67e22d7057
  1. 21
      ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.cs
  2. 34
      ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.il
  3. 7
      ICSharpCode.Decompiler/IL/ILReader.cs

21
ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.cs

@ -89,6 +89,8 @@ public class EdgeCases @@ -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 @@ -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()

34
ICSharpCode.Decompiler.Tests/TestCases/ILPretty/CompoundAssignmentOperatorEdgeCases.il

@ -188,6 +188,40 @@ @@ -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

7
ICSharpCode.Decompiler/IL/ILReader.cs

@ -1840,6 +1840,13 @@ namespace ICSharpCode.Decompiler.IL @@ -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)

Loading…
Cancel
Save