Fix #1021: Compound assignments on pointer types.

ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs

@@ -370,5 +370,13 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 			Data* ptr = &data;
 			this.ConvertIntToFloat(ptr->Position.GetHashCode());
 		}
+
+		private unsafe static void Issue1021(ref byte* bytePtr, ref short* shortPtr)
+		{
+			bytePtr += 4;
+			shortPtr += 2;
+			bytePtr -= 4;
+			shortPtr = (short*)((byte*)shortPtr - 3);
+		}
 	}
 }

ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.il

@@ -1398,6 +1398,43 @@
     IL_0025:  ret
   } // end of method UnsafeCode::Issue990
 
+  .method private hidebysig static void  Issue1021(uint8*& bytePtr,
+                                                   int16*& shortPtr) cil managed
+  {
+    // Code size       30 (0x1e)
+    .maxstack  8
+    IL_0000:  nop
+    IL_0001:  ldarg.0
+    IL_0002:  dup
+    IL_0003:  ldind.i
+    IL_0004:  ldc.i4.4
+    IL_0005:  conv.i
+    IL_0006:  add
+    IL_0007:  stind.i
+    IL_0008:  ldarg.1
+    IL_0009:  dup
+    IL_000a:  ldind.i
+    IL_000b:  ldc.i4.4
+    IL_000c:  conv.i
+    IL_000d:  add
+    IL_000e:  stind.i
+    IL_000f:  ldarg.0
+    IL_0010:  dup
+    IL_0011:  ldind.i
+    IL_0012:  ldc.i4.4
+    IL_0013:  conv.i
+    IL_0014:  sub
+    IL_0015:  stind.i
+    IL_0016:  ldarg.1
+    IL_0017:  ldarg.1
+    IL_0018:  ldind.i
+    IL_0019:  ldc.i4.3
+    IL_001a:  conv.i
+    IL_001b:  sub
+    IL_001c:  stind.i
+    IL_001d:  ret
+  } // end of method UnsafeCode::Issue1021
+
   .property instance int32* NullPointer()
   {
     .get instance int32* ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::get_NullPointer()
@@ -1408,4 +1445,4 @@
 // =============================================================
 
 // *********** DISASSEMBLY COMPLETE ***********************
-// WARNING: Created Win32 resource file ../../../TestCases/Pretty\UnsafeCode.res
+// WARNING: Created Win32 resource file C:\work\ILSpy\ICSharpCode.Decompiler.Tests\bin\Debug\net46\../../../TestCases/Pretty\UnsafeCode.res

ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.opt.il

@@ -1119,6 +1119,42 @@
     IL_0024:  ret
   } // end of method UnsafeCode::Issue990
 
+  .method private hidebysig static void  Issue1021(uint8*& bytePtr,
+                                                   int16*& shortPtr) cil managed
+  {
+    // Code size       29 (0x1d)
+    .maxstack  8
+    IL_0000:  ldarg.0
+    IL_0001:  dup
+    IL_0002:  ldind.i
+    IL_0003:  ldc.i4.4
+    IL_0004:  conv.i
+    IL_0005:  add
+    IL_0006:  stind.i
+    IL_0007:  ldarg.1
+    IL_0008:  dup
+    IL_0009:  ldind.i
+    IL_000a:  ldc.i4.4
+    IL_000b:  conv.i
+    IL_000c:  add
+    IL_000d:  stind.i
+    IL_000e:  ldarg.0
+    IL_000f:  dup
+    IL_0010:  ldind.i
+    IL_0011:  ldc.i4.4
+    IL_0012:  conv.i
+    IL_0013:  sub
+    IL_0014:  stind.i
+    IL_0015:  ldarg.1
+    IL_0016:  ldarg.1
+    IL_0017:  ldind.i
+    IL_0018:  ldc.i4.3
+    IL_0019:  conv.i
+    IL_001a:  sub
+    IL_001b:  stind.i
+    IL_001c:  ret
+  } // end of method UnsafeCode::Issue1021
+
   .property instance int32* NullPointer()
   {
     .get instance int32* ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::get_NullPointer()
@@ -1129,4 +1165,4 @@
 // =============================================================
 
 // *********** DISASSEMBLY COMPLETE ***********************
-// WARNING: Created Win32 resource file ../../../TestCases/Pretty\UnsafeCode.opt.res
+// WARNING: Created Win32 resource file C:\work\ILSpy\ICSharpCode.Decompiler.Tests\bin\Debug\net46\../../../TestCases/Pretty\UnsafeCode.opt.res

ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.opt.roslyn.il

@@ -1125,6 +1125,41 @@
     IL_0024:  ret
   } // end of method UnsafeCode::Issue990
 
