From cd993ad0741d767fb6694b1976ad753318ff0fdb Mon Sep 17 00:00:00 2001
From: Daniel Grunwald <daniel@danielgrunwald.de>
Date: Wed, 11 Oct 2017 20:53:00 +0200
Subject: [PATCH] Property validate exit points determined using
 post-dominance.

---
 .../IL/ControlFlow/LoopDetection.cs           | 49 +++++++++++++++++--
 1 file changed, 45 insertions(+), 4 deletions(-)

diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs
index ad95af4c0..e01a84129 100644
--- a/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs
+++ b/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs
@@ -293,22 +293,63 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 						commonAncestor = Dominance.FindCommonDominator(commonAncestor, revNode);
 					}
 				}
+				// All paths from within the loop to a reachable exit run through 'commonAncestor'.
+				// However, this doesn't mean that 'commonAncestor' is valid as an exit point.
+				// We walk up the post-dominator tree until we've got a valid exit point:
 				ControlFlowNode exitPoint;
 				while (commonAncestor.UserIndex >= 0) {
 					exitPoint = cfg[commonAncestor.UserIndex];
 					Debug.Assert(exitPoint.Visited == naturalLoop.Contains(exitPoint));
-					if (exitPoint.Visited) {
-						commonAncestor = commonAncestor.ImmediateDominator;
-						continue;
-					} else {
+					// It's possible that 'commonAncestor' is itself part of the natural loop.
+					// If so, it's not a valid exit point.
+					if (!exitPoint.Visited && ValidateExitPoint(loopHead, exitPoint)) {
+						// we found an exit point
 						return exitPoint;
 					}
+					commonAncestor = commonAncestor.ImmediateDominator;
 				}
 				// least common post-dominator is the artificial exit node
 				return null;
 			}
 		}
 
+		/// <summary>
+		/// Validates an exit point.
+		/// 
+		/// An exit point is invalid iff there is a node reachable from the exit point that
+		/// is dominated by the loop head, but not by the exit point.
+		/// (i.e. this method returns false iff the exit point's dominance frontier contains
+		/// a node dominated by the loop head. but we implement this the slow way because
+		/// we don't have dominance frontiers precomputed)
+		/// </summary>
+		/// <remarks>
+		/// We need this because it's possible that there's a return block (thus reverse-unreachable node ignored by post-dominance)
+		/// that is reachable both directly from the loop, and from the exit point.
+		/// </remarks>
+		bool ValidateExitPoint(ControlFlowNode loopHead, ControlFlowNode exitPoint)
+		{
+			var cfg = context.ControlFlowGraph;
+			return IsValid(exitPoint);
+
+			bool IsValid(ControlFlowNode node)
+			{
+				if (!cfg.HasReachableExit(node)) {
+					// Optimization: if the dominance frontier is empty, we don't need
+					// to check every node.
+					return true;
+				}
+				foreach (var succ in node.Successors) {
+					if (loopHead.Dominates(succ) && !exitPoint.Dominates(succ))
+						return false;
+				}
+				foreach (var child in node.DominatorTreeChildren) {
+					if (!IsValid(child))
+						return false;
+				}
+				return true;
+			}
+		}
+
 		/// <summary>
 		/// Pick exit point by picking any node that has no reachable exits.
 		///