Browse Source

Fix various issues that caused conversions to decompile to code with different semantics:

* ensure constant expressions of small integer type have the correct type
* add checked/unchecked annotations for casts, so that we generate the appropriate checked/unchecked blocks
* avoid moving label statements into checked blocks
pull/728/head
Daniel Grunwald 9 years ago
parent
commit
eb48a3764e
  1. 1
      ICSharpCode.Decompiler/CSharp/Annotations.cs
  2. 22
      ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs
  3. 21
      ICSharpCode.Decompiler/CSharp/Transforms/AddCheckedBlocks.cs
  4. 8
      ICSharpCode.Decompiler/IL/Instructions/Conv.cs
  5. 8
      ICSharpCode.Decompiler/Tests/Helpers/Tester.cs
  6. 1
      ICSharpCode.Decompiler/Tests/ICSharpCode.Decompiler.Tests.csproj
  7. 2
      ICSharpCode.Decompiler/Tests/TestCases/Comparisons.cs
  8. 137
      ICSharpCode.Decompiler/Tests/TestCases/Conversions.cs
  9. 18
      ICSharpCode.Decompiler/Tests/TestRunner.cs

1
ICSharpCode.Decompiler/CSharp/Annotations.cs

@ -35,6 +35,7 @@ namespace ICSharpCode.Decompiler.CSharp
// TODO: actually, we could use the type info instead? // TODO: actually, we could use the type info instead?
// * AddCheckedBlocks.CheckedAnnotation / AddCheckedBlocks.UnCheckedAnnotation is used on checked/unchecked integer arithmetic // * AddCheckedBlocks.CheckedAnnotation / AddCheckedBlocks.UnCheckedAnnotation is used on checked/unchecked integer arithmetic
// TODO: here the info is also redundant, we could peek at the BinaryNumericInstruction instead // TODO: here the info is also redundant, we could peek at the BinaryNumericInstruction instead
// but on the other hand, some unchecked casts are not backed by any BinaryNumericInstruction
/// <summary> /// <summary>
/// Currently unused; we'll probably use the LdToken ILInstruction as annotation instead when LdToken support gets reimplemented. /// Currently unused; we'll probably use the LdToken ILInstruction as annotation instead when LdToken support gets reimplemented.

22
ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@ -17,6 +17,7 @@
// DEALINGS IN THE SOFTWARE. // DEALINGS IN THE SOFTWARE.
using System.Diagnostics; using System.Diagnostics;
using ICSharpCode.Decompiler.CSharp.Transforms;
using ICSharpCode.NRefactory.CSharp.TypeSystem; using ICSharpCode.NRefactory.CSharp.TypeSystem;
using ExpressionType = System.Linq.Expressions.ExpressionType; using ExpressionType = System.Linq.Expressions.ExpressionType;
using ICSharpCode.NRefactory.CSharp.Refactoring; using ICSharpCode.NRefactory.CSharp.Refactoring;
@ -68,6 +69,17 @@ namespace ICSharpCode.Decompiler.CSharp
public ExpressionWithResolveResult ConvertConstantValue(ResolveResult rr) public ExpressionWithResolveResult ConvertConstantValue(ResolveResult rr)
{ {
var expr = astBuilder.ConvertConstantValue(rr); var expr = astBuilder.ConvertConstantValue(rr);
var pe = expr as PrimitiveExpression;
if (pe != null) {
if (pe.Value is sbyte)
expr = expr.CastTo(new NRefactory.CSharp.PrimitiveType("sbyte"));
else if (pe.Value is byte)
expr = expr.CastTo(new NRefactory.CSharp.PrimitiveType("byte"));
else if (pe.Value is short)
expr = expr.CastTo(new NRefactory.CSharp.PrimitiveType("short"));
else if (pe.Value is ushort)
expr = expr.CastTo(new NRefactory.CSharp.PrimitiveType("ushort"));
}
var exprRR = expr.Annotation<ResolveResult>(); var exprRR = expr.Annotation<ResolveResult>();
if (exprRR == null) { if (exprRR == null) {
exprRR = rr; exprRR = rr;
@ -510,9 +522,13 @@ namespace ICSharpCode.Decompiler.CSharp
} }
var targetType = compilation.FindType(inst.TargetType.ToKnownTypeCode()); var targetType = compilation.FindType(inst.TargetType.ToKnownTypeCode());
var rr = resolver.WithCheckForOverflow(inst.CheckForOverflow).ResolveCast(targetType, arg.ResolveResult); var rr = resolver.WithCheckForOverflow(inst.CheckForOverflow).ResolveCast(targetType, arg.ResolveResult);
return new CastExpression(ConvertType(targetType), arg.Expression) Expression castExpr = new CastExpression(ConvertType(targetType), arg.Expression);
.WithILInstruction(inst) if (inst.Kind == ConversionKind.Nop || inst.Kind == ConversionKind.Truncate || inst.Kind == ConversionKind.FloatToInt
.WithRR(rr); || (inst.Kind == ConversionKind.ZeroExtend && arg.Type.GetSign() == Sign.Signed))
{
castExpr.AddAnnotation(inst.CheckForOverflow ? AddCheckedBlocks.CheckedAnnotation : AddCheckedBlocks.UncheckedAnnotation);
}
return castExpr.WithILInstruction(inst).WithRR(rr);
} }
protected internal override TranslatedExpression VisitCall(Call inst) protected internal override TranslatedExpression VisitCall(Call inst)