+  .method private hidebysig static void  Issue1021(uint8*& bytePtr,
+                                                   int16*& shortPtr) cil managed
+  {
+    // Code size       28 (0x1c)
+    .maxstack  8
+    IL_0000:  ldarg.0
+    IL_0001:  ldarg.0
+    IL_0002:  ldind.i
+    IL_0003:  ldc.i4.4
+    IL_0004:  add
+    IL_0005:  stind.i
+    IL_0006:  ldarg.1
+    IL_0007:  ldarg.1
+    IL_0008:  ldind.i
+    IL_0009:  ldc.i4.2
+    IL_000a:  conv.i
+    IL_000b:  ldc.i4.2
+    IL_000c:  mul
+    IL_000d:  add
+    IL_000e:  stind.i
+    IL_000f:  ldarg.0
+    IL_0010:  ldarg.0
+    IL_0011:  ldind.i
+    IL_0012:  ldc.i4.4
+    IL_0013:  sub
+    IL_0014:  stind.i
+    IL_0015:  ldarg.1
+    IL_0016:  ldarg.1
+    IL_0017:  ldind.i
+    IL_0018:  ldc.i4.3
+    IL_0019:  sub
+    IL_001a:  stind.i
+    IL_001b:  ret
+  } // end of method UnsafeCode::Issue1021
+
   .property instance int32* NullPointer()
   {
     .get instance int32* ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::get_NullPointer()

ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.roslyn.il

@@ -1402,6 +1402,42 @@
     IL_0025:  ret
   } // end of method UnsafeCode::Issue990
 
+  .method private hidebysig static void  Issue1021(uint8*& bytePtr,
+                                                   int16*& shortPtr) cil managed
+  {
+    // Code size       29 (0x1d)
+    .maxstack  8
+    IL_0000:  nop
+    IL_0001:  ldarg.0
+    IL_0002:  ldarg.0
+    IL_0003:  ldind.i
+    IL_0004:  ldc.i4.4
+    IL_0005:  add
+    IL_0006:  stind.i
+    IL_0007:  ldarg.1
+    IL_0008:  ldarg.1
+    IL_0009:  ldind.i
+    IL_000a:  ldc.i4.2
+    IL_000b:  conv.i
+    IL_000c:  ldc.i4.2
+    IL_000d:  mul
+    IL_000e:  add
+    IL_000f:  stind.i
+    IL_0010:  ldarg.0
+    IL_0011:  ldarg.0
+    IL_0012:  ldind.i
+    IL_0013:  ldc.i4.4
+    IL_0014:  sub
+    IL_0015:  stind.i
+    IL_0016:  ldarg.1
+    IL_0017:  ldarg.1
+    IL_0018:  ldind.i
+    IL_0019:  ldc.i4.3
+    IL_001a:  sub
+    IL_001b:  stind.i
+    IL_001c:  ret
+  } // end of method UnsafeCode::Issue1021
+
   .property instance int32* NullPointer()
   {
     .get instance int32* ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::get_NullPointer()

ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@@ -793,13 +793,6 @@ namespace ICSharpCode.Decompiler.CSharp
 			}
 			TranslatedExpression offsetExpr = GetPointerArithmeticOffset(byteOffsetInst, byteOffsetExpr, pointerType, inst.CheckForOverflow)
 				?? FallBackToBytePointer();
