Fix #938: add support for I4->I stack type adjustments.

With this fix we still only propagate type information along forward branches, so some of the issues in #901 remain.

ICSharpCode.Decompiler.Tests/TestCases/Correctness/StackTypes.il

@@ -21,7 +21,7 @@ extends [mscorlib]System.Object
     .entrypoint
 
     call void Program::InlineAssignByte()
-    //call void Program::Int32OrNativeTests()
+    call void Program::Int32OrNativeTests()
 
     ret
   } // end of method Main
@@ -48,7 +48,6 @@ pointless:
     ret
   }
 
-  /*
   .method public static void Int32OrNativeTests()
   {
     ldstr "Int32OrNative(0x7fffffff, false) = {0}"
@@ -79,6 +78,7 @@ pointless:
     box native int
     call void [mscorlib]System.Console::WriteLine(string, object)
 
+    /*
     ldstr "Int32OrNativeLoopStyle(0x7fffffff):"
     call void [mscorlib]System.Console::WriteLine(string)
     ldc.i4 0x7fffffff
@@ -100,6 +100,7 @@ pointless:
     call native int Program::Int32OrNativeDeadCode(int32)
     box native int
     call void [mscorlib]System.Console::WriteLine(string, object)
+    */
 
     ldc.i4 0x7fffffff
     call void Program::RunInt32OrNativeMultiUse(int32)
@@ -115,6 +116,26 @@ pointless:
   use_i4:
     ldarg.0
     br after_if
+  use_native_int:
+    ldarg.0
+    conv.u
+    br after_if
+  after_if:
+    ldc.i4.1
+    add
+    ret
+  }
+  
+  /*
+  .method public static native int Int32OrNativeReordered(int32 val, bool use_native)
+  {
+    // The spec is ambiguous whether the addition will be in 32-bits or native size.
+    // The microsoft runtime picks native size.
+    ldarg.1
+    brtrue use_native_int
+  use_i4:
+    ldarg.0
+    br after_if
   after_if:
     ldc.i4.1
     add
@@ -124,9 +145,11 @@ pointless:
     conv.u
     br after_if
   }
-
+  
   .method public static void Int32OrNativeLoopStyle(int32 val)
   {
+    // The spec is ambiguous whether the addition will be in 32-bits or native size.
+    // The microsoft runtime picks native size.
     .locals init (
       int32 i
     )
@@ -149,6 +172,8 @@ pointless:
 
   .method public static native int Int32OrNativeDeadCode(int32 val)
   {
+    // The spec is ambiguous whether the addition will be in 32-bits or native size.
+    // The microsoft runtime picks 32-bits.
   use_i4:
     ldarg.0
     br after_if
@@ -161,6 +186,7 @@ pointless:
     conv.u
     br after_if
   }
+  */
 
   .method public static void RunInt32OrNativeMultiUse(int32 val)
   {
@@ -229,8 +255,7 @@ pointless:
     add
     ret
   }
