diff --git a/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs b/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs index 799f91517..ebd186188 100644 --- a/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs @@ -139,7 +139,7 @@ namespace ICSharpCode.Decompiler.Tests [Test] public void NullableTests([ValueSource("defaultOptions")] CompilerOptions options) { - RunCS(options: options); + RunCS(options: options, forceRoslynRecompile: true); } [Test] @@ -301,7 +301,7 @@ namespace ICSharpCode.Decompiler.Tests RunCS(options: options); } - void RunCS([CallerMemberName] string testName = null, CompilerOptions options = CompilerOptions.UseDebug) + void RunCS([CallerMemberName] string testName = null, CompilerOptions options = CompilerOptions.UseDebug, bool forceRoslynRecompile = false) { string testFileName = testName + ".cs"; string testOutputFileName = testName + Tester.GetSuffix(options) + ".exe"; @@ -311,7 +311,7 @@ namespace ICSharpCode.Decompiler.Tests outputFile = Tester.CompileCSharp(Path.Combine(TestCasePath, testFileName), options, outputFileName: Path.Combine(TestCasePath, testOutputFileName)); string decompiledCodeFile = Tester.DecompileCSharp(outputFile.PathToAssembly, Tester.GetSettings(options)); - if (options.HasFlag(CompilerOptions.UseMcs)) { + if (forceRoslynRecompile || options.HasFlag(CompilerOptions.UseMcs)) { // For second pass, use roslyn instead of mcs. // mcs has some compiler bugs that cause it to not accept ILSpy-generated code, // for example when there's unreachable code due to other compiler bugs in the first mcs run. diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/NullableTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/NullableTests.cs index 6659f1503..ab73363ac 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/NullableTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/NullableTests.cs @@ -26,6 +26,8 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness { AvoidLifting(); BitNot(); + FieldAccessOrderOfEvaluation(null); + ArrayAccessOrderOfEvaluation(); } static void AvoidLifting() @@ -82,5 +84,73 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness if (!b) throw new InvalidOperationException(); } + + + static T GetValue() + { + Console.WriteLine("GetValue"); + return default(T); + } + + int intField; + + static void FieldAccessOrderOfEvaluation(NullableTests c) + { + Console.WriteLine("GetInt, then NRE:"); + try { + c.intField = GetValue(); + } catch (Exception ex) { + Console.WriteLine(ex.Message); + } + Console.WriteLine("NRE before GetInt:"); + try { +#if CS60 + ref int i = ref c.intField; + i = GetValue(); +#endif + } catch (Exception ex) { + Console.WriteLine(ex.Message); + } + } + + static T[] GetArray() + { + Console.WriteLine("GetArray"); + return null; + } + + static int GetIndex() + { + Console.WriteLine("GetIndex"); + return 0; + } + + static void ArrayAccessOrderOfEvaluation() + { + Console.WriteLine("GetArray direct:"); + try { + GetArray()[GetIndex()] = GetValue(); + } catch (Exception ex) { + Console.WriteLine(ex.Message); + } + Console.WriteLine("GetArray with ref:"); + try { +#if CS60 + ref int elem = ref GetArray()[GetIndex()]; + elem = GetValue(); +#endif + } catch (Exception ex) { + Console.WriteLine(ex.Message); + } + Console.WriteLine("GetArray direct with value-type:"); + try { + // This line is mis-compiled by legacy csc: + // with the legacy compiler the NRE is thrown before the GetValue call; + // with Roslyn the NRE is thrown after the GetValue call. + GetArray()[GetIndex()] = GetValue(); + } catch (Exception ex) { + Console.WriteLine(ex.Message); + } + } } } diff --git a/ICSharpCode.Decompiler/IL/Instructions.cs b/ICSharpCode.Decompiler/IL/Instructions.cs index 7fd43ab93..5827aea95 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.cs +++ b/ICSharpCode.Decompiler/IL/Instructions.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2014 Daniel Grunwald +// Copyright (c) 2014-2020 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 diff --git a/ICSharpCode.Decompiler/IL/Instructions.tt b/ICSharpCode.Decompiler/IL/Instructions.tt index 69c2640eb..70bc3c322 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.tt +++ b/ICSharpCode.Decompiler/IL/Instructions.tt @@ -1,4 +1,4 @@ -// Copyright (c) 2014 Daniel Grunwald +// Copyright (c) 2014-2020 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 @@ -1158,6 +1158,13 @@ protected override void Disconnected() opCode.WriteOpCodePrefix.Add("if (IsReadOnly)" + Environment.NewLine + "\toutput.Write(\"readonly.\");"); opCode.PerformMatchConditions.Add("IsReadOnly == o.IsReadOnly"); }; + + static Action CustomInvariant(string code) + { + return opCode => { + opCode.Invariants.Add(code); + }; + } static Action Pattern = opCode => { BaseClass("PatternInstruction")(opCode); diff --git a/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs b/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs index 6fdd3e382..1c4ca2f41 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs @@ -43,4 +43,30 @@ namespace ICSharpCode.Decompiler.IL } } } + + public sealed partial class StObj + { + /// + /// For a store to a field or array element, C# will only throw NullReferenceException/IndexOfBoundsException + /// after the value-to-be-stored has been computed. + /// This means a LdFlda/LdElema used as target for StObj must have DelayExceptions==true to allow a translation to C# + /// without changing the program semantics. See https://github.com/icsharpcode/ILSpy/issues/2050 + /// + public bool CanInlineIntoTargetSlot(ILInstruction inst) + { + switch (inst.OpCode) { + case OpCode.LdElema: + case OpCode.LdFlda: + Debug.Assert(inst.HasDirectFlag(InstructionFlags.MayThrow)); + // If the ldelema/ldflda may throw a non-delayed exception, inlining will cause it + // to turn into a delayed exception after the translation to C#. + // This is only valid if the value computation doesn't involve any side effects. + return SemanticHelper.IsPure(this.Value.Flags); + // Note that after inlining such a ldelema/ldflda, the normal inlining rules will + // prevent us from inlining an effectful instruction into the value slot. + default: + return true; + } + } + } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs index 5dcc4e6c1..f1e5c3b93 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs @@ -581,6 +581,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms return FindResult.Stop; if (expr.MatchLdLoc(v) || expr.MatchLdLoca(v)) { // Match found, we can inline + if (expr.SlotInfo == StObj.TargetSlot && !((StObj)expr.Parent).CanInlineIntoTargetSlot(expressionBeingMoved)) { + // special case: the StObj.TargetSlot does not accept some kinds of expressions + return FindResult.Stop; + } return FindResult.Found(expr); } else if (expr is Block block) { // Inlining into inline-blocks? diff --git a/ICSharpCode.Decompiler/IL/Transforms/StatementTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/StatementTransform.cs index 34cc92593..27d83877f 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/StatementTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/StatementTransform.cs @@ -45,6 +45,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// Instructions prior to block.Instructions[pos] must not be modified. /// It is valid to read such instructions, but not recommended as those have not been transformed yet. + /// + /// This function is only called on control-flow blocks with unreachable end-point. + /// Thus, the last instruction in the block always must have the EndPointUnreachable flag. + /// ==> Instructions with reachable end can't be last. Some transforms use this to save some bounds checks. /// void Run(Block block, int pos, StatementTransformContext context); } @@ -122,6 +126,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms ctx.rerunPosition = null; } foreach (var transform in children) { + Debug.Assert(block.HasFlag(InstructionFlags.EndPointUnreachable)); transform.Run(block, pos, ctx); #if DEBUG block.Instructions[pos].CheckInvariant(ILPhase.Normal); diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs index 6a9d149c1..78a09fe81 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs @@ -338,9 +338,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// Handle simple case where RuntimeHelpers.InitializeArray is not used. /// - internal static bool HandleSimpleArrayInitializer(ILFunction function, Block block, int pos, ILVariable store, IType elementType, int[] arrayLength, out (ILInstruction[] Indices, ILInstruction Value)[] values, out int elementCount) + internal static bool HandleSimpleArrayInitializer(ILFunction function, Block block, int pos, ILVariable store, IType elementType, int[] arrayLength, out (ILInstruction[] Indices, ILInstruction Value)[] values, out int instructionsToRemove) { - elementCount = 0; + instructionsToRemove = 0; + int elementCount = 0; var length = arrayLength.Aggregate(1, (t, l) => t * l); values = new (ILInstruction[] Indices, ILInstruction Value)[length]; @@ -388,15 +389,35 @@ namespace ICSharpCode.Decompiler.IL.Transforms int j = 0; int i = pos; - for (; i < block.Instructions.Count; i++) { - if (!block.Instructions[i].MatchStObj(out ILInstruction target, out ILInstruction value, out IType type)) + int step; + while (i < block.Instructions.Count) { + InstructionCollection indices; + // stobj elementType(ldelema elementType(ldloc store, indices), value) + if (block.Instructions[i].MatchStObj(out ILInstruction target, out ILInstruction value, out IType type)) { + if (!(target is LdElema ldelem && ldelem.Array.MatchLdLoc(store))) + break; + indices = ldelem.Indices; + step = 1; + // stloc s(ldelema elementType(ldloc store, indices)) + // stobj elementType(ldloc s, value) + } else if (block.Instructions[i].MatchStLoc(out var addressTemporary, out var addressValue)) { + if (!(addressTemporary.IsSingleDefinition && addressTemporary.LoadCount == 1)) + break; + if (!(addressValue is LdElema ldelem && ldelem.Array.MatchLdLoc(store))) + break; + if (!(i + 1 < block.Instructions.Count)) + break; + if (!block.Instructions[i + 1].MatchStObj(out target, out value, out type)) + break; + if (!(target.MatchLdLoc(addressTemporary) && ldelem.Array.MatchLdLoc(store))) + break; + indices = ldelem.Indices; + step = 2; + } else { break; + } if (value.Descendants.OfType().Any(inst => inst.Variable == store)) break; - if (!(target is LdElema ldelem && ldelem.Array.MatchLdLoc(store))) - break; - var indices = ldelem.Indices; - if (indices.Count != arrayLength.Length) break; bool exact; @@ -409,11 +430,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (exact) { values[j] = (nextIndices, value); elementCount++; + instructionsToRemove += step; } else { values[j] = (nextIndices, null); } j++; } while (j < values.Length && !exact); + i += step; } if (i < block.Instructions.Count) { if (block.Instructions[i].MatchStObj(out ILInstruction target, out ILInstruction value, out IType type)) { @@ -430,7 +453,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms values[j] = (nextIndices, null); j++; } - if (pos + elementCount >= block.Instructions.Count) + if (pos + instructionsToRemove >= block.Instructions.Count) return false; return ShouldTransformToInitializer(function, block, pos, elementCount, length); } @@ -670,7 +693,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (type.GetStackType() != value.ResultType) { value = new Conv(value, type.ToPrimitiveType(), false, Sign.None); } - return new StObj(new LdElema(type, array, indices), value, type); + return new StObj(new LdElema(type, array, indices) { DelayExceptions = true }, value, type); } internal static ILInstruction GetNullExpression(IType elementType) diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs index ee1d3625f..4ed88e0c7 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs @@ -441,7 +441,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms return null; toBeConverted[i] = converted; } - return new LdObj(new LdElema(type.ElementType, array(), toBeConverted.SelectArray(f => f())), type.ElementType); + return new LdObj(new LdElema(type.ElementType, array(), toBeConverted.SelectArray(f => f())) { DelayExceptions = true }, type.ElementType); } return (Convert, type.ElementType); } @@ -514,7 +514,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms case IMethod method: return (targetVariable => new Call(method) { Arguments = { new LdLoc(targetVariable), value() } }, method.ReturnType); case IField field: - return (targetVariable => new StObj(new LdFlda(new LdLoc(targetVariable), (IField)member), value(), member.ReturnType), field.ReturnType); + return (targetVariable => new StObj(new LdFlda(new LdLoc(targetVariable), (IField)member) { DelayExceptions = true }, value(), member.ReturnType), field.ReturnType); } return (null, SpecialType.UnknownType); } @@ -782,9 +782,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms } else { var target = targetConverter(); if (member.DeclaringType.IsReferenceType == true) { - inst = new LdFlda(target, (IField)member); + inst = new LdFlda(target, (IField)member) { DelayExceptions = true }; } else { - inst = new LdFlda(new AddressOf(target, member.DeclaringType), (IField)member); + inst = new LdFlda(new AddressOf(target, member.DeclaringType), (IField)member) { DelayExceptions = true }; } } if (!(typeHint.SkipModifiers() is ByReferenceType && !member.ReturnType.IsByRefLike)) { @@ -997,7 +997,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms Block initializer = new Block(BlockKind.ArrayInitializer); initializer.Instructions.Add(new StLoc(variable, new NewArr(type, new LdcI4(convertedArguments.Length)))); for (int i = 0; i < convertedArguments.Length; i++) { - initializer.Instructions.Add(new StObj(new LdElema(type, new LdLoc(variable), new LdcI4(i)), convertedArguments[i](), type)); + initializer.Instructions.Add(new StObj(new LdElema(type, new LdLoc(variable), new LdcI4(i)) { DelayExceptions = true }, convertedArguments[i](), type)); } initializer.FinalInstruction = new LdLoc(variable); return initializer; diff --git a/ILSpy/TextView/ILAsm-Mode.xshd b/ILSpy/TextView/ILAsm-Mode.xshd index 12a5631ea..f24c9f6c8 100644 --- a/ILSpy/TextView/ILAsm-Mode.xshd +++ b/ILSpy/TextView/ILAsm-Mode.xshd @@ -149,6 +149,7 @@ newarr ldlen ldelema + ldelem ldelem.i1 ldelem.u1 ldelem.i2 @@ -160,6 +161,7 @@ ldelem.r4 ldelem.r8 ldelem.ref + stelem stelem.i stelem.i1 stelem.i2