-			if (!offsetExpr.Type.IsCSharpPrimitiveIntegerType()) {
-				// pointer arithmetic accepts all primitive integer types, but no enums etc.
-				StackType targetType = offsetExpr.Type.GetStackType() == StackType.I4 ? StackType.I4 : StackType.I8;
-				offsetExpr = offsetExpr.ConvertTo(
-					compilation.FindType(targetType.ToKnownTypeCode(offsetExpr.Type.GetSign())),
-					this);
-			}
 
 			if (left.Type.Kind == TypeKind.Pointer) {
 				Debug.Assert(inst.Operator == BinaryNumericOperator.Add || inst.Operator == BinaryNumericOperator.Sub);
@@ -821,50 +814,36 @@ namespace ICSharpCode.Decompiler.CSharp
 			TranslatedExpression FallBackToBytePointer()
 			{
 				pointerType = new PointerType(compilation.FindType(KnownTypeCode.Byte));
-				return byteOffsetExpr;
+				return EnsureIntegerType(byteOffsetExpr);
+			}
+		}
+
+		TranslatedExpression EnsureIntegerType(TranslatedExpression expr)
+		{
+			if (!expr.Type.IsCSharpPrimitiveIntegerType()) {
+				// pointer arithmetic accepts all primitive integer types, but no enums etc.
+				StackType targetType = expr.Type.GetStackType() == StackType.I4 ? StackType.I4 : StackType.I8;
+				expr = expr.ConvertTo(
+					compilation.FindType(targetType.ToKnownTypeCode(expr.Type.GetSign())),
+					this);
 			}
+			return expr;
 		}
 
 		TranslatedExpression? GetPointerArithmeticOffset(ILInstruction byteOffsetInst, TranslatedExpression byteOffsetExpr,
 			PointerType pointerType, bool checkForOverflow, bool unwrapZeroExtension = false)
 		{
-			if (byteOffsetInst is Conv conv && conv.InputType == StackType.I8 && conv.ResultType == StackType.I) {
+			var countOffsetInst = PointerArithmeticOffset.Detect(byteOffsetInst, pointerType,
-				byteOffsetInst = conv.Argument;
+				checkForOverflow: checkForOverflow,
+				unwrapZeroExtension: unwrapZeroExtension);
+			if (countOffsetInst == null) {
+				return null;
 			}
-			int? elementSize = ComputeSizeOf(pointerType.ElementType);
+			if (countOffsetInst == byteOffsetInst) {
-			if (elementSize == 1) {
+				return EnsureIntegerType(byteOffsetExpr);
-				return byteOffsetExpr;
+			} else {
-			} else if (byteOffsetInst is BinaryNumericInstruction mul && mul.Operator == BinaryNumericOperator.Mul) {
+				return EnsureIntegerType(Translate(countOffsetInst));
-				if (mul.CheckForOverflow != checkForOverflow)
-					return null;
-				if (mul.IsLifted)
-					return null;
-				if (elementSize > 0 && mul.Right.MatchLdcI(elementSize.Value)
-					|| mul.Right.UnwrapConv(ConversionKind.SignExtend) is SizeOf sizeOf && sizeOf.Type.Equals(pointerType.ElementType))
-				{
-					var countOffsetInst = mul.Left;
-					if (unwrapZeroExtension) {
-						countOffsetInst = countOffsetInst.UnwrapConv(ConversionKind.ZeroExtend);
-					}
-					return Translate(countOffsetInst);
-				}
-			} else if (byteOffsetInst.UnwrapConv(ConversionKind.SignExtend) is SizeOf sizeOf && sizeOf.Type.Equals(pointerType.ElementType)) {
-				return new PrimitiveExpression(1)
-					.WithILInstruction(byteOffsetInst)
-					.WithRR(new ConstantResolveResult(compilation.FindType(KnownTypeCode.Int32), 1));
-			} else if (byteOffsetInst.MatchLdcI(out long val)) {
-				// If the offset is a constant, it's possible that the compiler
-				// constant-folded the multiplication.
-				if (elementSize > 0 && (val % elementSize == 0) && val > 0) {
-					val /= elementSize.Value;
-					if (val <= int.MaxValue) {
-						return new PrimitiveExpression((int)val)
-							.WithILInstruction(byteOffsetInst)
-							.WithRR(new ConstantResolveResult(compilation.FindType(KnownTypeCode.Int32), val));
-					}
-				}
 			}
-			return null;
 		}
 
 		/// <summary>
@@ -928,21 +907,12 @@ namespace ICSharpCode.Decompiler.CSharp
 					if (elementType != null)
 						return elementType.Equals(pt.ElementType);
 					else if (elementSize > 0)
-						return ComputeSizeOf(pt.ElementType) == elementSize;
+						return PointerArithmeticOffset.ComputeSizeOf(pt.ElementType) == elementSize;
 				}
 				return false;
 			}
 		}
