Fix #3075: eliminate recursion in TopologicalSort and BlockTransform.

ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj

@@ -658,6 +658,7 @@
     <Compile Include="Util\CSharpPrimitiveCast.cs" />
     <Compile Include="Util\EmptyList.cs" />
     <Compile Include="Util\ExtensionMethods.cs" />
+    <Compile Include="Util\GraphTraversal.cs" />
     <Compile Include="Util\Interval.cs" />
     <Compile Include="Util\LazyInit.cs" />
     <Compile Include="Util\LongSet.cs" />

ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs

@@ -279,7 +279,7 @@ namespace ICSharpCode.Decompiler.IL
 			// Visit blocks in post-order
 			BitSet visited = new BitSet(Blocks.Count);
 			List<Block> postOrder = new List<Block>();
-			Visit(EntryPoint);
+			GraphTraversal.DepthFirstSearch(new[] { EntryPoint }, Successors, postOrder.Add, MarkAsVisited, reverseSuccessors: true);
 			postOrder.Reverse();
 			if (!deleteUnreachableBlocks)
 			{
@@ -291,24 +291,30 @@ namespace ICSharpCode.Decompiler.IL
 			}
 			return postOrder;
 
-			void Visit(Block block)
+			bool MarkAsVisited(Block block)
 			{
 				Debug.Assert(block.Parent == this);
 				if (!visited[block.ChildIndex])
 				{
 					visited[block.ChildIndex] = true;
+					return true;
+				}
+				else
+				{
+					return false;
+				}
+			}
 
-					foreach (var branch in block.Descendants.OfType<Branch>())
+			IEnumerable<Block> Successors(Block block)
+			{
+				foreach (var branch in block.Descendants.OfType<Branch>())
+				{
+					if (branch.TargetBlock.Parent == this)
 					{
-						if (branch.TargetBlock.Parent == this)
+						yield return branch.TargetBlock;
-						{
-							Visit(branch.TargetBlock);
-						}
 					}
-
-					postOrder.Add(block);
 				}
-			};
+			}
 		}
 
 		/// <summary>

ICSharpCode.Decompiler/IL/Transforms/BlockTransform.cs

@@ -5,6 +5,7 @@ using System.Linq;
 
 using ICSharpCode.Decompiler.FlowAnalysis;
 using ICSharpCode.Decompiler.IL.ControlFlow;
+using ICSharpCode.Decompiler.Util;
 
 namespace ICSharpCode.Decompiler.IL.Transforms
 {
@@ -105,29 +106,36 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			}
 		}
 
-		void VisitBlock(ControlFlowNode cfgNode, BlockTransformContext context)
+		/// <summary>
+		/// Walks the dominator tree rooted at entryNode, calling the transforms on each block.
+		/// </summary>
+		void VisitBlock(ControlFlowNode entryNode, BlockTransformContext context)
 		{
-			Block block = (Block)cfgNode.UserData;
+			IEnumerable<ControlFlowNode> Preorder(ControlFlowNode cfgNode)
-			context.StepStartGroup(block.Label, block);
-
-			context.ControlFlowNode = cfgNode;
-			context.Block = block;
-			context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count;
-			block.RunTransforms(PreOrderTransforms, context);
-
-			// First, process the children in the dominator tree.
-			// The ConditionDetection transform requires dominated blocks to
-			// be already processed.
-			foreach (var child in cfgNode.DominatorTreeChildren)
 			{
-				VisitBlock(child, context);
+				// preorder processing:
+				Block block = (Block)cfgNode.UserData;
+				context.StepStartGroup(block.Label, block);
+
+				context.ControlFlowNode = cfgNode;
+				context.Block = block;
+				context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count;
+				block.RunTransforms(PreOrderTransforms, context);
+
+				// process the children
+				return cfgNode.DominatorTreeChildren;
 			}
 
-			context.ControlFlowNode = cfgNode;
+			foreach (var cfgNode in TreeTraversal.PostOrder(entryNode, Preorder))
-			context.Block = block;
+			{
-			context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count;
+				// in post-order:
-			block.RunTransforms(PostOrderTransforms, context);
+				Block block = (Block)cfgNode.UserData;
-			context.StepEndGroup();
+				context.ControlFlowNode = cfgNode;
+				context.Block = block;
+				context.IndexOfFirstAlreadyTransformedInstruction = block.Instructions.Count;
+				block.RunTransforms(PostOrderTransforms, context);
+				context.StepEndGroup();
+			}
 		}
 	}
 }

ICSharpCode.Decompiler/Util/GraphTraversal.cs

