3.0, 3.1, master: Jit Range checks bug

[sandreenko]

dotnet/coreclr#23109 changed condition in IsMonotonicallyIncreasing from
bool alreadyPresent = m_pSearchPath->Set(expr, nullptr); to bool alreadyPresent = !m_pSearchPath->Set(expr, nullptr, SearchPath::Overwrite);.
I suspect that was a typo, PTAL @briansull.

Because of that we now consider all intervals as increasing (because there is nothing in m_pSearchPath when we start an iteration, so Set always returns false that means there was no previous value).

We use IsMonotonicallyIncreasing to determinate the lower bound, so in order to expose the issue we need to cause index out or range exception using the wrong lower bound value determined by IsMonotonicallyIncreasing == true.

For example:

using System;
using System.Diagnostics;
using System.Runtime.CompilerServices;

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

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int TestOutOfBoundLowerDecreasing()
    {
        int[] arr = new int[10];
        int i = 9;
        int j = 15;
        int sum = 0;
        while (j >= 0 && i < 10) // our range check optimizer is very naive, so if you don't have
        {                                      // i < 10, then it thinks `i` can overflow and doesn't bother 
                                               // calling `Widen` at all.
            --j;
            --i;
            sum += arr[i];  // range check will use 9 as lower  bound.
        }
        return sum;
    }

    public static int Main()
    {
        try
        {
            TestOutOfBoundProxy(TestOutOfBoundLowerDecreasing);
        }
        catch (Exception)
        {
            return 101;
        }

        return 100;
    }
}

Run x64 Debug with optimized code.
Expected:

C:\Program Files\dotnet\dotnet.exe (process 16152) exited with code 100.

Actual:

Process terminated. Assertion failed.
unreached
   at RangeChecks.TestOutOfBoundProxy(Func`1 actualTest) in C:\Users\seandree\source\repos\SmallTestsCSharp\TestDrawing\Program.cs:line 18
   at RangeChecks.Main() in C:\Users\seandree\source\repos\SmallTestsCSharp\TestDrawing\Program.cs:line 41

C:\Users\seandree\source\repos\SmallTestsCSharp\TestDrawing\bin\Debug\netcoreapp3.0\TestDrawing.exe (process 33588) exited with code -2146232797.
Press any key to close this window . .

The regression affects 3.0, 3.1 and master. Works right in 2.2.

PTAL @BruceForstall, @dotnet/jit-contrib.

Do we need to fix that in 3.1? In that case, I can merge a precise fix now, otherwise, I would like to continue working on bigger optimizations there and get rid of the existing logic (that is my follow-up for #198)

category:correctness
theme:bounds-checks
skill-level:expert
cost:medium

[BruceForstall]

This has been fixed in master and 3.1, so I think it's time to close.