-
+		
-		int? ComputeSizeOf(IType type)
-		{
-			var rr = resolver.ResolveSizeOf(type);
-			if (rr.IsCompileTimeConstant && rr.ConstantValue is int size)
-				return size;
-			else
-				return null;
-		}
-
 		TranslatedExpression HandleBinaryNumeric(BinaryNumericInstruction inst, BinaryOperatorType op)
 		{
 			var resolverWithOverflowCheck = resolver.WithCheckForOverflow(inst.CheckForOverflow);
@@ -1125,6 +1095,7 @@ namespace ICSharpCode.Decompiler.CSharp
 			var value = Translate(inst.Value);
 			value = PrepareArithmeticArgument(value, inst.RightInputType, inst.Sign, inst.IsLifted);
 			
+
 			TranslatedExpression resultExpr;
 			if (inst.CompoundAssignmentType == CompoundAssignmentType.EvaluatesToOldValue) {
 				Debug.Assert(op == AssignmentOperatorType.Add || op == AssignmentOperatorType.Subtract);
@@ -1144,14 +1115,22 @@ namespace ICSharpCode.Decompiler.CSharp
 			} else {
 				switch (op) {
 					case AssignmentOperatorType.Add:
-					case AssignmentOperatorType.Subtract: {
+					case AssignmentOperatorType.Subtract:
+						if (target.Type.Kind == TypeKind.Pointer) {
+							var pao = GetPointerArithmeticOffset(inst.Value, value, (PointerType)target.Type, inst.CheckForOverflow);
+							if (pao != null) {
+								value = pao.Value;
+							} else { 
+								value.Expression.AddChild(new Comment("ILSpy Error: GetPointerArithmeticOffset() failed", CommentType.MultiLine), Roles.Comment);
+							}
+						} else {
 							IType targetType = NullableType.GetUnderlyingType(target.Type).GetEnumUnderlyingType();
 							if (NullableType.IsNullable(value.Type)) {
 								targetType = NullableType.Create(compilation, targetType);
 							}
 							value = value.ConvertTo(targetType, this, inst.CheckForOverflow, allowImplicitConversion: true);
-							break;
 						}
+						break;
 					case AssignmentOperatorType.Multiply:
 					case AssignmentOperatorType.Divide:
 					case AssignmentOperatorType.Modulus:

ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj

@@ -286,6 +286,7 @@
     <Compile Include="DotNetCore\LightJson\Serialization\TextScanner.cs" />
     <Compile Include="DotNetCore\UniversalAssemblyResolver.cs" />
     <Compile Include="DotNetCore\UnresolvedAssemblyNameReference.cs" />
+    <Compile Include="IL\PointerArithmeticOffset.cs" />
     <Compile Include="IL\ControlFlow\AwaitInCatchTransform.cs" />
     <Compile Include="IL\ILAstWritingOptions.cs" />
     <Compile Include="IL\Instructions\LdFlda.cs" />

ICSharpCode.Decompiler/IL/Instructions/CompoundAssignmentInstruction.cs

@@ -96,6 +96,19 @@ namespace ICSharpCode.Decompiler.IL
 					default:
 						return false; // operator not supported on enum types
 				}
+			} else if (type.Kind == TypeKind.Pointer) {
+				switch (binary.Operator) {
+					case BinaryNumericOperator.Add:
+					case BinaryNumericOperator.Sub:
+						// ensure that the byte offset is a multiple of the pointer size
+						return PointerArithmeticOffset.Detect(
+							binary.Right,
+							(PointerType)type,
+							checkForOverflow: binary.CheckForOverflow
+						) != null;
+					default:
+						return false; // operator not supported on pointer types
+				}
 			}
 			if (binary.Sign != Sign.None) {
 				if (type.GetSign() != binary.Sign)

ICSharpCode.Decompiler/IL/PointerArithmeticOffset.cs

@@ -0,0 +1,84 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+using ICSharpCode.Decompiler.TypeSystem;
+
+namespace ICSharpCode.Decompiler.IL
+{
+	/// <summary>
+	/// Analyses the RHS of a 'ptr + int' or 'ptr - int' operation.
+	/// </summary>
+	struct PointerArithmeticOffset
+	{
+		/// <summary>
+		/// Given an instruction that computes a pointer arithmetic offset in bytes,
+		/// returns an instruction that computes the same offset in number of elements.
+		/// 
+		/// Returns null if no such instruction can be found.
+		/// </summary>
+		/// <param name="byteOffsetInst">Input instruction.</param>
+		/// <param name="pointerType">The pointer type.</param>
+		/// <param name="checkForOverflow">Whether the pointer arithmetic operation checks for overflow.</param>
+		/// <param name="unwrapZeroExtension">Whether to allow zero extensions in the mul argument.</param>
+		public static ILInstruction Detect(ILInstruction byteOffsetInst, PointerType pointerType,
+			bool checkForOverflow,
+			bool unwrapZeroExtension = false)
+		{
+			if (byteOffsetInst is Conv conv && conv.InputType == StackType.I8 && conv.ResultType == StackType.I) {
+				byteOffsetInst = conv.Argument;
+			}
+			int? elementSize = ComputeSizeOf(pointerType.ElementType);
+			if (elementSize == 1) {
+				return byteOffsetInst;
+			} else if (byteOffsetInst is BinaryNumericInstruction mul && mul.Operator == BinaryNumericOperator.Mul) {
+				if (mul.IsLifted)
+					return null;
+				if (mul.CheckForOverflow != checkForOverflow)
+					return null;
+				if (elementSize > 0 && mul.Right.MatchLdcI(elementSize.Value)
+					|| mul.Right.UnwrapConv(ConversionKind.SignExtend) is SizeOf sizeOf && sizeOf.Type.Equals(pointerType.ElementType)) {
+					var countOffsetInst = mul.Left;
+					if (unwrapZeroExtension) {
+						countOffsetInst = countOffsetInst.UnwrapConv(ConversionKind.ZeroExtend);
+					}
+					return countOffsetInst;
+				}
+			} else if (byteOffsetInst.UnwrapConv(ConversionKind.SignExtend) is SizeOf sizeOf && sizeOf.Type.Equals(pointerType.ElementType)) {
+				return new LdcI4(1) { ILRange = byteOffsetInst.ILRange };
+			} else if (byteOffsetInst.MatchLdcI(out long val)) {
+				// If the offset is a constant, it's possible that the compiler
+				// constant-folded the multiplication.
+				if (elementSize > 0 && (val % elementSize == 0) && val > 0) {
+					val /= elementSize.Value;
+					if (val <= int.MaxValue) {
+						return new LdcI4((int)val) { ILRange = byteOffsetInst.ILRange };
+					}
+				}
+			}
+			return null;
+		}
+
+		public static int? ComputeSizeOf(IType type)
+		{
+			switch (type.GetEnumUnderlyingType().GetDefinition()?.KnownTypeCode) {
+				case KnownTypeCode.Boolean:
+				case KnownTypeCode.SByte:
+				case KnownTypeCode.Byte:
+					return 1;
+				case KnownTypeCode.Char:
+				case KnownTypeCode.Int16:
+				case KnownTypeCode.UInt16:
+					return 2;
+				case KnownTypeCode.Int32:
+				case KnownTypeCode.UInt32:
+				case KnownTypeCode.Single:
+					return 4;
+				case KnownTypeCode.Int64:
+				case KnownTypeCode.UInt64:
+				case KnownTypeCode.Double:
+					return 8;
+			}
+			return null;
+		}
+	}
+}