21
ICSharpCode.Decompiler/CSharp/Transforms/AddCheckedBlocks.cs

@ -48,16 +48,16 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
(where goal 1 has the highest priority) (where goal 1 has the highest priority)
Goal 4a (open checked blocks as late as possible) is necessary so that we don't move variable declarations Goal 4a (open checked blocks as late as possible) is necessary so that we don't move variable declarations
into checked blocks, as the variable might still be used after the checked block. into checked blocks, as the variable might still be used after the checked block.
(this could cause DeclareVariables to omit the variable declaration, producing incorrect code) (this could cause DeclareVariables to omit the variable declaration, producing incorrect code)
Goal 4b (close checked blocks as late as possible) makes the code look nicer in this case: Goal 4b (close checked blocks as late as possible) makes the code look nicer in this case:
checked { checked {
int c = a + b; int c = a + b;
int r = a + c; int r = a + c;
return r; return r;
} }
If the checked block was closed as early as possible, the variable r would have to be declared outside If the checked block was closed as early as possible, the variable r would have to be declared outside
(this would work, but look badly) (this would work, but look badly)
*/ */
#region struct Cost #region struct Cost
@ -302,6 +302,13 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
costUncheckedContextCheckedBlockOpen += stmtResult.CostInCheckedContext; costUncheckedContextCheckedBlockOpen += stmtResult.CostInCheckedContext;
nodesUncheckedContextCheckedBlockOpen += stmtResult.NodesToInsertInCheckedContext; nodesUncheckedContextCheckedBlockOpen += stmtResult.NodesToInsertInCheckedContext;
if (statement is LabelStatement) {
// We can't move labels into blocks because that might cause goto-statements
// to be unable to just to the labels.
costCheckedContextUncheckedBlockOpen = Cost.Infinite;
costUncheckedContextCheckedBlockOpen = Cost.Infinite;
}
statement = statement.GetNextStatement(); statement = statement.GetNextStatement();
} }

8
ICSharpCode.Decompiler/IL/Instructions/Conv.cs

@ -17,6 +17,7 @@
// DEALINGS IN THE SOFTWARE. // DEALINGS IN THE SOFTWARE.
using System; using System;
using System.Diagnostics;
namespace ICSharpCode.Decompiler.IL namespace ICSharpCode.Decompiler.IL
{ {
@ -41,7 +42,7 @@ namespace ICSharpCode.Decompiler.IL
IntToFloat, IntToFloat,
/// <summary> /// <summary>
/// Float-to-integer conversion. /// Float-to-integer conversion.
/// (truncates toward zero) /// Truncates toward zero; may perform overflow-checking.
/// </summary> /// </summary>
FloatToInt, FloatToInt,
/// <summary> /// <summary>
@ -56,6 +57,7 @@ namespace ICSharpCode.Decompiler.IL
SignExtend, SignExtend,
/// <summary> /// <summary>
/// Conversion of integer type to larger unsigned integer type. /// Conversion of integer type to larger unsigned integer type.
/// May involve overflow checking (when converting from a small signed type).
/// </summary> /// </summary>
ZeroExtend, ZeroExtend,
/// <summary> /// <summary>
@ -110,6 +112,7 @@ namespace ICSharpCode.Decompiler.IL
this.CheckForOverflow = checkForOverflow; this.CheckForOverflow = checkForOverflow;
this.Sign = sign; this.Sign = sign;
this.Kind = GetConversionKind(targetType, argument.ResultType); this.Kind = GetConversionKind(targetType, argument.ResultType);
Debug.Assert(this.Kind != ConversionKind.Invalid);
} }
/// <summary> /// <summary>
@ -220,6 +223,9 @@ namespace ICSharpCode.Decompiler.IL
case ConversionKind.ZeroExtend: case ConversionKind.ZeroExtend:
output.Write("<zero extend>"); output.Write("<zero extend>");
break; break;
case ConversionKind.Invalid:
output.Write("<invalid>");
break;
} }
output.Write('('); output.Write('(');
Argument.WriteTo(output); Argument.WriteTo(output);

