Browse Source

Fix #1656: Disable CopyPropagation for split variables.

The code reordering done by copy propagation could cause the lifetimes of different parts of a split variable to start overlapping. This caused incorrect C# to be generated when the variable was recombined.
pull/1725/head
Daniel Grunwald 6 years ago
parent
commit
fb70a2861e
  1. 17
      ICSharpCode.Decompiler.Tests/TestCases/Correctness/FloatingPointArithmetic.cs
  2. 32
      ICSharpCode.Decompiler/IL/Transforms/CopyPropagation.cs

17
ICSharpCode.Decompiler.Tests/TestCases/Correctness/FloatingPointArithmetic.cs

@ -7,6 +7,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness @@ -7,6 +7,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
public static int Main(string[] args)
{
Issue999();
Issue1656();
return 0;
}
@ -20,5 +21,21 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness @@ -20,5 +21,21 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
{
return 0.99f * v + 0.01f;
}
static void Issue1656()
{
double primary = 'B';
CxAssert((++primary) == 'C');
CxAssert((--primary) == 'B');
CxAssert((primary++) == 'B');
CxAssert((primary--) == 'C');
}
static void CxAssert(bool v)
{
if (!v) {
throw new InvalidOperationException();
}
}
}
}

32
ICSharpCode.Decompiler/IL/Transforms/CopyPropagation.cs

@ -18,6 +18,7 @@ @@ -18,6 +18,7 @@
using ICSharpCode.Decompiler.TypeSystem;
using ICSharpCode.Decompiler.Util;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
@ -31,7 +32,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -31,7 +32,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
/// then we can replace the variable with the argument.
/// 2) assignments of address-loading instructions to local variables
/// </summary>
public class CopyPropagation : IBlockTransform, IILTransform
public class CopyPropagation : IILTransform
{
public static void Propagate(StLoc store, ILTransformContext context)
{
@ -41,26 +42,25 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -41,26 +42,25 @@ namespace ICSharpCode.Decompiler.IL.Transforms
DoPropagate(store.Variable, store.Value, block, ref i, context);
}
public void Run(Block block, BlockTransformContext context)
{
RunOnBlock(block, context);
}
public void Run(ILFunction function, ILTransformContext context)
{
var splitVariables = new HashSet<ILVariable>(ILVariableEqualityComparer.Instance);
foreach (var g in function.Variables.GroupBy(v => v, ILVariableEqualityComparer.Instance)) {
if (g.Count() > 1) {
splitVariables.Add(g.Key);
}
}
foreach (var block in function.Descendants.OfType<Block>()) {
if (block.Kind != BlockKind.ControlFlow)
continue;
RunOnBlock(block, context);
RunOnBlock(block, context, splitVariables);
}
}
static void RunOnBlock(Block block, ILTransformContext context)
static void RunOnBlock(Block block, ILTransformContext context, HashSet<ILVariable> splitVariables = null)
{
for (int i = 0; i < block.Instructions.Count; i++) {
ILVariable v;
ILInstruction copiedExpr;
if (block.Instructions[i].MatchStLoc(out v, out copiedExpr)) {
if (block.Instructions[i].MatchStLoc(out ILVariable v, out ILInstruction copiedExpr)) {
if (v.IsSingleDefinition && v.LoadCount == 0 && v.Kind == VariableKind.StackSlot) {
// dead store to stack
if (SemanticHelper.IsPure(copiedExpr.Flags)) {
@ -76,18 +76,21 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -76,18 +76,21 @@ namespace ICSharpCode.Decompiler.IL.Transforms
copiedExpr.AddILRange(block.Instructions[i]);
block.Instructions[i] = copiedExpr;
}
} else if (v.IsSingleDefinition && CanPerformCopyPropagation(v, copiedExpr)) {
} else if (v.IsSingleDefinition && CanPerformCopyPropagation(v, copiedExpr, splitVariables)) {
DoPropagate(v, copiedExpr, block, ref i, context);
}
}
}
}
static bool CanPerformCopyPropagation(ILVariable target, ILInstruction value)
static bool CanPerformCopyPropagation(ILVariable target, ILInstruction value, HashSet<ILVariable> splitVariables)
{
Debug.Assert(target.StackType == value.ResultType);
if (target.Type.IsSmallIntegerType())
return false;
if (splitVariables != null && splitVariables.Contains(target)) {
return false; // non-local code move might change semantics when there's split variables
}
switch (value.OpCode) {
case OpCode.LdLoca:
// case OpCode.LdElema:
@ -100,6 +103,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -100,6 +103,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return true;
case OpCode.LdLoc:
var v = ((LdLoc)value).Variable;
if (splitVariables != null && splitVariables.Contains(v)) {
return false; // non-local code move might change semantics when there's split variables
}
switch (v.Kind) {
case VariableKind.Parameter:
// Parameters can be copied only if they aren't assigned to (directly or indirectly via ldarga)

Loading…
Cancel
Save