Fix bug 1213453 (GitHub issue #1421): incorrect EH scopes for optimized loops

(Port changeset 1518001 from CodeGen)

The bug occurs when fgOptWhileLoop() duplicates a condition from one 'try' region into another, and that condition throws an exception, such as for an array bounds check. This was the test case:

try
{
    Console.WriteLine("try");
}
catch (IndexOutOfRangeException)
{
    Console.WriteLine("bad");
    result = FAIL;
}
while (a[42] != 0) {}

Before the fix, the "a[42] != 0" condition was duplicated into the 'try' region, thus causing an IndexOutOfRange exception to be thrown, and caught in the wrong region.

The main fix is to compare the 'try' region of the region where the condition exists and the region we intend to copy it to. The other changes, in optimizer.cpp and flowgraph.cpp, are "defense in depth" -- to be more careful with EH checks in two very similar optimizations.

This bug existed in AMD64 and ARM32.

There are 2 functions with ARM32 asm diffs.

There are 70 functions with AMD64 SuperAsm diffs.

There are many cases where we prohibit the optimization that caused the bug. There are a few cases where we don't do the optimization but could have if we were more careful about checking for precise EH conditions that are allowed, namely, that branching to the first block of a 'try' region is ok.

[tfs-changeset: 1518014]

2 files changed, 16 insertions, 0 deletions

diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp
index e7b7584ae8..5e24ab918b 100644
--- a/src/jit/flowgraph.cpp
+++ b/src/jit/flowgraph.cpp
@@ -14096,6 +14096,11 @@ bool                Compiler::fgOptimizeBranch(BasicBlock* bJump)
     if (bDest->bbJumpDest != bJump->bbNext)
         return false;
 
+	// 'bJump' must be in the same try region as the condition, since we're going to insert
+	// a duplicated condition in 'bJump', and the condition might include exception throwing code.
+	if (!BasicBlock::sameTryRegion(bJump, bDest))
+		return false;
+
     // do not jump into another try region
     BasicBlock* bDestNext = bDest->bbNext;
     if (bDestNext->hasTryIndex() && !BasicBlock::sameTryRegion(bJump, bDestNext))
diff --git a/src/jit/optimizer.cpp b/src/jit/optimizer.cpp
index 2f781e9840..7e85f60a3e 100644
--- a/src/jit/optimizer.cpp
+++ b/src/jit/optimizer.cpp
@@ -3316,6 +3316,17 @@ void                Compiler::fgOptWhileLoop(BasicBlock * block)
     // Since test is a BBJ_COND it will have a bbNext
     noway_assert(bTest->bbNext);
 
+	// 'block' must be in the same try region as the condition, since we're going to insert
+	// a duplicated condition in 'block', and the condition might include exception throwing code.
+	if (!BasicBlock::sameTryRegion(block, bTest))
+		return;
+
+    // We're going to change 'block' to branch to bTest->bbNext, so that also better be in the
+	// same try region (or no try region) to avoid generating illegal flow.
+    BasicBlock* bTestNext = bTest->bbNext;
+    if (bTestNext->hasTryIndex() && !BasicBlock::sameTryRegion(block, bTestNext))
+        return;
+
     GenTreePtr condStmt = optFindLoopTermTest(bTest);
 
     // bTest must only contain only a jtrue with no other stmts, we will only clone