Skip to content

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

Open
sailro wants to merge 1 commit into
icsharpcode:masterfrom
sailro:fix-unchecked-constant-overflow
Open

Fix #3826: wrap an overflowing constant subexpression in unchecked()#3828
sailro wants to merge 1 commit into
icsharpcode:masterfrom
sailro:fix-unchecked-constant-overflow

Conversation

@sailro

@sailro sailro commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Fixes #3826.

A common hand-written GetHashCode uses a local accumulator:

var hashCode = 1688038063;
hashCode = hashCode * -1521134295 + Type.GetHashCode();
...

Because hashCode is a local, every * -1521134295 is a runtime operation and
compiles fine. The decompiler inlines the accumulator into one nested expression,
so the leading 1688038063 * -1521134295 becomes a compile-time constant
subexpression. In C#, constant subexpressions are always evaluated in a checked
context regardless of the project's overflow setting, so the decompiled output
failed to compile with CS0220 ("operation overflows at compile time in checked
mode").

Fix

HandleBinaryNumeric now detects an unchecked binary operation whose result is a
compile-time constant that overflows in a checked context, and annotates it with
ExplicitUncheckedAnnotation so AddCheckedBlocks emits an explicit
unchecked(...) wrapper. This mirrors the existing handling of an overflowing
constant cast to nint/nuint (TranslatedExpression.cs).

Before:

return (1688038063 * -1521134295 + a) * -1521134295 + b;   // CS0220

After:

return (unchecked(1688038063 * -1521134295) + a) * -1521134295 + b;

Only the overflowing constant subexpression is wrapped; non-constant
multiplications are left as-is.

Test

Correctness/UncheckedConstants exercises the inlined-accumulator hash. Without
the fix the decompiled output fails to recompile (CS0220); with it the output
recompiles and produces identical results.

Real-world occurrence: DiffPlex DiffPiece.GetHashCode (net45).

…nchecked()

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes decompiled C# output failing to recompile when an inlined “hash accumulator” turns into a compile-time constant subexpression that overflows in checked constant-evaluation (CS0220). The change ensures the decompiler emits an explicit unchecked(...) wrapper only for the overflowing constant subexpression, preserving original unchecked runtime semantics and keeping non-constant arithmetic unchanged.

Changes:

  • Update ExpressionBuilder.HandleBinaryNumeric to apply AddCheckedBlocks.ExplicitUncheckedAnnotation for unchecked binary ops that become overflowing compile-time constants under checked evaluation.
  • Add ConstantBinaryOperatorOverflows(...) helper to detect checked-context overflow for already-constant binary operations.
  • Add a new correctness test case (UncheckedConstants) and wire it into CorrectnessTestRunner.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs Detects overflowing constant subexpressions in unchecked binary ops and forces explicit unchecked(...) emission via ExplicitUncheckedAnnotation.
ICSharpCode.Decompiler.Tests/TestCases/Correctness/UncheckedConstants.cs Adds a repro-style hash-chain test that becomes a constant overflow after inlining.
ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs Registers the new UncheckedConstants correctness test in the test runner.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inlined hash accumulator becomes a constant overflow (CS0220) in decompiled GetHashCode

2 participants