From 52089d3564c145a3ce1b92f0b720b5a22be5e9cb Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Thu, 24 Sep 2026 03:16:59 +0000
Subject: [PATCH] Undo: free and search the undo tree iteratively

Recursion depth equalled history length, so destroying or clearing a
buffer with a few hundred thousand undo steps overflowed the stack.
UndoSystem::clear() also freed only the first root branch, leaking the
root's siblings; it now frees the whole forest.

Audit finding M4.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB
---
 UndoSystem.cc | 53 +++++++++++++++++++++++++--------------------------
 UndoTree.cc   | 27 ++++++++++++++++----------
 UndoTree.h    |  5 +++++
 3 files changed, 48 insertions(+), 37 deletions(-)

diff --git a/UndoSystem.cc b/UndoSystem.cc
index 09f40a6..aad60c7 100644
--- a/UndoSystem.cc
+++ b/UndoSystem.cc
@@ -2,6 +2,7 @@
 #include "Buffer.h"
 #include <cassert>
 #include <cstdio>
+#include <vector>
 
 
 UndoSystem::UndoSystem(Buffer &owner, UndoTree &tree)
@@ -260,7 +261,8 @@ void
 UndoSystem::clear()
 {
 	discard_pending();
-	free_node(tree_.root);
+	// The root may have siblings (edits made after undoing to the start).
+	FreeUndoForest(tree_.root);
 	tree_.root       = nullptr;
 	tree_.current    = nullptr;
 	tree_.saved      = nullptr;
@@ -343,20 +345,13 @@ UndoSystem::apply(const UndoNode *node, int direction)
 void
 UndoSystem::free_node(UndoNode *node)
 {
+	// Free this node and its child subtree, but not its siblings.
 	if (!node)
 		return;
-	// Free child subtree(s) and sibling branches
-	if (node->child) {
-		// Free entire redo list starting at child, including each subtree
-		UndoNode *branch = node->child;
-		while (branch) {
-			UndoNode *next = branch->next;
-			free_node(branch);
-			branch = next;
-		}
-		node->child = nullptr;
-	}
+	UndoNode *children = node->child;
+	node->child        = nullptr;
 	delete node;
+	FreeUndoForest(children);
 }
 
 
@@ -364,26 +359,27 @@ void
 UndoSystem::free_branch(UndoNode *node)
 {
 	// Free a branch list (node and its next siblings) including their subtrees
-	while (node) {
-		UndoNode *next = node->next;
-		free_node(node);
-		node = next;
-	}
+	FreeUndoForest(node);
 }
 
 
+// Iterative search for `target` below `cur`; sets out_parent to its parent.
 static bool
 dfs_find_parent(UndoNode *cur, UndoNode *target, UndoNode *&out_parent)
 {
 	if (!cur)
 		return false;
-	for (UndoNode *child = cur->child; child != nullptr; child = child->next) {
-		if (child == target) {
-			out_parent = cur;
-			return true;
+	std::vector<UndoNode *> stack{cur};
+	while (!stack.empty()) {
+		UndoNode *node = stack.back();
+		stack.pop_back();
+		for (UndoNode *child = node->child; child != nullptr; child = child->next) {
+			if (child == target) {
+				out_parent = node;
+				return true;
+			}
+			stack.push_back(child);
 		}
-		if (dfs_find_parent(child, target, out_parent))
-			return true;
 	}
 	return false;
 }
@@ -447,11 +443,14 @@ UndoSystem::is_descendant(UndoNode *root, const UndoNode *target)
 {
 	if (!root || !target)
 		return false;
-	if (root == target)
-		return true;
-	for (UndoNode *child = root->child; child != nullptr; child = child->next) {
-		if (is_descendant(child, target))
+	std::vector<UndoNode *> stack{root};
+	while (!stack.empty()) {
+		UndoNode *node = stack.back();
+		stack.pop_back();
+		if (node == target)
 			return true;
+		for (UndoNode *child = node->child; child != nullptr; child = child->next)
+			stack.push_back(child);
 	}
 	return false;
 }
diff --git a/UndoTree.cc b/UndoTree.cc
index bf4d449..d45cba0 100644
--- a/UndoTree.cc
+++ b/UndoTree.cc
@@ -1,24 +1,31 @@
 // Undo logic is implemented in UndoSystem; this file only owns node lifetime.
 #include "UndoTree.h"
 
-namespace {
+#include <vector>
+
+
 void
-free_node_graph(UndoNode *node)
+FreeUndoForest(UndoNode *first)
 {
-	// Walk the sibling (redo-branch) list; for each node, recursively free its
-	// child subtree first, then the node itself.
-	while (node) {
-		UndoNode *next = node->next;
-		free_node_graph(node->child);
+	// Nodes form a first-child/next-sibling tree whose depth equals the
+	// length of the edit history, so walk it with an explicit stack.
+	std::vector<UndoNode *> stack;
+	if (first)
+		stack.push_back(first);
+	while (!stack.empty()) {
+		UndoNode *node = stack.back();
+		stack.pop_back();
+		if (node->child)
+			stack.push_back(node->child);
+		if (node->next)
+			stack.push_back(node->next);
 		delete node;
-		node = next;
 	}
 }
-} // namespace
 
 
 UndoTree::~UndoTree()
 {
-	free_node_graph(root);
+	FreeUndoForest(root);
 	delete pending;
 }
diff --git a/UndoTree.h b/UndoTree.h
index 034b315..bafdec9 100644
--- a/UndoTree.h
+++ b/UndoTree.h
@@ -15,3 +15,8 @@ struct UndoTree {
 	// created for it.
 	~UndoTree();
 };
+
+
+// Frees `first`, its whole child subtree, and every node on its `next`
+// (sibling) chain. Iterative, so history depth cannot overflow the stack.
+void FreeUndoForest(UndoNode *first);