[release/7.0] JIT: fix bug in cloning conditions for jagged array (#8…

…3414) (#83462)

Backport of #83414 to release/7.0, fixes #83242

When checking that an inner array access is in bounds, we must ensure any outer
access is fully in bounds too. We were checking that `idx < array.Len` but not
that `idx >= 0`.

Use an unsigned compare for this check so we can do both sides with a single
instruction.

Fixes #83242.

src/coreclr/jit/loopcloning.cpp

@@ -191,7 +191,15 @@ GenTree* LC_Condition::ToGenTree(Compiler* comp, BasicBlock* bb, bool invert)
     GenTree* op1Tree = op1.ToGenTree(comp, bb);
     GenTree* op2Tree = op2.ToGenTree(comp, bb);
     assert(genTypeSize(genActualType(op1Tree->TypeGet())) == genTypeSize(genActualType(op2Tree->TypeGet())));
-    return comp->gtNewOperNode(invert ? GenTree::ReverseRelop(oper) : oper, TYP_INT, op1Tree, op2Tree);
+
+    GenTree* result = comp->gtNewOperNode(invert ? GenTree::ReverseRelop(oper) : oper, TYP_INT, op1Tree, op2Tree);
+
+    if (compareUnsigned)
+    {
+        result->gtFlags |= GTF_UNSIGNED;
+    }
+
+    return result;
 }
 
 //--------------------------------------------------------------------------------------------------
@@ -941,12 +949,14 @@ void LC_ArrayDeref::DeriveLevelConditions(JitExpandArrayStack<JitExpandArrayStac
     }
     else
     {
-        // Adjust for level0 having just 1 condition and push condition (i < a.len).
+        // Adjust for level0 having just 1 condition and push conditions (i >= 0) && (i < a.len).
+        // We fold the two compares into one using unsigned compare, since we know a.len is non-negative.
+        //
         LC_Array arrLen = array;
         arrLen.oper     = LC_Array::ArrLen;
         arrLen.dim      = level - 1;
-        (*conds)[level * 2 - 1]->Push(
-            LC_Condition(GT_LT, LC_Expr(LC_Ident(Lcl(), LC_Ident::Var)), LC_Expr(LC_Ident(arrLen))));
+        (*conds)[level * 2 - 1]->Push(LC_Condition(GT_LT, LC_Expr(LC_Ident(Lcl(), LC_Ident::Var)),
+                                                   LC_Expr(LC_Ident(arrLen)), /* unsigned */ true));
 
         // Push condition (a[i] != null)
         LC_Array arrTmp = array;
@@ -1481,7 +1491,7 @@ bool Compiler::optComputeDerefConditions(unsigned loopNum, LoopCloneContext* con
         assert(maxRank != -1);
 
         // First level will always yield the null-check, since it is made of the array base variables.
-        // All other levels (dimensions) will yield two conditions ex: (i < a.length && a[i] != null)
+        // All other levels (dimensions) will yield two conditions ex: ((unsigned) i < a.length && a[i] != null)
         // So add 1 after rank * 2.
         const unsigned condBlocks = (unsigned)maxRank * 2 + 1;
 

src/coreclr/jit/loopcloning.h

@@ -625,12 +625,13 @@ struct LC_Condition
     LC_Expr    op1;
     LC_Expr    op2;
     genTreeOps oper;
+    bool       compareUnsigned;
 
 #ifdef DEBUG
     void Print()
     {
         op1.Print();
-        printf(" %s ", GenTree::OpName(oper));
+        printf(" %s%s ", GenTree::OpName(oper), compareUnsigned ? "U" : "");
         op2.Print();
     }
 #endif
@@ -646,7 +647,8 @@ struct LC_Condition
     LC_Condition()
     {
     }
-    LC_Condition(genTreeOps oper, const LC_Expr& op1, const LC_Expr& op2) : op1(op1), op2(op2), oper(oper)
+    LC_Condition(genTreeOps oper, const LC_Expr& op1, const LC_Expr& op2, bool asUnsigned = false)
+        : op1(op1), op2(op2), oper(oper), compareUnsigned(asUnsigned)
     {
     }
 

src/tests/JIT/Regression/JitBlue/Runtime_83242/Runtime_83242.cs

@@ -0,0 +1,55 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.CompilerServices;
+
+class Runtime_83242
+{
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Map(int i)
+    {
+        if (i == 5) return -1;
+        return i;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static void Setup(int[][] a)
+    {
+        for (int i = 0; i < a.Length; i++)
+        {
+            a[i] = new int[5];
+
+            for (int j = 0; j < 5; j++)
+            {
+                a[i][j] = j;
+            }
+        }
+    }
+
+    public static int Main()
+    {
+        int[][] a = new int[11][];
+        int sum = 0;
+        Setup(a);
+
+        for (int i = 0; i < a.Length; i++)
+        {
+            int ii = Map(i);
+
+            // Need to ensure ii >= 0 is in the cloning
+            // conditions for the following loop
+
+            for (int j = 0; j < 5; j++)
+            {
+                if (ii >= 0)
+                {
+                    sum += a[ii][j];
+                }
+            }
+        }
+
+        Console.WriteLine($"sum is {sum}\n");
+        return sum;
+    }
+}

src/tests/JIT/Regression/JitBlue/Runtime_83242/Runtime_83242.csproj

@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>