diff --git a/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs b/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs index b091121d3..36f1d3981 100644 --- a/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs +++ b/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) { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/UncheckedConstants.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/UncheckedConstants.cs new file mode 100644 index 000000000..9e09f9260 --- /dev/null +++ b/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; + } + } +} diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 0b6b87504..fc4f6ad48 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/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 } } + /// + /// 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. + /// + 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);