JIT (bug): Bounds check wrongly removed — decreasing recurrence treated as monotonically increasing
- Dominant language
- C#
- Stars
- 18.3k
- Forks
- 5.6k
- PR merge metrics
- PR metrics pending
Description
`RangeCheck::IsMonotonicallyIncreasing` concludes that an index derived from a *decreasing* integer recurrence is monotonically increasing, so the array bounds check is removed. The negative index is then zero-extended into the element address, giving an out-of-bounds read / `AccessViolationException` instead of `IndexOutOfRangeException`.
### Minimal Repro
```csharp
using System;
using System.Runtime.CompilerServices;
class Program
{
[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
static int Test(int[] arr, int n)
{
int sum = 0, x = -5, y = 0;
for (int c = 0; c < n; c++)
{
if (x >= 50 || y >= 50) break;
int idx = x + 100;
if (idx >= arr.Length) break;
sum += arr[idx];
int t = x + y;
y = x;
x = t;
}
return sum;
}
static void Main()
{
int[] arr = new int[100];
try
{
Console.WriteLine("Test: " + Test(arr, 8));
}
catch (IndexOutOfRangeException)
{
Console.WriteLine("Test: IndexOutOfRangeException");
}
}
}
```
`x` walks `-5, -5, -10, -15, -25, -40, -65, -105`, so on the 8th iteration `idx == -5` passes the user's upper-bound guard.
### Expected
```
Test: IndexOutOfRangeException
```
### Actual
```
UB
```
No `CORINFO_HELP_RNGCHKFAIL` is emitted; the negative index is zero-extended (`mov r9d, r9d`) into the load address. `DOTNET_JITMinOpts=1` is correct.
### Notes
- x64, `main` @ `6ed2ba318dcaa936da60f7d6bc9eb30cad54fc0c`. **Also reproduces on .NET 10.0.12 Release**, so it is not a regression in `main`; memory-safety hole reachable from safe C#.
- Root cause: the search path in `RangeCheck::IsMonotonicallyIncreasing` (`src/coreclr/jit/rangecheck.cpp`) is keyed only on `expr`, not on `(expr, rejectNegativeConst)`. `x = PHI(-5, x + y)` re-enters with `rejectNegativeConst == true` for the `ADD(x, y)` operand, hits `alreadyPresent` and returns `true` without ever re-checking the `-5` seed.
- JitDump shows the resulting bogus lower bound: `Computed Range [000031] => <90, $145 + -1>`, then `[RangeCheck::OptimizeRangeCheck] Between bounds`.
Contributor guide
Research direction
Start with src/coreclr/jit/rangecheck.cpp and the RangeCheck::IsMonotonicallyIncreasing search path, then reproduce the supplied C# program with the stated commit and inspect the JitDump. Done means the decreasing recurrence no longer causes the bounds check to be removed and the program produces IndexOutOfRangeException rather than an out-of-bounds read.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100