Browse Source

Don't use compound assignment when the RHS value does not fit into the LHS type.

pull/1129/head
Daniel Grunwald 7 years ago
parent
commit
75a627d40b
  1. 66
      ICSharpCode.Decompiler.Tests/TestCases/Correctness/CompoundAssignment.cs
  2. 27
      ICSharpCode.Decompiler/CSharp/Transforms/AddCheckedBlocks.cs
  3. 2
      ICSharpCode.Decompiler/IL/Instructions/BinaryNumericInstruction.cs
  4. 46
      ICSharpCode.Decompiler/IL/Instructions/CompoundAssignmentInstruction.cs
  5. 5
      ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs

66
ICSharpCode.Decompiler.Tests/TestCases/Correctness/CompoundAssignment.cs

@ -31,6 +31,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness @@ -31,6 +31,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
UnsignedShiftRightInstanceField();
UnsignedShiftRightStaticProperty();
DivideByBigValue();
Overflow();
}
static void Test(int a, int b)
@ -77,9 +78,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness @@ -77,9 +78,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
}
}
static ushort shortField;
static short shortField;
public static ushort ShortProperty {
public static short ShortProperty {
get {
Console.WriteLine("In get_ShortProperty");
return shortField;
@ -90,6 +91,19 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness @@ -90,6 +91,19 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
}
}
static byte byteField;
public static byte ByteProperty {
get {
Console.WriteLine("In get_ByteProperty");
return byteField;
}
set {
Console.WriteLine("In set_ByteProperty, value={0}", value);
byteField = value;
}
}
public static Dictionary<string, int> GetDict()
{
Console.WriteLine("In GetDict()");
@ -144,15 +158,55 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness @@ -144,15 +158,55 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
static void UnsignedShiftRightStaticProperty()
{
Console.WriteLine("UnsignedShiftRightStaticProperty:");
StaticProperty = -15;
Test(X(), StaticProperty = (int)((uint)StaticProperty >> 2));
ShortProperty = -20;
ShortProperty = (short)((uint)StaticProperty >> 2);
ShortProperty = -30;
ShortProperty = (short)((ushort)StaticProperty >> 2);
}
static void DivideByBigValue()
{
ShortProperty = 5;
// can't use "ShortProperty /= (ushort)(ushort.MaxValue + 3)" because that would be division by 2.
ShortProperty = (ushort)(ShortProperty / (ushort.MaxValue + 3));
Console.WriteLine("DivideByBigValue:");
ByteProperty = 5;
// can't use "ByteProperty /= (byte)(byte.MaxValue + 3)" because that would be division by 2.
ByteProperty = (byte)(ByteProperty / (byte.MaxValue + 3));
ByteProperty = 200;
ByteProperty = (byte)(ByteProperty / Id(byte.MaxValue + 3));
ShortProperty = short.MaxValue;
ShortProperty = (short)(ShortProperty / (short.MaxValue + 3));
}
static void Overflow()
{
Console.WriteLine("Overflow:");
ByteProperty = 0;
ByteProperty = (byte)checked(ByteProperty + 300);
try {
ByteProperty = checked((byte)(ByteProperty + 300));
} catch (OverflowException) {
Console.WriteLine("Overflow OK");
}
ByteProperty = 200;
ByteProperty = (byte)checked(ByteProperty + 100);
ByteProperty = 201;
try {
ByteProperty = checked((byte)(ByteProperty + 100));
} catch (OverflowException) {
Console.WriteLine("Overflow OK");
}
}
static T Id<T>(T val)
{
return val;
}
}
}

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

