JIT: missing bounds check (?)

[AndyAyersMS]

Found by llm-fuzz, based on JIT\Regression\JitBlue\Runtime_1104

;;; minopts

i=8, j=14
i=7, j=13
i=6, j=12
i=5, j=11
i=4, j=10
i=3, j=9
i=2, j=8
i=1, j=7
i=0, j=6
caught IndexOutOfRangeException
Final checksum: 100

;;; tc=0

i=8, j=14
i=7, j=13
i=6, j=12
i=5, j=11
i=4, j=10
i=3, j=9
i=2, j=8
i=1, j=7
i=0, j=6
Fatal error.
System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at Runtime_1104.TestOutOfBoundLowerDecreasing()
   at Runtime_1104.TestOutOfBoundProxy(System.Func`1<Int32>)
   at Runtime_1104.Main()

Compile and run with TC=0:

// Mutation 4
// foreach with interface call, conditional execution, and stack allocatable object
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;

struct MyValue
{
    public int Number;
    public override string ToString() => Number.ToString();
}

public class Runtime_1104
{
    static int TestOutOfBoundProxy(Func<int> actualTest)
    {
        try
        {
            actualTest();
        }
        catch (IndexOutOfRangeException)
        {
            Console.WriteLine("caught IndexOutOfRangeException");
            return 100;
        }
        Debug.Fail("unreached");
        return 101;
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int TestOutOfBoundLowerDecreasing()
    {
        int[] arr = new int[10];
        for (int m = 0; m < arr.Length; m++) arr[m] = m * 3;

        List<MyValue> myValues = new List<MyValue>();
        for (int n = 0; n < 10; n++)
        {
            myValues.Add(new MyValue { Number = n });
        }

        int sum = 0;
        int i = 9;
        int j = 15;

        while (j >= 0 && i < 10)
        {
            --j;
            --i;
            sum += arr[i];

            // redundant branch testing j >= 0 twice
            if (j >= 0)
            {
                if (j >= 0)
                {
                    Console.WriteLine($"i={i}, j={j}");
                }
            }
        }

        // foreach with interface call (ToString) on unmodified variable
        if (sum > 0)
        {
            foreach (MyValue val in myValues)
            {
                sum += int.Parse(val.ToString());
            }
        }

        Console.WriteLine($"Checksum: {sum}");
        return sum;
    }

    public static void Main()
    {
        int result;
        try
        {
            result = TestOutOfBoundProxy(TestOutOfBoundLowerDecreasing);
        }
        catch (Exception)
        {
            result = 101;
        }
        Console.WriteLine($"Final checksum: {result}");
    }
}

[AndyAyersMS]

A different variant with the same failure

// Mutation 8
// Added foreach cloning, redundant branches, and stack allocatable objects with conditional control flow
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;

struct SimpleStruct
{
    public int Number;
}

public class Runtime_1104
{
    static int TestOutOfBoundProxy(Func<int> actualTest)
    {
        try
        {
            actualTest();
        }
        catch (IndexOutOfRangeException)
        {
            Console.WriteLine("caught IndexOutOfRangeException");
            return 100;
        }
        Debug.Fail("unreached");
        return 101;
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int TestOutOfBoundLowerDecreasing()
    {
        int[] arr = new int[10];
        for (int x = 0; x < arr.Length; x++)
            arr[x] = x + 7;

        List<SimpleStruct> simpleList = new List<SimpleStruct>();
        for (int y = 0; y < 10; y++)
            simpleList.Add(new SimpleStruct { Number = y });

        int sum = 0;
        int i = 9;
        int j = 15;

        while (j >= 0 && i < 10)
        {
            --j;
            --i;
            sum += arr[i];
            // redundant branch testing i < 10 twice
            if (i < 10)
            {
                if (i < 10)
                {
                    Console.WriteLine($"i={i}, j={j}");
                }
            }
        }

        if (sum > 10)
        {
            foreach (var s in simpleList)
            {
                sum += s.Number;
            }
            // cloning: foreach cloning with same list
            foreach (var s in simpleList)
            {
                sum += s.Number * 2;
            }
        }

        Console.WriteLine($"Checksum: {sum}");
        return sum;
    }

    public static void Main()
    {
        int result;
        try
        {
            result = TestOutOfBoundProxy(TestOutOfBoundLowerDecreasing);
        }
        catch (Exception)
        {
            result = 101;
        }
        Console.WriteLine($"Final checksum: {result}");
    }
}

[AndyAyersMS]

FYI @dotnet/jit-contrib

[sumansuhag]

while (j >= 0 && i < 10)
{
--j;
--i;
sum += arr[i]; // Potential out-of-bounds access
To reproduce this issue, use a .NET runtime debug build with the settings COMPlus_JitDisasm=* and COMPlus_TieredCompilation=0 to look at the generated assembly.

Make sure to check the JIT dump to see if there's a bounds check for arr[i]. If you find that this leads to unsafe memory access that can be reproduced, report it in the JIT repository.

It looks like the problem stems from the JIT optimization skipping the bounds check. In the TestOutOfBoundLowerDecreasing test, the index i goes down inside the loop before it's used to access arr[i]. The loop only checks if i is less than 10, but it doesn’t check if i is zero or more. This works fine in minopts mode where you get an IndexOutOfRangeException, but in optimized mode, it throws a fatal AccessViolationException. This suggests that the bounds check is missing. It might be a regression in how loops are optimized or how ranges are analyzed. It would be good to take a closer look at the JIT behavior at the IR level.

[amanasifkhalid]

@AndyAyersMS does it repro with the fix in #113935 included? I suspect this is the issue that PR fixes.

[EgorBo]

@amanasifkhalid yes, I confirm it's fixed by #113935

[EgorBo]

Closed by #113935