8
ICSharpCode.Decompiler/Tests/Helpers/Tester.cs

@ -5,6 +5,7 @@ using System.Diagnostics;
using System.IO; using System.IO;
using System.Linq; using System.Linq;
using System.Text; using System.Text;
using System.Text.RegularExpressions;
using System.Threading.Tasks; using System.Threading.Tasks;
using ICSharpCode.Decompiler.CSharp; using ICSharpCode.Decompiler.CSharp;
using ICSharpCode.Decompiler.CSharp.Transforms; using ICSharpCode.Decompiler.CSharp.Transforms;
@ -26,12 +27,17 @@ namespace ICSharpCode.Decompiler.Tests.Helpers
{ {
public static CompilerResults CompileCSharp(string sourceFileName, CompilerOptions flags = CompilerOptions.UseDebug) public static CompilerResults CompileCSharp(string sourceFileName, CompilerOptions flags = CompilerOptions.UseDebug)
{ {
List<string> sourceFileNames = new List<string> { sourceFileName };
foreach (Match match in Regex.Matches(File.ReadAllText(sourceFileName), @"#include ""([\w\d./]+)""")) {
sourceFileNames.Add(Path.GetFullPath(Path.Combine(Path.GetDirectoryName(sourceFileName), match.Groups[1].Value)));
}
CSharpCodeProvider provider = new CSharpCodeProvider(new Dictionary<string, string> { { "CompilerVersion", "v4.0" } }); CSharpCodeProvider provider = new CSharpCodeProvider(new Dictionary<string, string> { { "CompilerVersion", "v4.0" } });
CompilerParameters options = new CompilerParameters(); CompilerParameters options = new CompilerParameters();
options.GenerateExecutable = true; options.GenerateExecutable = true;
options.CompilerOptions = "/unsafe /o" + (flags.HasFlag(CompilerOptions.Optimize) ? "+" : "-") + (flags.HasFlag(CompilerOptions.UseDebug) ? " /debug" : ""); options.CompilerOptions = "/unsafe /o" + (flags.HasFlag(CompilerOptions.Optimize) ? "+" : "-") + (flags.HasFlag(CompilerOptions.UseDebug) ? " /debug" : "");
options.ReferencedAssemblies.Add("System.Core.dll"); options.ReferencedAssemblies.Add("System.Core.dll");
CompilerResults results = provider.CompileAssemblyFromFile(options, sourceFileName); CompilerResults results = provider.CompileAssemblyFromFile(options, sourceFileNames.ToArray());
if (results.Errors.Cast<CompilerError>().Any(e => !e.IsWarning)) { if (results.Errors.Cast<CompilerError>().Any(e => !e.IsWarning)) {
StringBuilder b = new StringBuilder("Compiler error:"); StringBuilder b = new StringBuilder("Compiler error:");
foreach (var error in results.Errors) { foreach (var error in results.Errors) {

1
ICSharpCode.Decompiler/Tests/ICSharpCode.Decompiler.Tests.csproj

@ -117,6 +117,7 @@
<Compile Include="Loops.cs" /> <Compile Include="Loops.cs" />
<Compile Include="TestCases\CompoundAssignment.cs" /> <Compile Include="TestCases\CompoundAssignment.cs" />
<Compile Include="TestCases\ControlFlow.cs" /> <Compile Include="TestCases\ControlFlow.cs" />
<Compile Include="TestCases\Conversions.cs" />
<Compile Include="TestCases\DecimalFields.cs" /> <Compile Include="TestCases\DecimalFields.cs" />
<Compile Include="TestCases\Comparisons.cs" /> <Compile Include="TestCases\Comparisons.cs" />
<Compile Include="TestCases\Generics.cs" /> <Compile Include="TestCases\Generics.cs" />

2
ICSharpCode.Decompiler/Tests/TestCases/Comparisons.cs

@ -18,7 +18,7 @@
using System; using System;
//#pragma warning disable 652 #pragma warning disable 652
namespace ICSharpCode.Decompiler.Tests.TestCases namespace ICSharpCode.Decompiler.Tests.TestCases
{ {

137
ICSharpCode.Decompiler/Tests/TestCases/Conversions.cs

@ -0,0 +1,137 @@
// Copyright (c) 2016 Daniel Grunwald
//
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
// software and associated documentation files (the "Software"), to deal in the Software
// without restriction, including without limitation the rights to use, copy, modify, merge,
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
// to whom the Software is furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in all copies or
// substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
// #include "../../../NRefactory/ICSharpCode.NRefactory/Utils/CSharpPrimitiveCast.cs"
using System;
using ICSharpCode.NRefactory.Utils;
namespace ICSharpCode.Decompiler.Tests.TestCases
{
public class Conversions
{
static readonly TypeCode[] targetTypes = {
TypeCode.Char,
TypeCode.SByte,
TypeCode.Byte,
TypeCode.Int16,
TypeCode.UInt16,
TypeCode.Int32,
TypeCode.UInt32,
TypeCode.Int64,
TypeCode.UInt64,
TypeCode.Single,
TypeCode.Double,
//TypeCode.Decimal
};
static object[] inputValues = {
'\0',
'a',
'\uFFFE',
sbyte.MinValue,
sbyte.MaxValue,
(sbyte)-1,
(sbyte)1,
byte.MinValue,
byte.MaxValue,
(byte)1,
short.MinValue,
short.MaxValue,
(short)-1,
(short)1,
ushort.MinValue,
ushort.MaxValue,
(ushort)1,
int.MinValue,
int.MaxValue,
(int)-1,
(int)1,
uint.MinValue,
uint.MaxValue,
(uint)1,
long.MinValue,
long.MaxValue,
(long)-1,
(long)1,
ulong.MinValue,
ulong.MaxValue,
(ulong)1,
-1.1f,
1.1f,
float.MinValue,
float.MaxValue,
float.NegativeInfinity,
float.PositiveInfinity,
float.NaN,
-1.1,
1.1,
double.MinValue,
double.MaxValue,
double.NegativeInfinity,
double.PositiveInfinity,
double.NaN,
decimal.MinValue,
decimal.MaxValue,
decimal.MinusOne,
decimal.One
};
static void Main(string[] args)
{
RunTest(checkForOverflow: false);
RunTest(checkForOverflow: true);
}
static void RunTest(bool checkForOverflow)
{
string mode = checkForOverflow ? "checked" : "unchecked";
foreach (object input in inputValues) {
string inputType = input.GetType().Name;
foreach (var targetType in targetTypes) {
try {
object result = CSharpPrimitiveCast.Cast(targetType, input, checkForOverflow);
Console.WriteLine("{0} ({1})({2}){3} = ({4}){5}", mode, targetType, inputType, input,
result.GetType().Name, result);
} catch (Exception ex) {
Console.WriteLine("{0} ({1})({2}){3} = {4}", mode, targetType, inputType, input,
ex.GetType().Name);
}
}
}
}
static object MM(sbyte c)
{
checked {
return (UInt64)c;
}
}
}
}

18
ICSharpCode.Decompiler/Tests/TestRunner.cs

@ -34,6 +34,12 @@ namespace ICSharpCode.Decompiler.Tests
TestCompileDecompileCompileOutputAll("Comparisons.cs"); TestCompileDecompileCompileOutputAll("Comparisons.cs");
} }
[Test]
public void Conversions()
{
TestCompileDecompileCompileOutputAll("Conversions.cs");
}
[Test] [Test]
public void HelloWorld() public void HelloWorld()
{ {
@ -111,6 +117,18 @@ namespace ICSharpCode.Decompiler.Tests
if (result1 != result2 || output1 != output2 || error1 != error2) { if (result1 != result2 || output1 != output2 || error1 != error2) {
Console.WriteLine("Test {0} failed.", testFileName); Console.WriteLine("Test {0} failed.", testFileName);
Console.WriteLine("Decompiled code in {0}:line 1", decompiledCodeFile); Console.WriteLine("Decompiled code in {0}:line 1", decompiledCodeFile);
string outputFileName = Path.Combine(Path.GetTempPath(), Path.GetFileNameWithoutExtension(testFileName));
File.WriteAllText(outputFileName + ".original.out", output1);
File.WriteAllText(outputFileName + ".decompiled.out", output2);
int diffLine = 0;
foreach (var pair in output1.Split('\n').Zip(output2.Split('\n'), Tuple.Create)) {
diffLine++;
if (pair.Item1 != pair.Item2) {
break;
}
}
Console.WriteLine("Output: {0}.original.out:line {1}", outputFileName, diffLine);
Console.WriteLine("Output: {0}.decompiled.out:line {1}", outputFileName, diffLine);
} }
Assert.AreEqual(result1, result2); Assert.AreEqual(result1, result2);

Loading…
Cancel
Save