Skip to content

Make wrapping arithmetic unchecked - #648

Merged
SimonCropp merged 3 commits into
mainfrom
fix/unchecked-arithmetic
Oct 11, 2026
Merged

SimonCropp merged 3 commits into
mainfrom
fix/unchecked-arithmetic

Conversation

@SimonCropp

Copy link
Copy Markdown
Owner

Bug

Polyfill is compiled into the consuming project, so it inherits the consumer's CheckForOverflowUnderflow setting. Several polyfills rely on wrapping arithmetic without saying so, and throw OverflowException in a checked build:

  • BitConverter.UInt32BitsToSingle, UInt64BitsToDouble, SingleToUInt32Bits, DoubleToUInt64Bits: signed/unsigned reinterpreting casts, e.g. SingleToUInt32Bits(-1f)
  • BitOperations.IsPow2 (all six overloads, in both BitOperations.cs and BitOperationsIsPow2.cs): value & (value - 1) for 0u, int.MinValue, long.MinValue, ...
  • RandomNumberGenerator.GetInt32(fromInclusive, toExclusive): (uint) casts of negative bounds, e.g. GetInt32(-10, -5)
  • Random.NextInt64(minValue, maxValue): maxValue - minValue and the final cast/add for ranges wider than long.MaxValue, e.g. NextInt64(long.MinValue, long.MaxValue)

Fix

Wrap those expressions in unchecked(...). No behaviour change in unchecked builds.

Test

Added edge-value tests: the four BitConverter *_NegativeOne tests, BitOperations_IsPow2_NativeMinValue, GetInt32_Range_Negative, GetInt32_Range_Full, RandomNextInt64_NegativeRange and RandomNextInt64_FullRange (the existing IsPow2 tests already cover 0u and int.MinValue).

The test projects build unchecked, so these only fail before the fix in a checked build. Verified locally with -p:CheckForOverflowUnderflow=true on net48: 13 of the 21 selected tests failed with OverflowException before the fix and all pass after. In the normal build they pass on net48 and net8.0 (polyfill) and net10.0 (BCL). The BitOperationsIsPow2.cs copy is only active on netcoreapp3.0 to net6.0, whose runtimes are not installed locally, so that file is covered by CI only.

@SimonCropp SimonCropp added this to the 11.4.4 milestone Oct 11, 2026
@SimonCropp
SimonCropp force-pushed the fix/unchecked-arithmetic branch from 14410a6 to de90bd4 Compare October 11, 2026 22:35
@SimonCropp
SimonCropp merged commit 8a2509d into main Oct 11, 2026
@SimonCropp
SimonCropp deleted the fix/unchecked-arithmetic branch October 11, 2026 22:57
This was referenced Oct 12, 2026
This was referenced Oct 14, 2026
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.

2 participants