From a82430fc785c934e0f594fdc3fe2e7f62ea6a1ca Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Wed, 1 Nov 2017 19:07:58 +0100 Subject: [PATCH] Add missing safety checks to inline assignment transforms. --- .../IL/Transforms/TransformAssignment.cs | 47 ++++++++++++------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs index 0272b5e3f..47dff8a0a 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs @@ -54,7 +54,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// stloc s(value) /// stloc l(ldloc s) /// stobj(..., ldloc s) - /// where ... is pure and does not use s or l + /// where ... is pure and does not use s or l, + /// and where neither the 'stloc s' nor the 'stobj' truncates /// --> /// stloc l(stobj (..., value)) /// @@ -65,6 +66,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// stloc s(value) /// stobj (..., ldloc s) + /// where ... is pure and does not use s, and where the 'stobj' does not truncate /// --> /// stloc s(stobj (..., value)) /// @@ -75,9 +77,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// stloc s(value) /// call set_Property(..., ldloc s) - /// where the '...' arguments are free of side-effects and not using 's' + /// where the '...' arguments are pure and not using 's' /// --> /// stloc s(Block InlineAssign { call set_Property(..., stloc i(value)); final: ldloc i }) + /// new temporary 'i' has type of the property; transform only valid if 'stloc i' doesn't truncate /// bool TransformInlineAssignmentStObjOrCall(Block block, int pos) { @@ -94,6 +97,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (block.Instructions[pos + 1] is StLoc localStore) { // with extra local if (localStore.Variable.Kind != VariableKind.Local || !localStore.Value.MatchLdLoc(inst.Variable)) return false; + // if we're using an extra local, we'll delete "s", so check that that doesn't have any additional uses if (!(inst.Variable.IsSingleDefinition && inst.Variable.LoadCount == 2)) return false; local = localStore.Variable; @@ -211,30 +215,31 @@ namespace ICSharpCode.Decompiler.IL.Transforms internal static bool HandleCallCompoundAssign(CallInstruction setterCall, StatementTransformContext context) { // callvirt set_Property(ldloc S_1, binary.op(callvirt get_Property(ldloc S_1), value)) - // ==> compound.op.new(callvirt(callvirt get_Property(ldloc S_1)), value) + // ==> compound.op.new(callvirt get_Property(ldloc S_1), value) var setterValue = setterCall.Arguments.LastOrDefault(); var storeInSetter = setterValue as StLoc; if (storeInSetter != null) { // callvirt set_Property(ldloc S_1, stloc v(binary.op(callvirt get_Property(ldloc S_1), value))) - // ==> stloc v(compound.op.new(callvirt(callvirt get_Property(ldloc S_1)), value)) + // ==> stloc v(compound.op.new(callvirt get_Property(ldloc S_1), value)) setterValue = storeInSetter.Value; } - if (setterValue is Conv conv && conv.Kind == ConversionKind.Truncate && conv.TargetType.IsSmallIntegerType()) { - // for compound assignments to small integers, the compiler emits a "conv" instruction - setterValue = conv.Argument; - } else { - conv = null; - } + setterValue = UnwrapSmallIntegerConv(setterValue, out var conv); if (!(setterValue is BinaryNumericInstruction binary)) return false; var getterCall = binary.Left as CallInstruction; if (!MatchingGetterAndSetterCalls(getterCall, setterCall)) return false; IType targetType = getterCall.Method.ReturnType; - if (!CompoundAssignmentInstruction.IsBinaryCompatibleWithType(binary, targetType)) + if (!ValidateCompoundAssign(binary, conv, targetType)) return false; - if (conv != null && !(conv.TargetType == targetType.ToPrimitiveType() && conv.CheckForOverflow == binary.CheckForOverflow)) - return false; // conv does not match binary operation + if (storeInSetter != null && storeInSetter.Variable.Type.IsSmallIntegerType()) { + // 'stloc v' implicitly truncates. + // Ensure that type of 'v' must match type of the property: + if (storeInSetter.Variable.Type.GetSize() != targetType.GetSize()) + return false; + if (storeInSetter.Variable.Type.GetSign() != targetType.GetSign()) + return false; + } context.Step($"Compound assignment to '{getterCall.Method.AccessorOwner.Name}'", setterCall); ILInstruction newInst = new CompoundAssignmentInstruction( binary, getterCall, binary.Right, @@ -250,6 +255,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// stobj(target, binary.op(ldobj(target), ...)) + /// where target is pure /// => compound.op(target, ...) /// /// @@ -288,6 +294,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// stloc s(value) /// stloc l(ldloc s) + /// where neither 'stloc s' nor 'stloc l' truncates the value /// --> /// stloc s(stloc l(value)) /// @@ -304,6 +311,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; if (!nextInst.Value.MatchLdLoc(inst.Variable)) return false; + if (IsImplicitTruncation(inst.Value, inst.Variable.Type)) { + // 'stloc s' is implicitly truncating the stack value + return false; + } if (IsImplicitTruncation(inst.Value, nextInst.Variable.Type)) { // 'stloc l' is implicitly truncating the stack value return false; @@ -441,14 +452,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// stobj(target, binary.add(stloc l(ldobj(target)), ldc.i4 1)) - /// where target is pure and does not use 'l' + /// where target is pure and does not use 'l', and the 'stloc l' does not truncate /// --> /// stloc l(compound.op.old(ldobj(target), ldc.i4 1)) /// /// -or- /// /// call set_Prop(args..., binary.add(stloc l(call get_Prop(args...)), ldc.i4 1)) - /// where target is pure and does not use 'l' + /// where args.. are pure and do not use 'l', and the 'stloc l' does not truncate /// --> /// stloc l(compound.op.old(call get_Prop(target), ldc.i4 1)) /// @@ -474,6 +485,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; if (!ValidateCompoundAssign(binary, conv, targetType)) return false; + if (IsImplicitTruncation(stloc.Value, stloc.Variable.Type)) + return false; context.Step("TransformPostIncDecOperatorWithInlineStore", store); block.Instructions[pos] = new StLoc(stloc.Variable, new CompoundAssignmentInstruction( binary, stloc.Value, binary.Right, targetType, CompoundAssignmentType.EvaluatesToOldValue)); @@ -483,7 +496,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// stloc l(ldobj(target)) /// stobj(target, binary.op(ldloc l, ldc.i4 1)) - /// target is pure and does not use 'l' + /// target is pure and does not use 'l', 'stloc does not truncate' /// --> /// stloc l(compound.op.old(ldobj(target), ldc.i4 1)) /// @@ -507,6 +520,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; if (!ValidateCompoundAssign(binary, conv, targetType)) return false; + if (IsImplicitTruncation(value, targetType)) + return false; context.Step("TransformPostIncDecOperator", inst); inst.Value = new CompoundAssignmentInstruction(binary, inst.Value, binary.Right, targetType, CompoundAssignmentType.EvaluatesToOldValue); block.Instructions.RemoveAt(i + 1);