dotnet / dotnet/runtime

JIT (bug): Bounds check wrongly removed — decreasing recurrence treated as monotonically increasing

Open
#133,818 1 comment 0 reactions 0 assignees View on GitHub
area-CodeGen-coreclr
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.