@ -16,6 +16,7 @@ @@ -16,6 +16,7 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
using System;
using System.Linq;
using ICSharpCode.Decompiler.CSharp.Syntax;
@ -107,6 +108,22 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -107,6 +108,22 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
{
return string.Format("[{0} + {1}]", Blocks, Expressions);
}
/// <summary>
/// Gets the new cost if an expression with this cost is wrapped in a checked/unchecked expression.
/// </summary>
/// <returns></returns>
internal Cost WrapInCheckedExpr()
{
if (Expressions == 0) {
return new Cost(Blocks, 1);
} else {
// hack: penalize multiple layers of nested expressions
// This doesn't really fit into the original idea of minimizing the number of block+expressions;
// but tends to produce better-looking results due to less nesting.
return new Cost(Blocks, Expressions + 2);
}
}
}
#endregion
@ -324,11 +341,13 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -324,11 +341,13 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
// We cannot use checked/unchecked for top-level-expressions.
} else if (expr.Role.IsValid(Expression.Null)) {
// We use '<' so that expressions are introduced on the deepest level possible (goal 3)
if (result.CostInCheckedContext + new Cost(0, 1) < result.CostInUncheckedContext) {
result.CostInUncheckedContext = result.CostInCheckedContext + new Cost(0, 1);
var costIfWrapWithChecked = result.CostInCheckedContext.WrapInCheckedExpr();
var costIfWrapWithUnchecked = result.CostInUncheckedContext.WrapInCheckedExpr();
if (costIfWrapWithChecked < result.CostInUncheckedContext) {
result.CostInUncheckedContext = costIfWrapWithChecked;
result.NodesToInsertInUncheckedContext = result.NodesToInsertInCheckedContext + new InsertedExpression(expr, true);
} else if (result.CostInUncheckedContext + new Cost(0, 1) < result.CostInCheckedContext) {
result.CostInCheckedContext = result.CostInUncheckedContext + new Cost(0, 1);
} else if (costIfWrapWithUnchecked < result.CostInCheckedContext) {
result.CostInCheckedContext = costIfWrapWithUnchecked;
result.NodesToInsertInCheckedContext = result.NodesToInsertInUncheckedContext + new InsertedExpression(expr, false);
}
}

2
ICSharpCode.Decompiler/IL/Instructions/BinaryNumericInstruction.cs

@ -148,7 +148,7 @@ namespace ICSharpCode.Decompiler.IL @@ -148,7 +148,7 @@ namespace ICSharpCode.Decompiler.IL
}
}
string GetOperatorName(BinaryNumericOperator @operator)
internal static string GetOperatorName(BinaryNumericOperator @operator)
{
switch (@operator) {
case BinaryNumericOperator.Add:

46
ICSharpCode.Decompiler/IL/Instructions/CompoundAssignmentInstruction.cs

@ -111,9 +111,19 @@ namespace ICSharpCode.Decompiler.IL @@ -111,9 +111,19 @@ namespace ICSharpCode.Decompiler.IL
}
}
if (binary.Sign != Sign.None) {
if (type.GetSign() != binary.Sign)
return false;
if (type.IsCSharpSmallIntegerType()) {
// C# will use numeric promotion to int, binary op must be signed
if (binary.Sign != Sign.Signed)
return false;
} else {
// C# will use sign from type
if (type.GetSign() != binary.Sign)
return false;
}
}
// Can't transform if the RHS value would be need to be truncated for the LHS type.
if (Transforms.TransformAssignment.IsImplicitTruncation(binary.Right, type, binary.IsLifted))
return false;
return true;
}
@ -148,40 +158,12 @@ namespace ICSharpCode.Decompiler.IL @@ -148,40 +158,12 @@ namespace ICSharpCode.Decompiler.IL
return flags;
}
}
string GetOperatorName(BinaryNumericOperator @operator)
{
switch (@operator) {
case BinaryNumericOperator.Add:
return "add";
case BinaryNumericOperator.Sub:
return "sub";
case BinaryNumericOperator.Mul:
return "mul";
case BinaryNumericOperator.Div:
return "div";
case BinaryNumericOperator.Rem:
return "rem";
case BinaryNumericOperator.BitAnd:
return "bit.and";
case BinaryNumericOperator.BitOr:
return "bit.or";
case BinaryNumericOperator.BitXor:
return "bit.xor";
case BinaryNumericOperator.ShiftLeft:
return "bit.shl";
case BinaryNumericOperator.ShiftRight:
return "bit.shr";
default:
throw new ArgumentOutOfRangeException();
}
}
public override void WriteTo(ITextOutput output, ILAstWritingOptions options)
{
ILRange.WriteTo(output, options);
output.Write(OpCode);
output.Write("." + GetOperatorName(Operator));
output.Write("." + BinaryNumericInstruction.GetOperatorName(Operator));
if (CompoundAssignmentType == CompoundAssignmentType.EvaluatesToNewValue)
output.Write(".new");
else

5
ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs

@ -332,7 +332,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -332,7 +332,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
/// Gets whether 'stobj type(..., value)' would evaluate to a different value than 'value'
/// due to implicit truncation.
/// </summary>
bool IsImplicitTruncation(ILInstruction value, IType type)
static internal bool IsImplicitTruncation(ILInstruction value, IType type, bool allowNullableValue = false)
{
if (!type.IsSmallIntegerType()) {
// Implicit truncation in ILAst only happens for small integer types;
@ -363,6 +363,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -363,6 +363,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return false; // comp returns 0 or 1, which always fits
} else {
IType inferredType = value.InferType();
if (allowNullableValue) {
inferredType = NullableType.GetUnderlyingType(inferredType);
}
if (inferredType.Kind != TypeKind.Unknown) {
return !(inferredType.GetSize() <= type.GetSize() && inferredType.GetSign() == type.GetSign());
}

Loading…
Cancel
Save