JIT: fix issue in cloning loops with trys in handlers (#113586)

Loop cloning with EH had a bug when:
* the loop contained a try T0 that was within a handler H1
* the associated try T1 was also within the loop
* the entire loop was within another try T2

Here T0's containing try is T2, and its enclosing try index was properly
adjusted when the EH table expands as part of cloning. We were doing another
index adjustment later which lead to an out of bounds index.

Also cleaned up the code that detects which try regions need cloning as in the
above we only need to consider T1. When we go to clone it we also will clone H1
 and hence T2.

Also fixed/added some more dumping, and some new test cases.

This fix addresses a problem that came up stress testing #112998. Since the
underlying bug does not require inlining to create the EH setup above,
I am fixing it separately.

Author: AndyAyersMS

src/coreclr/jit/fgehopt.cpp

@@ -2680,8 +2680,8 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
                 if (bbIsTryBeg(block))
                 {
                     assert(added);
-                    JITDUMP("==> found try entry for EH#%02u nested in handler at " FMT_BB "\n", block->bbNum,
-                            block->getTryIndex());
+                    JITDUMP("==> found try entry for EH#%02u nested in handler at " FMT_BB "\n", block->getTryIndex(),
+                            block->bbNum);
                     regionsToProcess.Push(block->getTryIndex());
                 }
             }
@@ -2761,6 +2761,12 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
         assert(insertBeforeIndex == enclosingTryIndex);
     }
 
+    if (insertBeforeIndex != compHndBBtabCount)
+    {
+        JITDUMP("Existing EH region(s) EH#%02u...EH#%02u will become EH#%02u...EH#%02u\n", insertBeforeIndex,
+                compHndBBtabCount - 1, insertBeforeIndex + regionCount, compHndBBtabCount + regionCount - 1);
+    }
+
     // Once we call fgTryAddEHTableEntries with deferCloning = false,
     // all the EH indicies at or above insertBeforeIndex will shift,
     // and the EH table may reallocate.
@@ -2860,7 +2866,7 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
         //
         if (ebd->ebdEnclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX)
         {
-            if (XTnum < clonedOutermostRegionIndex)
+            if (ebd->ebdEnclosingTryIndex < clonedOutermostRegionIndex)
             {
                 ebd->ebdEnclosingTryIndex += (unsigned short)indexShift;
             }
@@ -2873,7 +2879,7 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
 
         if (ebd->ebdEnclosingHndIndex != EHblkDsc::NO_ENCLOSING_INDEX)
         {
-            if (XTnum < clonedOutermostRegionIndex)
+            if (ebd->ebdEnclosingHndIndex < clonedOutermostRegionIndex)
             {
                 ebd->ebdEnclosingHndIndex += (unsigned short)indexShift;
             }

src/coreclr/jit/flowgraph.cpp

@@ -6140,14 +6140,30 @@ bool FlowGraphNaturalLoop::CanDuplicateWithEH(INDEBUG(const char** reason))
             // Check if this is an "outermost" try within the loop.
             // If so, we have more checking to do later on.
             //
-            const bool headerInTry         = header->hasTryIndex();
-            unsigned   blockIndex          = block->getTryIndex();
-            unsigned   outermostBlockIndex = comp->ehTrueEnclosingTryIndex(blockIndex);
+            bool const     headerIsInTry     = header->hasTryIndex();
+            unsigned const blockTryIndex     = block->getTryIndex();
+            unsigned const enclosingTryIndex = comp->ehTrueEnclosingTryIndex(blockTryIndex);
 
-            if ((headerInTry && (outermostBlockIndex == header->getTryIndex())) ||
-                (!headerInTry && (outermostBlockIndex == EHblkDsc::NO_ENCLOSING_INDEX)))
+            if ((headerIsInTry && (enclosingTryIndex == header->getTryIndex())) ||
+                (!headerIsInTry && (enclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX)))
             {
-                tryRegionsToClone.Push(block);
+                // When we clone a try we also clone its handler.
+                //
+                // This try may be enclosed in a handler whose try begin is in the loop.
+                // If so we'll clone this try when we clone (the handler of) that try.
+                //
+                bool isInHandlerOfInLoopTry = false;
+                if (block->hasHndIndex())
+                {
+                    unsigned const    enclosingHndIndex = block->getHndIndex();
+                    BasicBlock* const associatedTryBeg  = comp->ehGetDsc(enclosingHndIndex)->ebdTryBeg;
+                    isInHandlerOfInLoopTry              = this->ContainsBlock(associatedTryBeg);
+                }
+
+                if (!isInHandlerOfInLoopTry)
+                {
+                    tryRegionsToClone.Push(block);
+                }
             }
         }
 

src/tests/JIT/opt/Cloning/loops_with_eh.cs

@@ -17,6 +17,7 @@
 // g   - giant finally (TF will remain try finally)
 // p   - regions are serial, not nested
 // TFi - try finally with what follows in the finally
+// I   - inlining
 // 
 // x: we currently cannot clone loops where the try is the first thing
 // as the header and preheader are different regions
@@ -1127,5 +1128,142 @@ public static int Sum_TFiTFxL(int[] data, int n)
 
         return sum;
     }
+
+    [Fact]
+    public static int Test_LTFiTF() => Sum_LTFiTF(data, n) - 130;
+
+    public static int Sum_LTFiTF(int[] data, int n)
+    {
+        int sum = 0;
+
+        for (int i = 0; i < n; i++)
+        {
+            sum += data[i];
+            try
+            {
+                SideEffect();
+            }
+            finally
+            {
+                try
+                {
+                    SideEffect();
+                }
+                finally
+                {
+                    sum += 1;
+                }
+
+                sum += 1;
+            }
+        }
+
+        return sum;
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static int SomeEH()
+    {
+        int sum = 0;
+
+        try
+        {
+            SideEffect();
+        }
+        finally
+        {
+            try
+            {
+                SideEffect();
+            }
+            finally
+            {
+                sum += 1;
+            }
+
+            sum += 1;
+        }
+
+        return sum;
+    }
+
+    [Fact]
+    public static int Test_LITFiTF() => Sum_LITFiTF(data, n) - 130;
+
+    public static int Sum_LITFiTF(int[] data, int n)
+    {
+        int sum = 0;
+
+        for (int i = 0; i < n; i++)
+        {
+            sum += data[i];
+            sum += SomeEH();
+        }
+
+        return sum;
+    }
+
+    [Fact]
+    public static int Test_TFLTFiTF() => Sum_TFLTFiTF(data, n) - 131;
+
+    public static int Sum_TFLTFiTF(int[] data, int n)
+    {
+        int sum = 0;
+
+        try
+        {
+            for (int i = 0; i < n; i++)
+            {
+                sum += data[i];
+                try
+                {
+                    SideEffect();
+                }
+                finally
+                {
+                    try
+                    {
+                        SideEffect();
+                    }
+                    finally
+                    {
+                        sum += 1;
+                    }
+
+                    sum += 1;
+                }
+            }
+        }
+        finally
+        {
+            SideEffect();
+            sum += 1;
+        }
+        return sum;
+    }
+
+    // [Fact]
+    public static int Test_TFLITFiTF() => Sum_TFLITFiTF(data, n) - 131;
+
+    public static int Sum_TFLITFiTF(int[] data, int n)
+    {
+        int sum = 0;
+
+        try
+        {
+            for (int i = 0; i < n; i++)
+            {
+                sum += data[i];
+                sum += SomeEH();
+            }
+        }
+        finally
+        {
+            SideEffect();
+            sum += 1;
+        }
+
+        return sum;
+    }
 }