Fix #1333: Ensure we convert to the correct type when calling instance methods on value types

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

@@ -22,6 +22,7 @@ extends [mscorlib]System.Object
 
     call void Program::InlineAssignByte()
     call void Program::Int32OrNativeTests()
+	call void Program::ByRefInstanceCallWithTypeMismatchTests()
 
     ret
   } // end of method Main
@@ -284,4 +285,42 @@ pointless:
     call void [mscorlib]System.Console::WriteLine(object)
     ret
   }
+
+  .method public static void ByRefInstanceCallWithTypeMismatchTests()
+  {
+    ldstr "ByRefInstanceCallWithTypeMismatch(0) = {0}"
+    ldc.i4.0
+    call string Program::ByRefInstanceCallWithTypeMismatch(int32)
+    call void [mscorlib]System.Console::WriteLine(string, object)
+
+    ldstr "ByRefInstanceCallWithTypeMismatch(1) = {0}"
+    ldc.i4.1
+    call string Program::ByRefInstanceCallWithTypeMismatch(int32)
+    call void [mscorlib]System.Console::WriteLine(string, object)
+
+    ldstr "Issue1333() = {0}"
+    call string Program::Issue1333()
+    call void [mscorlib]System.Console::WriteLine(string, object)
+
+    ret
+  }
+  
+  .method public hidebysig static string ByRefInstanceCallWithTypeMismatch(int32 val) cil managed
+  {
+    ldarga val
+    constrained. [mscorlib]System.Boolean
+    callvirt instance string [mscorlib]System.Object::ToString()
+    ret
+  }
+  
+  .method public hidebysig static string Issue1333() cil managed
+  {
+    .locals (bool)
+    ldc.i4.0
+    stloc.0
+    ldloca.s 0
+    constrained. [mscorlib]System.Boolean
+    callvirt instance string [mscorlib]System.Object::ToString()
+    ret
+  }
 }

ICSharpCode.Decompiler/CSharp/CallBuilder.cs

