Fix GH Issue 16892 - GC hole due to GT_INDEX_ADDR

src/jit/compiler.cpp

@@ -9546,10 +9546,6 @@ int cTreeFlagsIR(Compiler* comp, GenTree* tree)
 
             case GT_INDEX:
 
-                if (tree->gtFlags & GTF_INX_RNGCHK)
-                {
-                    chars += printf("[INX_RNGCHK]");
-                }
                 if (tree->gtFlags & GTF_INX_REFARR_LAYOUT)
                 {
                     chars += printf("[INX_REFARR_LAYOUT]");
@@ -9558,6 +9554,12 @@ int cTreeFlagsIR(Compiler* comp, GenTree* tree)
                 {
                     chars += printf("[INX_STRING_LAYOUT]");
                 }
+                __fallthrough;
+            case GT_INDEX_ADDR:
+                if (tree->gtFlags & GTF_INX_RNGCHK)
+                {
+                    chars += printf("[INX_RNGCHK]");
+                }
                 break;
 
             case GT_IND:

src/jit/flowgraph.cpp

@@ -23615,6 +23615,8 @@ Compiler::fgWalkResult Compiler::fgChkThrowCB(GenTree** pTree, fgWalkData* data)
             break;
 
         case GT_INDEX:
+        case GT_INDEX_ADDR:
+            // These two call CORINFO_HELP_RNGCHKFAIL for Debug code
             if (tree->gtFlags & GTF_INX_RNGCHK)
             {
                 return Compiler::WALK_ABORT;

src/jit/gentree.h

@@ -893,7 +893,7 @@ struct GenTree
 #define GTF_FLD_VOLATILE            0x40000000 // GT_FIELD/GT_CLS_VAR -- same as GTF_IND_VOLATILE
 #define GTF_FLD_INITCLASS           0x20000000 // GT_FIELD/GT_CLS_VAR -- field access requires preceding class/static init helper
 
-#define GTF_INX_RNGCHK              0x80000000 // GT_INDEX -- the array reference should be range-checked.
+#define GTF_INX_RNGCHK              0x80000000 // GT_INDEX/GT_INDEX_ADDR -- the array reference should be range-checked.
 #define GTF_INX_REFARR_LAYOUT       0x20000000 // GT_INDEX
 #define GTF_INX_STRING_LAYOUT       0x40000000 // GT_INDEX -- this uses the special string array layout
 

src/jit/morph.cpp

@@ -1590,6 +1590,7 @@ void fgArgInfo::ArgsComplete()
         // This means unnesting, sorting, etc.  Technically this is overly
         // conservative, but I want to avoid as much special-case debug-only code
         // as possible, so leveraging the GTF_CALL flag is the easiest.
+        //
         if (!(argx->gtFlags & GTF_CALL) && (argx->gtFlags & GTF_EXCEPT) && (argCount > 1) &&
             compiler->opts.compDbgCode &&
             (compiler->fgWalkTreePre(&argx, Compiler::fgChkThrowCB) == Compiler::WALK_ABORT))
@@ -6063,6 +6064,7 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree)
         indexAddr->gtFlags |= (array->gtFlags | index->gtFlags) & GTF_ALL_EFFECT;
 
         // Mark the indirection node as needing a range check if necessary.
+        // Note this will always be true unless JitSkipArrayBoundCheck() is used
         if ((indexAddr->gtFlags & GTF_INX_RNGCHK) != 0)
         {
             fgSetRngChkTarget(indexAddr);

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

@@ -0,0 +1,104 @@
+using System;
+using System.Runtime.CompilerServices;
+//
+//  Test case for a GC Stress 4 failure
+//
+//  This test was failing during a GC Stress 4 run in the method Test(...)
+//
+//  The failure requires that this test be built with Debug codegen
+//
+//  The GC Stress failure will occur if the JIT
+//     1. has evaluated and stored the two outgoing stack based arguments: a5, a6
+//     2. and then performs a call to the helper CORINFO_HELP_RNGCHKFAIL
+//
+//  With the fix the JIT will evaluate the arr[3] with the rangecheck 
+//  into a new compiler temp, before storing any outgoing arguments.
+//
+
+class Item
+{
+    int _value;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public Item(int value)  { _value = value; }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public int GetValue() { return _value; }
+}
+
+class Program
+{
+    public Item[] itemArray;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    void Init()
+    {
+        itemArray = new Item[11];
+        for (int i=0; i<11; i++)
+        {
+            itemArray[i] = new Item(i);
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Compute(Item r1, Item r2, Item r3, Item r4, Item s5, Item s6)
+    {
+        int result = r1.GetValue();
+        result += r2.GetValue();
+        result += r3.GetValue();
+        result += r4.GetValue();
+        result += s5.GetValue();
+        result += s6.GetValue();
+        return result;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    int Test(Item a1, Item a2, Item a4, Item a5, Item a6)
+    {
+        Item[] arr = itemArray;
+        int result = 0;
+
+        // Insure that we have to generate fully interruptible GC information
+        // Form a possible infinte loop that the JIT believes could execute
+        // without encountering a GC safe point.
+        //
+        do {
+            if (result < 5)
+            {
+                result = Compute(a1, a2, arr[3], a4, a5, a6);
+            }
+        } while (result < 0);
+
+        return result;
+
+    }
+
+    static int Main(string[] args)
+    {
+        Program prog = new Program();
+
+        prog.Init();
+
+        Item[] arr = prog.itemArray;
+
+        Item obj1 = arr[1];
+        Item obj2 = arr[2];
+        Item obj3 = arr[3];
+        Item obj4 = arr[4];
+        Item obj5 = arr[5];
+        Item obj6 = arr[6];
+
+        int result = prog.Test(obj1, obj2, obj4, obj5, obj6);
+
+        if (result == 21)
+        {
+            Console.WriteLine("Passed");
+            return 100;
+        }
+        else
+        {
+            Console.WriteLine("Failed");
+            return -1;
+        }
+    }
+}

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

@@ -0,0 +1,41 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{76E69AA0-8C5A-4F76-8561-B8089FFA8D79}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+  </PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <PropertyGroup>
+    <DebugType>Full</DebugType>
+    <Optimize>False</Optimize>
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+  <PropertyGroup>
+    <ProjectAssetsFile>$(JitPackagesConfigFileDirectory)benchmark\obj\project.assets.json</ProjectAssetsFile>
+  </PropertyGroup>
+</Project>