@@ -0,0 +1,115 @@
+// Copyright (c) 2023 Daniel Grunwald
+// 
+// Permission is hereby granted, free of charge, to any person obtaining a copy of this
+// software and associated documentation files (the "Software"), to deal in the Software
+// without restriction, including without limitation the rights to use, copy, modify, merge,
+// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
+// to whom the Software is furnished to do so, subject to the following conditions:
+// 
+// The above copyright notice and this permission notice shall be included in all copies or
+// substantial portions of the Software.
+// 
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
+// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
+// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+// DEALINGS IN THE SOFTWARE.
+
+#nullable enable
+
+using System;
+using System.Collections.Generic;
+
+namespace ICSharpCode.Decompiler.Util;
+
+static class GraphTraversal
+{
+	/// <summary>
+	/// Depth-first-search of an graph data structure.
+	/// The two callbacks (successorFunc + postorderAction) will be called exactly once for each node reachable from startNodes.
+	/// </summary>
+	/// <param name="startNodes">The start nodes.</param>
+	/// <param name="visitedFunc">Called multiple times per node. The first call should return true, subsequent calls must return false.
+	/// If this function is not provided, normal Equals/GetHashCode will be used to compare nodes.</param>
+	/// <param name="successorFunc">The function that gets the successors of an element. Called in pre-order.</param>
+	/// <param name="postorderAction">Called in post-order.</param>
+	/// <param name="reverseSuccessors">
+	/// With reverse_successors=True, the start_nodes and each list of successors will be handled in reverse order.
+	/// This is useful if the post-order will be reversed later (e.g. for a topological sort)
+	/// so that blocks which could be output in either order (e.g. then-block and else-block of an if)
+	/// will maintain the order of the edges (then-block before else-block).
+	/// </param>
+	public static void DepthFirstSearch<T>(IEnumerable<T> startNodes, Func<T, IEnumerable<T>?> successorFunc, Action<T> postorderAction, Func<T, bool>? visitedFunc = null, bool reverseSuccessors = false)
+	{
+		/*
+        Pseudocode:
+            def dfs_walk(start_nodes, successor_func, visited, postorder_func, reverse_successors):
+                if reverse_successors:
+                    start_nodes = reversed(start_nodes)
+                for node in start_nodes:
+                    if node in visited: continue
+                    visited.insert(node)
+                    children = successor_func(node)
+                    dfs_walk(children, successor_func, visited, postorder_action, reverse_successors)
+                    postorder_action(node)
+        
+        The actual implementation here is equivalent but does not use recursion,
+        so that we don't blow the stack on large graphs.
+        A single stack holds the "continuations" of work that needs to be done.
+        These can be either "visit continuations" (=execute the body of the pseudocode
+        loop for the given node) or "postorder continuations" (=execute postorder_action)
+        */
+		// Use a List as stack (but allowing for the Reverse() usage)
+		var worklist = new List<(T node, bool isPostOrderContinuation)>();
+		visitedFunc ??= new HashSet<T>().Add;
+		foreach (T node in startNodes)
+		{
+			worklist.Add((node, false));
+		}
+		if (!reverseSuccessors)
+		{
+			// Our use of a stack will reverse the order of the nodes.
+			// If that's not desired, restore original order by reversing twice.
+			worklist.Reverse();
+		}
+		// Process outstanding continuations:
+		while (worklist.Count > 0)
+		{
+			var (node, isPostOrderContinuation) = worklist.Last();
+			if (isPostOrderContinuation)
+			{
+				// Execute postorder_action
+				postorderAction(node);
+				worklist.RemoveAt(worklist.Count - 1);
+				continue;
+			}
+			// Execute body of loop
+			if (!visitedFunc(node))
+			{
+				// Already visited
+				worklist.RemoveAt(worklist.Count - 1);
+				continue;
+			}
+			// foreach-loop-iteration will end with postorder_func call,
+			// so switch the type of continuation for this node
+			int oldWorkListSize = worklist.Count;
+			worklist[oldWorkListSize - 1] = (node, true);
+			// Create "visit continuations" for all successor nodes:
+			IEnumerable<T>? children = successorFunc(node);
+			if (children != null)
+			{
+				foreach (T child in children)
+				{
+					worklist.Add((child, false));
+				}
+			}
+			// Our use of a stack will reverse the order of the nodes.
+			// If that's not desired, restore original order by reversing twice.
+			if (!reverseSuccessors)
+			{
+				worklist.Reverse(oldWorkListSize, worklist.Count - oldWorkListSize);
+			}
+		}
+	}
+}