Browse Source

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

After inlining turns a runtime accumulator (e.g. a hand-written GetHashCode prime
chain `h = h * -1521134295 + ...`) into one expression, the leading `SEED * PRIME`
becomes a compile-time constant subexpression. C# always evaluates constant
subexpressions in a checked context, so emitting it bare fails to compile with
CS0220, even though the surrounding context is unchecked.

HandleBinaryNumeric now annotates such an overflowing constant binary operation
with ExplicitUncheckedAnnotation (instead of the implicit UncheckedAnnotation), so
AddCheckedBlocks wraps it in an explicit unchecked(...) - mirroring the existing
handling of overflowing constant n(u)int casts.

Assisted-by: Copilot:claude-opus-4.8:GitHub Copilot CLI
pull/3828/head
Sebastien Lebreton 2 weeks ago
parent
commit
1d429d0c2f
  1. 6
      ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs
  2. 45
      ICSharpCode.Decompiler.Tests/TestCases/Correctness/UncheckedConstants.cs
  3. 35
      ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

6
ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs

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

45
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;
}
}
}

35
ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@ -1677,7 +1677,26 @@ namespace ICSharpCode.Decompiler.CSharp
.WithRR(rr); .WithRR(rr);
if (BinaryOperatorMightCheckForOverflow(op) && !inst.UnderlyingResultType.IsFloatType()) 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; 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) TranslatedExpression HandleShift(BinaryNumericInstruction inst, BinaryOperatorType op, TranslationContext context)
{ {
var left = Translate(inst.Left); var left = Translate(inst.Left);

Loading…
Cancel
Save