@@ -1185,6 +1185,7 @@ namespace ICSharpCode.Decompiler.CSharp
 
 		TranslatedExpression HandleDelegateConstruction(CallInstruction inst)
 		{
+			ILInstruction thisArg = inst.Arguments[0];
 			ILInstruction func = inst.Arguments[1];
 			IMethod method;
 			switch (func.OpCode) {
@@ -1203,16 +1204,27 @@ namespace ICSharpCode.Decompiler.CSharp
 			bool requireTarget;
 			if (method.IsExtensionMethod && invokeMethod != null && method.Parameters.Count - 1 == invokeMethod.Parameters.Count) {
 				targetType = method.Parameters[0].Type;
-				target = expressionBuilder.Translate(inst.Arguments[0], targetType);
-				target = ExpressionBuilder.UnwrapBoxingConversion(target);
+				if (targetType.Kind == TypeKind.ByReference && thisArg is Box thisArgBox) {
+					targetType = ((ByReferenceType)targetType).ElementType;
+					thisArg = thisArgBox.Argument;
+				}
+				target = expressionBuilder.Translate(thisArg, targetType);
 				requireTarget = true;
 			} else {
 				targetType = method.DeclaringType;
-				target = expressionBuilder.TranslateTarget(inst.Arguments[0],
+				if (targetType.IsReferenceType == false && thisArg is Box thisArgBox) {
+					// Normal struct instance method calls (which TranslateTarget is meant for) expect a 'ref T',
+					// but delegate construction uses a 'box T'.
+					if (thisArgBox.Argument is LdObj ldobj) {
+						thisArg = ldobj.Target;
+					} else {
+						thisArg = new AddressOf(thisArgBox.Argument);
+					}
+				}
+				target = expressionBuilder.TranslateTarget(thisArg,
 					nonVirtualInvocation: func.OpCode == OpCode.LdFtn,
 					memberStatic: method.IsStatic,
 					memberDeclaringType: method.DeclaringType);
-				target = ExpressionBuilder.UnwrapBoxingConversion(target);
 				requireTarget = expressionBuilder.HidesVariableWithName(method.Name)
 					|| (method.IsStatic ? !expressionBuilder.IsCurrentOrContainingType(method.DeclaringTypeDefinition) : !(target.Expression is ThisReferenceExpression));
 			}

ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@@ -1669,7 +1669,7 @@ namespace ICSharpCode.Decompiler.CSharp
 		}
 		
 		internal TranslatedExpression TranslateTarget(ILInstruction target, bool nonVirtualInvocation,
-			bool memberStatic, IType memberDeclaringType)
+			bool memberStatic, IType memberDeclaringType, bool unwrapBoxingConversion = false)
 		{
 			// If references are missing member.IsStatic might not be set correctly.
 			// Additionally check target for null, in order to avoid a crash.
@@ -1680,9 +1680,15 @@ namespace ICSharpCode.Decompiler.CSharp
 						.WithRR(new ThisResolveResult(memberDeclaringType, nonVirtualInvocation));
 				} else {
 					var translatedTarget = Translate(target, memberDeclaringType);
-					if (CallInstruction.ExpectedTypeForThisPointer(memberDeclaringType) == StackType.Ref && translatedTarget.Type.GetStackType().IsIntegerType()) {
-						// when accessing members on value types, ensure we use a reference and not a pointer
-						translatedTarget = translatedTarget.ConvertTo(new ByReferenceType(memberDeclaringType), this);
+					if (unwrapBoxingConversion) {
+						translatedTarget = ExpressionBuilder.UnwrapBoxingConversion(translatedTarget);
+					}
+					if (CallInstruction.ExpectedTypeForThisPointer(memberDeclaringType) == StackType.Ref) {
+						// When accessing members on value types, ensure we use a reference of the correct type,
+						// and not a pointer or a reference to a different type (issue #1333)
+						if (!(translatedTarget.Type is ByReferenceType brt && NormalizeTypeVisitor.TypeErasure.EquivalentTypes(brt.ElementType, memberDeclaringType))) {
+							translatedTarget = translatedTarget.ConvertTo(new ByReferenceType(memberDeclaringType), this);
+						}
 					}
 					if (translatedTarget.Expression is DirectionExpression) {
 						// (ref x).member => x.member
@@ -2386,8 +2392,14 @@ namespace ICSharpCode.Decompiler.CSharp
 		
 		protected internal override TranslatedExpression VisitAddressOf(AddressOf inst, TranslationContext context)
 		{
+			IType targetTypeHint = null;
+			if (context.TypeHint is ByReferenceType brt) {
+				targetTypeHint = brt.ElementType;
+			} else if (context.TypeHint is PointerType pt) {
+				targetTypeHint = pt.ElementType;
+			}
 			// HACK: this is only correct if the argument is an R-value; otherwise we're missing the copy to the temporary
-			var value = Translate(inst.Value);
+			var value = Translate(inst.Value, targetTypeHint);
 			return new DirectionExpression(FieldDirection.Ref, value)
 				.WithILInstruction(inst)
 				.WithRR(new ByReferenceResolveResult(value.ResolveResult, false));

ICSharpCode.Decompiler/CSharp/TranslatedExpression.cs

@@ -348,9 +348,18 @@ namespace ICSharpCode.Decompiler.CSharp
 				return pointerExpr.ConvertTo(targetType, expressionBuilder);
 			}
 			if (targetType.Kind == TypeKind.ByReference) {
+				var elementType = ((ByReferenceType)targetType).ElementType;
+				if (this.Expression is DirectionExpression thisDir && this.ILInstructions.Any(i => i.OpCode == OpCode.AddressOf)
+					&& thisDir.Expression.GetResolveResult()?.Type.GetStackType() == elementType.GetStackType()) {
+					// When converting a reference to a temporary to a different type,
+					// apply the cast to the temporary instead.
+					var convertedTemp = this.UnwrapChild(thisDir.Expression).ConvertTo(elementType, expressionBuilder, checkForOverflow);
+					return new DirectionExpression(FieldDirection.Ref, convertedTemp)
+						.WithILInstruction(this.ILInstructions)
+						.WithRR(new ByReferenceResolveResult(convertedTemp.ResolveResult, false));
+				}
 				// Convert from integer/pointer to reference.
 				// First, convert to the corresponding pointer type:
-				var elementType = ((ByReferenceType)targetType).ElementType;
 				var arg = this.ConvertTo(new PointerType(elementType), expressionBuilder, checkForOverflow);
 				Expression expr;
 				ResolveResult elementRR;