-  */
-  
+
   .method public static void Print(native int val)
   {
     ldarg.0

ICSharpCode.Decompiler/IL/BlockBuilder.cs

@@ -117,7 +117,7 @@ namespace ICSharpCode.Decompiler.IL
 			foreach (var inst in instructions) {
 				cancellationToken.ThrowIfCancellationRequested();
 				int start = inst.ILRange.Start;
-				if (currentBlock == null || incomingBranches[start]) {
+				if (currentBlock == null || (incomingBranches[start] && !IsStackAdjustment(inst))) {
 					// Finish up the previous block
 					FinalizeCurrentBlock(start, fallthrough: true);
 					// Leave nested containers if necessary
@@ -158,6 +158,11 @@ namespace ICSharpCode.Decompiler.IL
 			ConnectBranches(mainContainer, cancellationToken);
 		}
 
+		static bool IsStackAdjustment(ILInstruction inst)
+		{
+			return inst is StLoc stloc && stloc.IsStackAdjustment;
+		}
+
 		private void FinalizeCurrentBlock(int currentILOffset, bool fallthrough)
 		{
 			if (currentBlock == null)

ICSharpCode.Decompiler/IL/ILReader.cs

@@ -64,6 +64,7 @@ namespace ICSharpCode.Decompiler.IL
 		Dictionary<int, ImmutableStack<ILVariable>> stackByOffset;
 		Dictionary<Cil.ExceptionHandler, ILVariable> variableByExceptionHandler;
 		UnionFind<ILVariable> unionFind;
+		List<(ILVariable, ILVariable)> stackMismatchPairs;
 		IEnumerable<ILVariable> stackVariables;
 		
 		void Init(Cil.MethodBody body)
@@ -74,8 +75,9 @@ namespace ICSharpCode.Decompiler.IL
 			this.debugInfo = body.Method.DebugInformation;
 			this.currentInstruction = null;
 			this.nextInstructionIndex = 0;
-			this.currentStack = System.Collections.Immutable.ImmutableStack<ILVariable>.Empty;
+			this.currentStack = ImmutableStack<ILVariable>.Empty;
 			this.unionFind = new UnionFind<ILVariable>();
+			this.stackMismatchPairs = new List<(ILVariable, ILVariable)>();
 			this.methodReturnStackType = typeSystem.Resolve(body.Method.ReturnType).GetStackType();
 			InitParameterVariables();
 			this.localVariables = body.Variables.SelectArray(CreateILVariable);
@@ -84,7 +86,7 @@ namespace ICSharpCode.Decompiler.IL
 					v.HasInitialValue = true;
 				}
 			}
-			mainContainer = new BlockContainer(methodReturnStackType);
+			this.mainContainer = new BlockContainer(methodReturnStackType);
 			this.instructionBuilder = new List<ILInstruction>();
 			this.isBranchTarget = new BitArray(body.CodeSize);
 			this.stackByOffset = new Dictionary<int, ImmutableStack<ILVariable>>();
@@ -186,35 +188,87 @@ namespace ICSharpCode.Decompiler.IL
 			Warnings.Add(string.Format("IL_{0:x4}: {1}", currentInstruction.Offset, message));
 		}
 
-		void MergeStacks(ImmutableStack<ILVariable> a, ImmutableStack<ILVariable> b)
+		ImmutableStack<ILVariable> MergeStacks(ImmutableStack<ILVariable> a, ImmutableStack<ILVariable> b)
 		{
-			var enum1 = a.GetEnumerator();
-			var enum2 = b.GetEnumerator();
-			bool ok1 = enum1.MoveNext();
-			bool ok2 = enum2.MoveNext();
-			while (ok1 && ok2) {
-				if (enum1.Current.StackType != enum2.Current.StackType) {
-					Warn("Incompatible stack types: " + enum1.Current.StackType + " vs " + enum2.Current.StackType);
+			if (CheckStackCompatibleWithoutAdjustments(a, b)) {
+				// We only need to union the input variables, but can 
+				// otherwise re-use the existing stack.
+				ImmutableStack<ILVariable> output = a;
+				while (!a.IsEmpty && !b.IsEmpty) {
+					Debug.Assert(a.Peek().StackType == b.Peek().StackType);
+					unionFind.Merge(a.Peek(), b.Peek());
+					a = a.Pop();
+					b = b.Pop();
 				}
-				unionFind.Merge(enum1.Current, enum2.Current);
-				ok1 = enum1.MoveNext();
-				ok2 = enum2.MoveNext();
-			}
-			if (ok1 || ok2) {
+				return output;
+			} else if (a.Count() != b.Count()) {
+				// Let's not try to merge mismatched stacks.
 				Warn("Incompatible stack heights: " + a.Count() + " vs " + b.Count());
+				return a;
+			} else {
+				// The more complex case where the stacks don't match exactly.
+				var output = new List<ILVariable>();
+				while (!a.IsEmpty && !b.IsEmpty) {
+					var varA = a.Peek();
+					var varB = b.Peek();
+					if (varA.StackType == varB.StackType) {
+						unionFind.Merge(varA, varB);
+						output.Add(varA);
+					} else {
+						if (!IsValidTypeStackTypeMerge(varA.StackType, varB.StackType)) {
+							Warn("Incompatible stack types: " + varA.StackType + " vs " + varB.StackType);
+						}
+						if (varA.StackType > varB.StackType) {
+							output.Add(varA);
+							// every store to varB should also store to varA
+							stackMismatchPairs.Add((varB, varA));
+						} else {
+							output.Add(varB);
+							// every store to varA should also store to varB
+							stackMismatchPairs.Add((varA, varB));
+						}
+					}
+					a = a.Pop();
+					b = b.Pop();
+				}
+				// because we built up output by popping from the input stacks, we need to reverse it to get back the original order
+				output.Reverse();
+				return ImmutableStack.CreateRange(output);
 			}
 		}
 
+		static bool CheckStackCompatibleWithoutAdjustments(ImmutableStack<ILVariable> a, ImmutableStack<ILVariable> b)
+		{
+			while (!a.IsEmpty && !b.IsEmpty) {
+				if (a.Peek().StackType != b.Peek().StackType)
+					return false;
+				a = a.Pop();
+				b = b.Pop();
+			}
+			return a.IsEmpty && b.IsEmpty;
+		}
+
+		private bool IsValidTypeStackTypeMerge(StackType stackType1, StackType stackType2)
+		{
+			if (stackType1 == StackType.I && stackType2 == StackType.I4)
+				return true;
+			if (stackType1 == StackType.I4 && stackType2 == StackType.I)
+				return true;
+			// allow merging unknown type with any other type
+			return stackType1 == StackType.Unknown || stackType2 == StackType.Unknown;
+		}
+
 		void StoreStackForOffset(int offset, ImmutableStack<ILVariable> stack)
 		{
-			ImmutableStack<ILVariable> existing;
-			if (stackByOffset.TryGetValue(offset, out existing)) {
-				MergeStacks(existing, stack);
+			if (stackByOffset.TryGetValue(offset, out var existing)) {
+				var newStack = MergeStacks(existing, stack);
+				if (newStack != existing)
+					stackByOffset[offset] = newStack;
 			} else {
 				stackByOffset.Add(offset, stack);
 			}
 		}
-
+		
 		void ReadInstructions(CancellationToken cancellationToken)
 		{
 			// Fill isBranchTarget and branchStackDict based on exception handlers
@@ -272,6 +326,44 @@ namespace ICSharpCode.Decompiler.IL
 				instructionBuilder[i] = instructionBuilder[i].AcceptVisitor(visitor);
 			}
 			stackVariables = visitor.variables;
+			InsertStackAdjustments();
+		}
+
+		void InsertStackAdjustments()
+		{
+			if (stackMismatchPairs.Count == 0)
+				return;
+			var dict = new MultiDictionary<ILVariable, ILVariable>();
+			foreach (var (origA, origB) in stackMismatchPairs) {
+				var a = unionFind.Find(origA);
+				var b = unionFind.Find(origB);
+				Debug.Assert(a.StackType < b.StackType);
+				// For every store to a, insert a converting store to b.
+				if (!dict[a].Contains(b))
+					dict.Add(a, b);
+			}
+			var newInstructions = new List<ILInstruction>();
+			foreach (var inst in instructionBuilder) {
+				newInstructions.Add(inst);
+				if (inst is StLoc store) {
+					foreach (var additionalVar in dict[store.Variable]) {
+						ILInstruction value = new LdLoc(store.Variable);
+						switch (additionalVar.StackType) {
+							case StackType.I:
+								value = new Conv(value, PrimitiveType.I, false, Sign.None);
+								break;
+							case StackType.I8:
+								value = new Conv(value, PrimitiveType.I8, false, Sign.None);
+								break;
+						}
+						newInstructions.Add(new StLoc(additionalVar, value) {
+							IsStackAdjustment = true,
+							ILRange = inst.ILRange
+						});
+					}
+				}
+			}
+			instructionBuilder = newInstructions;
 		}
 
 		/// <summary>
@@ -282,6 +374,12 @@ namespace ICSharpCode.Decompiler.IL
 			Init(body);
 			ReadInstructions(cancellationToken);
 			foreach (var inst in instructionBuilder) {
+				if (inst is StLoc stloc && stloc.IsStackAdjustment) {
+					output.Write("          ");
+					inst.WriteTo(output, new ILAstWritingOptions());
+					output.WriteLine();
+					continue;
+				}
 				output.Write("   [");
 				bool isFirstElement = true;
 				foreach (var element in stackByOffset[inst.ILRange.Start]) {

ICSharpCode.Decompiler/IL/Instructions.cs

@@ -2317,6 +2317,8 @@ namespace ICSharpCode.Decompiler.IL
 			clone.Value = this.value.Clone();
 			return clone;
 		}
+		/// <summary>If true, this stloc represents a stack type adjustment. This field is only used in ILReader and BlockBuilder, and should be ignored by ILAst transforms.</summary>
+		internal bool IsStackAdjustment;
 		public override StackType ResultType { get { return variable.StackType; } }
 		protected override InstructionFlags ComputeFlags()
 		{

ICSharpCode.Decompiler/IL/Instructions.tt

@@ -153,6 +153,9 @@
 		new OpCode("stloc", "Stores a value into a local variable. (IL: starg/stloc)" + Environment.NewLine
 				+ "Evaluates to the value that was stored (for byte/short variables: evaluates to the truncated value, sign/zero extended back to I4 based on variable.Type.GetSign())",
 			CustomClassName("StLoc"), HasVariableOperand("Store"), CustomArguments("value"),
+			AdditionalMember(
+				"/// <summary>If true, this stloc represents a stack type adjustment. This field is only used in ILReader and BlockBuilder, and should be ignored by ILAst transforms.</summary>" + Environment.NewLine
+				+ "internal bool IsStackAdjustment;"),
 			ResultType("variable.StackType")),
 		new OpCode("addressof", "Stores the value into an anonymous temporary variable, and returns the address of that variable.",
 			CustomClassName("AddressOf"), CustomArguments("value"), ResultType("Ref")),
@@ -571,6 +574,13 @@ namespace ICSharpCode.Decompiler.IL
 		};
 	}
 
+	static Action<OpCode> AdditionalMember(string declaration)
+	{
+		return opCode => {
+			opCode.Members.Add(declaration);
+		};
+	}
+
 	// ResultType trait: the instruction has the specified result type.
 	static Action<OpCode> ResultType(string type)
 	{

ICSharpCode.Decompiler/IL/StackType.cs

@@ -24,6 +24,15 @@ namespace ICSharpCode.Decompiler.IL
 	/// </summary>
 	public enum StackType : byte
 	{
+		// Note: the numeric of these enum members is relevant for ILReader.MergeStacks:
+		// when two branches meet where a stack slot has different types, the type after
+		// the branch is the one with the higher numeric value.
+
+		/// <summary>
+		/// The stack type is unknown; for example a call returning an unknown type
+		/// because an assembly reference isn't loaded.
+		/// Can also occur with invalid IL.
+		/// </summary>
 		Unknown,
 		/// <summary>32-bit integer</summary>
 		/// <remarks>
@@ -33,23 +42,24 @@ namespace ICSharpCode.Decompiler.IL
 		/// and any enums with one of the above as underlying type.
 		/// </remarks>
 		I4,
+		/// <summary>native-size integer, or unmanaged pointer</summary>
+		/// <remarks>
+		/// Used for C# <c>IntPtr</c>, <c>UIntPtr</c> and any native pointer types (<c>void*</c> etc.)
+		/// Also used for IL function pointer types.
+		/// </remarks>
+		I,
 		/// <summary>64-bit integer</summary>
 		/// <remarks>
 		/// Used for C# <c>long</c>, <c>ulong</c>,
 		/// and any enums with one of the above as underlying type.
 		/// </remarks>
 		I8,
-		/// <summary>native-size integer, or unmanaged pointer</summary>
-		/// <remarks>
-		/// Used for C# <c>IntPtr</c>, <c>UIntPtr</c> and any native pointer types (<c>void*</c> etc.)
-		/// </remarks>
-		I,
 		/// <summary>Floating point number</summary>
 		/// <remarks>
 		/// Used for C# <c>float</c> and <c>double</c>.
 		/// </remarks>
 		F,
-		/// <summary>Another stack type. Includes objects, value types, function pointers, ...</summary>
+		/// <summary>Another stack type. Includes objects, value types, ...</summary>
 		O,
 		/// <summary>A managed pointer</summary>
 		Ref,