Merge pull request #3828 from sailro/fix-unchecked-constant-overflow

Fix #3826: wrap an overflowing constant subexpression in unchecked()

ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs

@@ -358,6 +358,12 @@ namespace ICSharpCode.Decompiler.Tests
 			await RunCS(options: options);
 		}
 
+		[Test]
+		public async Task UncheckedConstants([ValueSource(nameof(defaultOptions))] CompilerOptions options)
+		{
+			await RunCS(options: options);
+		}
+
 		[Test]
 		public async Task ConditionalAttr([ValueSource(nameof(defaultOptions))] CompilerOptions options)
 		{

ICSharpCode.Decompiler.Tests/TestCases/Correctness/UncheckedConstants.cs

@@ -0,0 +1,45 @@
+// Copyright (c) AlphaSierraPapa for the SharpDevelop Team
+// 
+// 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
+// without restriction, including without limitation the rights to use, copy, modify, merge,
+// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
+// to whom the Software is furnished to do so, subject to the following conditions:
+// 
+// The above copyright notice and this permission notice shall be included in all copies or
+// substantial portions of the Software.
+// 
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
+// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
+// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+// DEALINGS IN THE SOFTWARE.
+
+using System;
+
+namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
+{
+	internal class UncheckedConstants
+	{
+		private static void Main()
+		{
+			Console.WriteLine(CombineHash(1, 2));
+			Console.WriteLine(CombineHash(-5, 100));
+			Console.WriteLine(CombineHash(int.MaxValue, int.MinValue));
+		}
+
+		// A compiler-generated-style hash using a local accumulator. After the decompiler inlines the
+		// accumulator, the leading "1688038063 * -1521134295" becomes a compile-time constant
+		// subexpression that overflows int. Constant subexpressions are always overflow-checked at
+		// compile time, so the decompiled output must wrap it in unchecked(...) or it fails to compile
+		// with CS0220 ("operation overflows at compile time in checked mode").
+		private static int CombineHash(int a, int b)
+		{
+			int num = 1688038063;
+			num = num * -1521134295 + a;
+			num = num * -1521134295 + b;
+			return num;
+		}
+	}
+}

ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@@ -1677,7 +1677,26 @@ namespace ICSharpCode.Decompiler.CSharp
 				.WithRR(rr);
 			if (BinaryOperatorMightCheckForOverflow(op) && !inst.UnderlyingResultType.IsFloatType())
 			{
-				resultExpr.Expression.AddAnnotation(inst.CheckForOverflow ? AddCheckedBlocks.CheckedAnnotation : AddCheckedBlocks.UncheckedAnnotation);
+				object annotation;
+				if (inst.CheckForOverflow)
+				{
+					annotation = AddCheckedBlocks.CheckedAnnotation;
+				}
+				else if (rr.IsCompileTimeConstant && ConstantBinaryOperatorOverflows(op, left.ResolveResult, right.ResolveResult))
+				{
+					// A compile-time constant subexpression is always evaluated in a checked context,
+					// even within an (implicitly) unchecked context, so emitting it bare would fail to
+					// compile with CS0220 ("operation overflows at compile time in checked mode").
+					// Force an explicit unchecked(...) wrapper, just like an overflowing constant
+					// n(u)int cast. This typically happens after inlining turns a runtime accumulator
+					// (e.g. a GetHashCode prime chain) into a constant subexpression.
+					annotation = AddCheckedBlocks.ExplicitUncheckedAnnotation;
+				}
+				else
+				{
+					annotation = AddCheckedBlocks.UncheckedAnnotation;
+				}
+				resultExpr.Expression.AddAnnotation(annotation);
 			}
 			return resultExpr;
 		}
@@ -1780,6 +1799,20 @@ namespace ICSharpCode.Decompiler.CSharp
 			}
 		}
 
+		/// <summary>
+		/// Returns true if the (already unchecked-resolved) constant binary operation overflows when
+		/// evaluated in a checked context. Used to decide whether an explicit unchecked(...) wrapper is
+		/// required: a compile-time constant subexpression is always overflow-checked at compile time,
+		/// regardless of the surrounding (implicitly) unchecked context.
+		/// </summary>
+		bool ConstantBinaryOperatorOverflows(BinaryOperatorType op, ResolveResult left, ResolveResult right)
+		{
+			if (left.ConstantValue == null || right.ConstantValue == null)
+				return false;
+			var checkedResult = resolver.WithCheckForOverflow(true).ResolveBinaryOperator(op, left, right);
+			return checkedResult.IsError;
+		}
+
 		TranslatedExpression HandleShift(BinaryNumericInstruction inst, BinaryOperatorType op, TranslationContext context)
 		{
 			var left = Translate(inst.Left);