Fix region asserts for Leaves converted to Br in non exception finally region

Meghana Gupta · Meghana Gupta · commit b7251c713c92 · 2017-07-12T10:55:16.000-07:00
Mark Leaves converted to Br in non excpetion finally region with m_leaveConvToBr bit on BranchInstr in DBG builds,
so that FlowGraph::Destroy asserts correctly for such branches
diff --git a/lib/Backend/FlowGraph.cpp b/lib/Backend/FlowGraph.cpp
@@ -515,6 +515,9 @@ FlowGraph::Build(void)
                     Assert(currentLabel->GetRegion()->GetMatchingTryRegion()->GetMatchingFinallyRegion(false) == currentLabel->GetRegion());
                     // Convert Leave to Br because we execute non-excepting Finally in native code
                     instr->m_opcode = Js::OpCode::Br;
+#if DBG
+                    instr->AsBranchInstr()->m_leaveConvToBr = true;
+#endif
                 }
             }
             else if (instr->m_opcode == Js::OpCode::Finally)
@@ -1724,18 +1727,22 @@ FlowGraph::Destroy(void)
                         Assert(region->GetType() == RegionTypeTry || region->GetType() == RegionTypeCatch || region->GetType() == RegionTypeFinally);
                         break;
                     case Js::OpCode::Br:
-                        if (region->GetType() == RegionTypeCatch && region != predRegion)
+                        if (predBlock->GetLastInstr()->AsBranchInstr()->m_leaveConvToBr)
+                        {
+                            // Leave converted to Br in finally region
+                            AssertMsg(region == predRegion->GetParent(), "Bad region prop in finally");
+                        }
+                        else if (region->GetType() == RegionTypeCatch && region != predRegion)
                         {
                             AssertMsg(predRegion->GetType() == RegionTypeTry, "Bad region type for the try");
                         }
                         else if (region->GetType() == RegionTypeFinally && region != predRegion)
                         {
-                            // When we add edge from finally to early exit, and break block removal moves the edge into finally region, we can end up with an edge between finally and non eh region
+                            AssertMsg(predRegion->GetType() == RegionTypeTry, "Bad region type for the try");
                         }
                         else
                         {
-                            // Leave's within non excepting finallys that are not early exit edges are converted to br
-                            AssertMsg((predRegion->IsNonExceptingFinally() && region == predRegion->GetParent()) || region == predRegion, "Bad region propagation through interior block");
+                            AssertMsg(region == predRegion, "Bad region propagation through interior block");
                         }
                         break;
                     default:
diff --git a/lib/Backend/IR.h b/lib/Backend/IR.h
@@ -739,6 +739,7 @@ class BranchInstr : public Instr
 #if DBG
     bool                 m_isMultiBranch;
     bool                 m_isHelperToNonHelperBranch;
+    bool                 m_leaveConvToBr;
 #endif
 
 public:
@@ -751,6 +752,7 @@ class BranchInstr : public Instr
     {
 #if DBG
         m_isMultiBranch = false;
+        m_leaveConvToBr = false;
 #endif
     }
 

Original file line number	Diff line number	Diff line change
`@@ -515,6 +515,9 @@ FlowGraph::Build(void)`
`515`	`515`	`Assert(currentLabel->GetRegion()->GetMatchingTryRegion()->GetMatchingFinallyRegion(false) == currentLabel->GetRegion());`
`516`	`516`	`// Convert Leave to Br because we execute non-excepting Finally in native code`
`517`	`517`	`instr->m_opcode = Js::OpCode::Br;`
	`518`	`+#if DBG`
	`519`	`+ instr->AsBranchInstr()->m_leaveConvToBr = true;`
	`520`	`+#endif`
`518`	`521`	`}`
`519`	`522`	`}`
`520`	`523`	`else if (instr->m_opcode == Js::OpCode::Finally)`
`@@ -1724,18 +1727,22 @@ FlowGraph::Destroy(void)`
`1724`	`1727`	`Assert(region->GetType() == RegionTypeTry \|\| region->GetType() == RegionTypeCatch \|\| region->GetType() == RegionTypeFinally);`
`1725`	`1728`	`break;`
`1726`	`1729`	`case Js::OpCode::Br:`
`1727`		`- if (region->GetType() == RegionTypeCatch && region != predRegion)`
	`1730`	`+ if (predBlock->GetLastInstr()->AsBranchInstr()->m_leaveConvToBr)`
	`1731`	`+ {`
	`1732`	`+ // Leave converted to Br in finally region`
	`1733`	`+ AssertMsg(region == predRegion->GetParent(), "Bad region prop in finally");`
	`1734`	`+ }`
	`1735`	`+ else if (region->GetType() == RegionTypeCatch && region != predRegion)`
`1728`	`1736`	`{`
`1729`	`1737`	`AssertMsg(predRegion->GetType() == RegionTypeTry, "Bad region type for the try");`
`1730`	`1738`	`}`
`1731`	`1739`	`else if (region->GetType() == RegionTypeFinally && region != predRegion)`
`1732`	`1740`	`{`
`1733`		`- // When we add edge from finally to early exit, and break block removal moves the edge into finally region, we can end up with an edge between finally and non eh region`
	`1741`	`+ AssertMsg(predRegion->GetType() == RegionTypeTry, "Bad region type for the try");`
`1734`	`1742`	`}`
`1735`	`1743`	`else`
`1736`	`1744`	`{`
`1737`		`- // Leave's within non excepting finallys that are not early exit edges are converted to br`
`1738`		`- AssertMsg((predRegion->IsNonExceptingFinally() && region == predRegion->GetParent()) \|\| region == predRegion, "Bad region propagation through interior block");`
	`1745`	`+ AssertMsg(region == predRegion, "Bad region propagation through interior block");`
`1739`	`1746`	`}`
`1740`	`1747`	`break;`
`1741`	`1748`	`default:`
Original file line number	Diff line number	Diff line change
`@@ -739,6 +739,7 @@ class BranchInstr : public Instr`
`739`	`739`	`#if DBG`
`740`	`740`	`bool m_isMultiBranch;`
`741`	`741`	`bool m_isHelperToNonHelperBranch;`
	`742`	`+ bool m_leaveConvToBr;`
`742`	`743`	`#endif`
`743`	`744`
`744`	`745`	`public:`
`@@ -751,6 +752,7 @@ class BranchInstr : public Instr`
`751`	`752`	`{`
`752`	`753`	`#if DBG`
`753`	`754`	`m_isMultiBranch = false;`
	`755`	`+ m_leaveConvToBr = false;`
`754`	`756`	`#endif`
`755`	`757`	`}`
`756`	`758`