diff --git a/ICSharpCode.Decompiler/IL/ILReader.cs b/ICSharpCode.Decompiler/IL/ILReader.cs index 74346375d..6d398a0d8 100644 --- a/ICSharpCode.Decompiler/IL/ILReader.cs +++ b/ICSharpCode.Decompiler/IL/ILReader.cs @@ -859,6 +859,8 @@ namespace ICSharpCode.Decompiler.IL case ILOpCode.Conv_ovf_u_un: return Push(new Conv(Pop(), PrimitiveType.U, true, Sign.Unsigned)); case ILOpCode.Cpblk: + // This preserves the evaluation order because the ILAst will run + // destAddress; sourceAddress; size. return new Cpblk(size: Pop(StackType.I4), sourceAddress: PopPointer(), destAddress: PopPointer()); case ILOpCode.Div: return BinaryNumeric(BinaryNumericOperator.Div, false, Sign.Signed); @@ -871,6 +873,8 @@ namespace ICSharpCode.Decompiler.IL case ILOpCode.Endfinally: return new Leave(null); case ILOpCode.Initblk: + // This preserves the evaluation order because the ILAst will run + // address; value; size. return new Initblk(size: Pop(StackType.I4), value: Pop(StackType.I4), address: PopPointer()); case ILOpCode.Jmp: return DecodeJmp(); @@ -983,7 +987,7 @@ namespace ICSharpCode.Decompiler.IL case ILOpCode.Or: return BinaryNumeric(BinaryNumericOperator.BitOr); case ILOpCode.Pop: - FlushExpressionStack(); + FlushExpressionStack(); // discard only the value, not the side-effects Pop(); return new Nop() { Kind = NopKind.Pop }; case ILOpCode.Rem: @@ -1002,21 +1006,22 @@ namespace ICSharpCode.Decompiler.IL case ILOpCode.Starg_s: return Starg(ILParser.DecodeIndex(ref reader, opCode)); case ILOpCode.Stind_i1: - return new StObj(value: Pop(StackType.I4), target: PopPointer(), type: compilation.FindType(KnownTypeCode.SByte)); + // target will run before value, thus preserving the evaluation order + return new StObj(value: Pop(StackType.I4), target: PopStObjTarget(), type: compilation.FindType(KnownTypeCode.SByte)); case ILOpCode.Stind_i2: - return new StObj(value: Pop(StackType.I4), target: PopPointer(), type: compilation.FindType(KnownTypeCode.Int16)); + return new StObj(value: Pop(StackType.I4), target: PopStObjTarget(), type: compilation.FindType(KnownTypeCode.Int16)); case ILOpCode.Stind_i4: - return new StObj(value: Pop(StackType.I4), target: PopPointer(), type: compilation.FindType(KnownTypeCode.Int32)); + return new StObj(value: Pop(StackType.I4), target: PopStObjTarget(), type: compilation.FindType(KnownTypeCode.Int32)); case ILOpCode.Stind_i8: - return new StObj(value: Pop(StackType.I8), target: PopPointer(), type: compilation.FindType(KnownTypeCode.Int64)); + return new StObj(value: Pop(StackType.I8), target: PopStObjTarget(), type: compilation.FindType(KnownTypeCode.Int64)); case ILOpCode.Stind_r4: - return new StObj(value: Pop(StackType.F4), target: PopPointer(), type: compilation.FindType(KnownTypeCode.Single)); + return new StObj(value: Pop(StackType.F4), target: PopStObjTarget(), type: compilation.FindType(KnownTypeCode.Single)); case ILOpCode.Stind_r8: - return new StObj(value: Pop(StackType.F8), target: PopPointer(), type: compilation.FindType(KnownTypeCode.Double)); + return new StObj(value: Pop(StackType.F8), target: PopStObjTarget(), type: compilation.FindType(KnownTypeCode.Double)); case ILOpCode.Stind_i: - return new StObj(value: Pop(StackType.I), target: PopPointer(), type: compilation.FindType(KnownTypeCode.IntPtr)); + return new StObj(value: Pop(StackType.I), target: PopStObjTarget(), type: compilation.FindType(KnownTypeCode.IntPtr)); case ILOpCode.Stind_ref: - return new StObj(value: Pop(StackType.O), target: PopPointer(), type: compilation.FindType(KnownTypeCode.Object)); + return new StObj(value: Pop(StackType.O), target: PopStObjTarget(), type: compilation.FindType(KnownTypeCode.Object)); case ILOpCode.Stloc: case ILOpCode.Stloc_s: return Stloc(ILParser.DecodeIndex(ref reader, opCode)); @@ -1048,11 +1053,12 @@ namespace ICSharpCode.Decompiler.IL case ILOpCode.Cpobj: { var type = ReadAndDecodeTypeReference(); + // OK, 'target' runs before 'value: ld' var ld = new LdObj(PopPointer(), type); return new StObj(PopPointer(), ld, type); } case ILOpCode.Initobj: - return InitObj(PopPointer(), ReadAndDecodeTypeReference()); + return InitObj(PopStObjTarget(), ReadAndDecodeTypeReference()); case ILOpCode.Isinst: return Push(new IsInst(Pop(StackType.O), ReadAndDecodeTypeReference())); case ILOpCode.Ldelem: @@ -1080,6 +1086,7 @@ namespace ICSharpCode.Decompiler.IL case ILOpCode.Ldelem_ref: return LdElem(compilation.FindType(KnownTypeCode.Object)); case ILOpCode.Ldelema: + // LdElema will evalute the array before the indices, so we're preserving the evaluation order return Push(new LdElema(indices: Pop(), array: Pop(), type: ReadAndDecodeTypeReference())); case ILOpCode.Ldfld: { @@ -1149,7 +1156,8 @@ namespace ICSharpCode.Decompiler.IL case ILOpCode.Stobj: { var type = ReadAndDecodeTypeReference(); - return new StObj(value: Pop(type.GetStackType()), target: PopPointer(), type: type); + // OK, target runs before value + return new StObj(value: Pop(type.GetStackType()), target: PopStObjTarget(), type: type); } case ILOpCode.Throw: return new Throw(Pop()); @@ -1240,6 +1248,24 @@ namespace ICSharpCode.Decompiler.IL return new LdLoc(currentStack.Peek()); } + /// + /// Pops a value/instruction from the evaluation stack. + /// Note that instructions popped from the stack must be evaluated in the order they + /// were pushed (so in reverse order of the pop calls!). + /// + /// For instructions like 'conv' that pop a single element and then push their result, + /// it's fine to pop just one element as the instruction itself will end up on the stack, + /// thus maintaining the evaluation order. + /// For instructions like 'call' that pop multiple arguments, it's critical that + /// the evaluation order of the resulting ILAst will be reverse from the order of the push + /// calls. + /// For instructions like 'brtrue', it's fine to pop only a part of the stack because + /// ReadInstructions() will flush the evaluation stack before outputting the brtrue instruction. + /// + /// Use FlushExpressionStack() to ensure that following Pop() calls do not return + /// instructions that involve side-effects. This way evaluation order is preserved + /// no matter which order the ILAst will execute the popped instructions in. + /// ILInstruction Pop() { if (expressionStack.Count > 0) @@ -1362,6 +1388,17 @@ namespace ICSharpCode.Decompiler.IL } } + ILInstruction PopStObjTarget() + { + // stobj has a special invariant (StObj.CheckTargetSlot) + // that prohibts inlining LdElema/LdFlda. + if (expressionStack.LastOrDefault() is LdElema or LdFlda) + { + FlushExpressionStack(); + } + return PopPointer(); + } + ILInstruction PopFieldTarget(IField field) { switch (field.DeclaringType.IsReferenceType) @@ -1505,6 +1542,7 @@ namespace ICSharpCode.Decompiler.IL var value = Pop(type.GetStackType()); var index = Pop(); var array = Pop(); + // OK, evaluation order is array, index, value return new StObj(new LdElema(type, array, index) { DelayExceptions = true }, value, type); } @@ -1588,6 +1626,8 @@ namespace ICSharpCode.Decompiler.IL { arguments[0] = Pop(CallInstruction.ExpectedTypeForThisPointer(constrainedPrefix ?? method.DeclaringType)); } + // arguments is in reverse order of the Pop calls, thus + // arguments is now in the correct evaluation order. switch (method.DeclaringType.Kind) { case TypeKind.Array: @@ -1598,20 +1638,23 @@ namespace ICSharpCode.Decompiler.IL if (method.Name == "Set") { var target = arguments[0]; - var value = arguments.Last(); var indices = arguments.Skip(1).Take(arguments.Length - 2).ToArray(); + var value = arguments.Last(); + // preserves evaluation order target,indices,value return new StObj(new LdElema(elementType, target, indices) { DelayExceptions = true }, value, elementType); } if (method.Name == "Get") { var target = arguments[0]; var indices = arguments.Skip(1).ToArray(); + // preserves evaluation order target,indices return Push(new LdObj(new LdElema(elementType, target, indices) { DelayExceptions = true }, elementType)); } if (method.Name == "Address") { var target = arguments[0]; var indices = arguments.Skip(1).ToArray(); + // preserves evaluation order target,indices return Push(new LdElema(elementType, target, indices)); } Warn("Unknown method called on array type: " + method.Name); @@ -1657,6 +1700,8 @@ namespace ICSharpCode.Decompiler.IL { arguments[0] = Pop(); } + // arguments is in reverse order of the Pop calls, thus + // arguments is now in the correct evaluation order. var call = new CallIndirect( header.IsInstance, header.HasExplicitThis, @@ -1674,6 +1719,7 @@ namespace ICSharpCode.Decompiler.IL { var right = Pop(); var left = Pop(); + // left will run before right, thus preserving the evaluation order if ((left.ResultType == StackType.O || left.ResultType == StackType.Ref) && right.ResultType.IsIntegerType()) { @@ -1856,6 +1902,12 @@ namespace ICSharpCode.Decompiler.IL StoreStackForOffset(targetILOffset, ref currentStack); } + /// + /// The expression stack holds ILInstructions that might have side-effects + /// that should have already happened (in the order of the pushes). + /// This method forces these instructions to be added to the instructionBuilder. + /// This is used e.g. to avoid moving side-effects past branches. + /// private void FlushExpressionStack() { foreach (var inst in expressionStack) @@ -1902,6 +1954,7 @@ namespace ICSharpCode.Decompiler.IL { var right = Pop(); var left = Pop(); + // left will run before right, thus preserving the evaluation order if (@operator != BinaryNumericOperator.Add && @operator != BinaryNumericOperator.Sub) { // we are treating all Refs as I, make the conversion explicit