Merge remote-tracking branch 'origin/fix-2050'

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.

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<T>()
+		{
+			Console.WriteLine("GetValue");
+			return default(T);
+		}
+
+		int intField;
+
+		static void FieldAccessOrderOfEvaluation(NullableTests c)
+		{
+			Console.WriteLine("GetInt, then NRE:");
+			try {
+				c.intField = GetValue<int>();
+			} catch (Exception ex) {
+				Console.WriteLine(ex.Message);
+			}
+			Console.WriteLine("NRE before GetInt:");
+			try {
+#if CS60
+				ref int i = ref c.intField;
+				i = GetValue<int>();
+#endif
+			} catch (Exception ex) {
+				Console.WriteLine(ex.Message);
+			}
+		}
+
+		static T[] GetArray<T>()
+		{
+			Console.WriteLine("GetArray");
+			return null;
+		}
+
+		static int GetIndex()
+		{
+			Console.WriteLine("GetIndex");
+			return 0;
+		}
+
+		static void ArrayAccessOrderOfEvaluation()
+		{
+			Console.WriteLine("GetArray direct:");
+			try {
+				GetArray<int>()[GetIndex()] = GetValue<int>();
+			} catch (Exception ex) {
+				Console.WriteLine(ex.Message);
+			}
+			Console.WriteLine("GetArray with ref:");
+			try {
+#if CS60
+				ref int elem = ref GetArray<int>()[GetIndex()];
+				elem = GetValue<int>();
+#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<TimeSpan>()[GetIndex()] = GetValue<TimeSpan>();
+			} catch (Exception ex) {
+				Console.WriteLine(ex.Message);
+			}
+		}
 	}
 }

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

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
@@ -1159,6 +1159,13 @@ protected override void Disconnected()
 		opCode.PerformMatchConditions.Add("IsReadOnly == o.IsReadOnly");
 	};
 	
+	static Action<OpCode> CustomInvariant(string code)
+	{
+		return opCode => {
+			opCode.Invariants.Add(code);
+		};
+	}
+
 	static Action<OpCode> Pattern = opCode => {
 		BaseClass("PatternInstruction")(opCode);
 		opCode.Namespace = "ICSharpCode.Decompiler.IL.Patterns";

ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs

@@ -43,4 +43,30 @@ namespace ICSharpCode.Decompiler.IL
 			}
 		}
 	}
+
+	public sealed partial class StObj
+	{
+		/// <summary>
+		/// 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
+		/// </summary>
+		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;
+			}
+		}
+	}
 }

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?

ICSharpCode.Decompiler/IL/Transforms/StatementTransform.cs

@@ -45,6 +45,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// <remarks>
 		/// 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.
 		/// </remarks>
 		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);

ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs

@@ -338,9 +338,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// <summary>
 		/// Handle simple case where RuntimeHelpers.InitializeArray is not used.
 		/// </summary>
-		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++) {
+			int step;
-				if (!block.Instructions[i].MatchStObj(out ILInstruction target, out ILInstruction value, out IType type))
+			while (i < block.Instructions.Count) {
+				InstructionCollection<ILInstruction> 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;
-				if (value.Descendants.OfType<IInstructionWithVariableOperand>().Any(inst => inst.Variable == store))
+					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 (!(target is LdElema ldelem && ldelem.Array.MatchLdLoc(store)))
+					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<IInstructionWithVariableOperand>().Any(inst => inst.Variable == 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)

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;

ILSpy/TextView/ILAsm-Mode.xshd

@@ -149,6 +149,7 @@
 			<Word>newarr</Word>
 			<Word>ldlen</Word>
 			<Word>ldelema</Word>
+			<Word>ldelem</Word>
 			<Word>ldelem.i1</Word>
 			<Word>ldelem.u1</Word>
 			<Word>ldelem.i2</Word>
@@ -160,6 +161,7 @@
 			<Word>ldelem.r4</Word>
 			<Word>ldelem.r8</Word>
 			<Word>ldelem.ref</Word>
+			<Word>stelem</Word>
 			<Word>stelem.i</Word>
 			<Word>stelem.i1</Word>
 			<Word>stelem.i2</Word>