Check the native stack guard in one function per call cycle

Every unbounded recursion repeats some cycle of the call graph, so a
stack check in one function of each cycle bounds it. stackCheckedFunctions
now picks a feedback vertex set of the stack-check graph greedily (the
function with the most caller-callee pairs first) instead of checking
every function on a cycle, and adds a check where a run of unchecked cycle
functions would exceed 32 calls, so the stack used between two checks
stays of the same order as before.

Worker entry polls follow: a synchronous function on a cycle that may
throw needs no entry poll, since a descent that never returns ends at a
checked function of its cycle. NoThrow contracts keep the model in which
every function on a cycle checks, a superset that does not depend on the
choice.

packages/compiler/src/backend/may-throw.test.ts

@@ -406,7 +406,7 @@ test("worker programs poll for termination only where execution can be unbounded
   expect(computeMayThrow({ ...mod, workers: false }).workerEntryPolls).toBeUndefined();
 });
 
-test("stack checks cover recursive components and recursion through reentrant operations", () => {
+test("stack checks cover every recursive cycle, including recursion through reentrant operations", () => {
   const mod = moduleWith(
     fn("caller", [call("self"), call("left"), call("leaf"), call("walker")], []),
     fn("self", [call("self"), call("leaf")], []),
@@ -420,7 +420,12 @@ test("stack checks cover recursive components and recursion through reentrant op
     fn("visitor", [callClosure("leaf")], []),
   );
   const answer = computeMayThrow(mod, { stackChecks: true });
-  expect([...answer.stackChecks!].sort()).toEqual(["left", "right", "self", "step", "walker"]);
+  // One check per cycle: self, one of left and right, and one of walker and
+  // step (the earlier function on ties).
+  expect([...answer.stackChecks!].sort()).toEqual(["left", "self", "walker"]);
+  expect(
+    [...computeMayThrow(mod, { stackChecks: "everyCycleMember" }).stackChecks!].sort(),
+  ).toEqual(["left", "right", "self", "step", "walker"]);
   // Checked functions throw the overflow RangeError, so their callers poll;
   // a throwing closure target makes every indirect call poll as well.
   expect(answer.indirect).toBe(true);
@@ -437,7 +442,7 @@ test("stack checks cover recursive components and recursion through reentrant op
   expect(computeMayThrow(mod).fns.size).toBe(0);
 });
 
-test("stack-checked synchronous functions skip the worker entry poll", () => {
+test("synchronous functions on a checked cycle skip the worker entry poll", () => {
   const asyncSelf: IrFunction = { ...fn("asyncSelf", [call("asyncSelf")], []), async: true };
   const mod: IrModule = {
     ...moduleWith(
@@ -452,10 +457,11 @@ test("stack-checked synchronous functions skip the worker entry poll", () => {
     workers: true,
   };
   const answer = computeMayThrow(mod, { stackChecks: true });
-  expect([...answer.stackChecks!].sort()).toEqual(["asyncSelf", "caller", "left", "right", "self"]);
-  // Callers test after every call of a stack-checked function, and its
-  // overflow unwinds through those tests, so only the module entry, async
-  // bodies and unbounded functions without a stack check poll on entry.
+  expect([...answer.stackChecks!].sort()).toEqual(["asyncSelf", "caller", "left", "self"]);
+  // Callers test after every call of a function on a checked cycle (it may
+  // throw), and an overflow unwinds through those tests, so only the module
+  // entry, async bodies and unbounded functions off any cycle poll on entry;
+  // right needs no poll although left holds the cycle's check.
   expect([...answer.workerEntryPolls!].sort()).toEqual(["asyncSelf", "caller", "walker"]);
   expect(answer.fns.has("self") && answer.fns.has("left") && answer.fns.has("right")).toBe(true);
   // Without stack checks (WASI, Windows, libraries) every cycle keeps its poll.
@@ -469,6 +475,48 @@ test("stack-checked synchronous functions skip the worker entry poll", () => {
   ]);
 });
 
+test("stack checks meet every cycle once and bound the runs between checks", () => {
+  // A ring of 80 functions, with a chord from fn10 back to fn0, and a
+  // separate ring of three whose functions also call each other backwards.
+  const ring = Array.from({ length: 80 }, (_, i) =>
+    fn(`fn${i}`, i === 10 ? [call("fn11"), call("fn0")] : [call(`fn${(i + 1) % 80}`)], []),
+  );
+  const mod = moduleWith(
+    ...ring,
+    fn("a", [call("b"), call("c")], []),
+    fn("b", [call("c"), call("a")], []),
+    fn("c", [call("a"), call("b")], []),
+  );
+  const checks = computeMayThrow(mod, { stackChecks: true }).stackChecks!;
+  // Removing the checked functions leaves no cycle, and no run of more than
+  // 32 unchecked calls.
+  const callees = new Map(
+    mod.functions.map((f) => [
+      f.name,
+      f.body.flatMap((s) =>
+        s.kind === "exprStmt" && s.expr.kind === "call" ? [s.expr.callee] : [],
+      ),
+    ]),
+  );
+  const longest = new Map<string, number>();
+  const visiting = new Set<string>();
+  const run = (name: string): number => {
+    if (checks.has(name)) return 0;
+    expect(visiting.has(name)).toBe(false);
+    const known = longest.get(name);
+    if (known !== undefined) return known;
+    visiting.add(name);
+    const depth = 1 + Math.max(0, ...callees.get(name)!.map(run));
+    visiting.delete(name);
+    longest.set(name, depth);
+    return depth;
+  };
+  for (const f of mod.functions) expect(run(f.name)).toBeLessThanOrEqual(32);
+  // fn0 breaks both ring cycles, fn33 and fn66 bound the run along the
+  // ring, and two of a, b and c break their four cycles.
+  expect([...checks].sort()).toEqual(["a", "b", "fn0", "fn33", "fn66"]);
+});
+
 test("class members are reachable from reentrant operations", () => {
   const mod: IrModule = {
     ...moduleWith(

packages/compiler/src/backend/may-throw.ts

@@ -37,7 +37,11 @@ export type MayThrowReason =
 export function computeMayThrow(
   mod: IrModule,
   options: {
-    stackChecks?: boolean;
+    /** Native stack checks: true for the functions that check on entry (one
+     * per call cycle, see stackCheckedFunctions); "everyCycleMember" for a
+     * model in which every function on a cycle checks, which answers a
+     * superset that does not depend on that choice. */
+    stackChecks?: boolean | "everyCycleMember";
     explain?: boolean;
     /** computePublishedTypes(mod), when the caller already has it. */
     published?: ReturnType<typeof computePublishedTypes>;
@@ -56,8 +60,16 @@ export function computeMayThrow(
   stackChecks?: Set<string>;
 } {
   const terminations = mod.workers === true ? workerTerminationPolls(mod, options.nodes) : null;
+  const stackGraph =
+    options.stackChecks === undefined || options.stackChecks === false
+      ? null
+      : stackCheckGraph(mod, options.nodes);
   const stackChecks =
-    options.stackChecks === true ? stackCheckedFunctions(mod, options.nodes) : null;
+    stackGraph === null
+      ? null
+      : options.stackChecks === "everyCycleMember"
+        ? stackGraph.cyclic
+        : cycleGuards(stackGraph);
   // Stores that can reach a published object throw the frozen TypeError.
   const published =
     options.published !== undefined ? options.published : computePublishedTypes(mod);
@@ -488,7 +500,9 @@ export function computeMayThrow(
     fns: may,
     ...(why !== null ? { why } : {}),
     indirect,
-    ...(terminations ? { workerEntryPolls: entryPolls(mod, terminations.entry, stackChecks) } : {}),
+    ...(terminations
+      ? { workerEntryPolls: entryPolls(mod, terminations.entry, stackGraph?.cyclic ?? null, may) }
+      : {}),
     ...(stackChecks ? { stackChecks } : {}),
   };
 }
@@ -720,28 +734,32 @@ function describeThrowingNode(rec: IrExpr | IrStmt): string {
 }
 
 /** The termination polls a worker program places on function entry. A
- * stack-checked synchronous function needs none: it may throw (the stack
- * guard seeds the fixpoint), so every caller tests the pending state after
- * the call, and a descent that never returns ends at the stack guard, whose
- * RangeError unwinds through those tests. An outstanding terminate request
- * keeps the alert word nonzero, so the first test that runs afterwards (a
- * post-call test, a loop poll or the unwinding) stops the thread; dropping
- * the entry poll only delays the stop by bounded work, which Node's
- * asynchronous terminate allows. The entry poll's slow path rejoins the
- * body, so every polled function kept its arguments live across a call. */
+ * synchronous function on a call cycle that may throw needs none: every
+ * caller tests the pending state after the call, and a descent that never
+ * returns passes a stack-checked function of each cycle again and again
+ * (stackCheckedFunctions) and ends at its guard, whose RangeError unwinds
+ * through those tests. An outstanding terminate request keeps the alert word
+ * nonzero, so the first test that runs afterwards (a post-call test, a loop
+ * poll or the unwinding) stops the thread; dropping the entry poll only
+ * delays the stop by bounded work, which Node's asynchronous terminate
+ * allows. The entry poll's slow path rejoins the body, so every polled
+ * function kept its arguments live across a call. */
 function entryPolls(
   mod: IrModule,
   entry: Set<string>,
-  stackChecks: Set<string> | null,
+  cyclic: Set<string> | null,
+  may: Set<string>,
 ): Set<string> {
-  if (stackChecks === null) return entry;
+  if (cyclic === null) return entry;
   // Calling an async or generator function never tests the caller's pending
   // state, and the module entry has no caller: those keep their polls.
   const keep = new Set(
     mod.functions.filter((fn) => fn.async || fn.generator !== undefined).map((fn) => fn.name),
   );
   keep.add(mod.entry);
-  return new Set([...entry].filter((name) => keep.has(name) || !stackChecks.has(name)));
+  return new Set(
+    [...entry].filter((name) => keep.has(name) || !cyclic.has(name) || !may.has(name)),
+  );
 }
 
 /** Statement and expression kinds that cannot reenter scriptc code except
@@ -971,16 +989,31 @@ export function workerTerminationPolls(
   return { entry, polls };
 }
 
-/** The functions that can take part in unbounded recursion, which check the
- * native stack guard on entry: every function on a cycle of the call graph.
- * Direct edges are calls, constructions and virtual dispatch. A function
- * containing any operation that can reenter scriptc code other than through
- * those edges (an indirect call, a callback-taking or dynamic operation, a
- * library call; see boundedNode) also reaches every function that such an
- * operation can invoke: closure targets and class members. Non-recursive
- * functions, including leaf callbacks, stay check-free. */
+/** The call graph that bounds recursion, and the functions on its cycles:
+ * the ones that can take part in unbounded recursion. Direct edges are
+ * calls, constructions and virtual dispatch. A function containing any
+ * operation that can reenter scriptc code other than through those edges
+ * (an indirect call, a callback-taking or dynamic operation, a library call;
+ * see boundedNode) also reaches every function that such an operation can
+ * invoke: closure targets and class members, behind one shared `reentry`
+ * node. */
+interface StackCheckGraph {
+  edges: Map<string, string[]>;
+  cyclic: Set<string>;
+}
+
+const REENTRY = "\0reentry";
+
+/** The functions that check the native stack guard on entry: one function
+ * of every cycle of the call graph (stackCheckGraph), which bounds every
+ * unbounded recursion, since its frames repeat the cycle's guarded function
+ * again and again. Non-recursive functions, including leaf callbacks, stay
+ * check-free. See cycleGuards for the choice. */
 export function stackCheckedFunctions(mod: IrModule, nodes?: IrNodeIndex): Set<string> {
-  const reentry = "\0reentry";
+  return cycleGuards(stackCheckGraph(mod, nodes));
+}
+
+function stackCheckGraph(mod: IrModule, nodes?: IrNodeIndex): StackCheckGraph {
   const methodImpls = new Map<string, string[]>();
   const memberPrefixes = new Set<string>();
   for (const cls of mod.classes ?? []) {
@@ -1019,16 +1052,157 @@ export function stackCheckedFunctions(mod: IrModule, nodes?: IrNodeIndex): Set<s
     };
     eachNodeOfKinds(fn, scan, visit);
     const callees = out.filter((callee) => names.has(callee));
-    if (reenters) callees.push(reentry);
+    if (reenters) callees.push(REENTRY);
     edges.set(fn.name, callees);
   }
   edges.set(
-    reentry,
+    REENTRY,
     [...targets].filter((name) => names.has(name)),
   );
   const cyclic = cyclicFunctions(edges);
-  cyclic.delete(reentry);
-  return cyclic;
+  cyclic.delete(REENTRY);
+  return { edges, cyclic };
+}
+
+/** The longest run of calls between unchecked functions of a cycle that
+ * cycleGuards allows; deeper runs get a check. Acyclic call chains below a
+ * check are as deep as the program makes them already, so this keeps the
+ * stack used between two checks of the same order. */
+const MAX_UNCHECKED_CYCLE_RUN = 32;
+
+/** A small set of functions that meets every cycle of the stack-check graph
+ * (a feedback vertex set), chosen greedily: functions that cannot lie on a
+ * remaining cycle (no caller or no callee left among the candidates) drop
+ * out, a function that calls itself is chosen, and otherwise the function
+ * with the most caller-callee pairs, the one that breaks the most cycles,
+ * is chosen and removed; ties go to the earlier function. The reentry node
+ * is not a function and is never chosen. A last pass in topological order
+ * of the unchosen cycle functions adds a check where a run of them would
+ * exceed MAX_UNCHECKED_CYCLE_RUN calls. */
+function cycleGuards(graph: StackCheckGraph): Set<string> {
+  const { edges, cyclic } = graph;
+  const chosen = new Set<string>();
+  if (cyclic.size === 0) return chosen;
+  const order = new Map<string, number>();
+  const callees = new Map<string, Set<string>>();
+  const callers = new Map<string, Set<string>>();
+  const candidates = [...[...edges.keys()].filter((name) => cyclic.has(name)), REENTRY];
+  for (const name of candidates) {
+    order.set(name, order.size);
+    callees.set(name, new Set());
+    callers.set(name, new Set());
+  }
+  for (const name of candidates)
+    for (const callee of edges.get(name) ?? [])
+      if (callees.has(callee)) {
+        callees.get(name)!.add(callee);
+        callers.get(callee)!.add(name);
+      }
+  // Max-heap of [score, order, name]; an entry is stale once its function
+  // is gone or its score changed, and is skipped when popped.
+  const heap: [number, number, string][] = [];
+  const score = (name: string): number =>
+    name === REENTRY ? -1 : callers.get(name)!.size * callees.get(name)!.size;
+  const above = (x: [number, number, string], y: [number, number, string]): boolean =>
+    x[0] > y[0] || (x[0] === y[0] && x[1] < y[1]);
+  const push = (name: string): void => {
+    heap.push([score(name), order.get(name)!, name]);
+    for (let i = heap.length - 1; i > 0;) {
+      const parent = (i - 1) >> 1;
+      if (!above(heap[i]!, heap[parent]!)) break;
+      [heap[i], heap[parent]] = [heap[parent]!, heap[i]!];
+      i = parent;
+    }
+  };
+  const pop = (): [number, number, string] => {
+    const top = heap[0]!;
+    const last = heap.pop()!;
+    if (heap.length > 0) {
+      heap[0] = last;
+      for (let i = 0; ;) {
+        let best = i;
+        for (const child of [2 * i + 1, 2 * i + 2])
+          if (child < heap.length && above(heap[child]!, heap[best]!)) best = child;
+        if (best === i) break;
+        [heap[i], heap[best]] = [heap[best]!, heap[i]!];
+        i = best;
+      }
+    }
+    return top;
+  };
+  const doomed: string[] = [];
+  const remove = (name: string): void => {
+    const out = callees.get(name)!;
+    const into = callers.get(name)!;
+    callees.delete(name);
+    callers.delete(name);
+    for (const callee of out)
+      if (callee !== name) {
+        const set = callers.get(callee)!;
+        set.delete(name);
+        if (set.size === 0) doomed.push(callee);
+        else push(callee);
+      }
+    for (const caller of into)
+      if (caller !== name) {
+        const set = callees.get(caller)!;
+        set.delete(name);
+        if (set.size === 0) doomed.push(caller);
+        else push(caller);
+      }
+  };
+  // Removes the functions that can no longer lie on a cycle, transitively.
+  const settle = (): void => {
+    while (doomed.length > 0) {
+      const name = doomed.pop()!;
+      if (callees.has(name)) remove(name);
+    }
+  };
+  for (const name of candidates) {
+    if (callees.get(name)!.size === 0 || callers.get(name)!.size === 0) doomed.push(name);
+    else push(name);
+  }
+  settle();
+  for (const name of candidates)
+    if (callees.get(name)?.has(name) === true) {
+      chosen.add(name);
+      remove(name);
+      settle();
+    }
+  while (heap.length > 0) {
+    const [entryScore, , name] = pop();
+    if (!callees.has(name) || entryScore !== score(name) || name === REENTRY) continue;
+    chosen.add(name);
+    remove(name);
+    settle();
+  }
+  // Bound the runs of unchecked cycle functions (acyclic once the chosen
+  // ones are removed; the reentry node takes no frame): the longest run
+  // ending at each function, in topological order, and a check where it
+  // would grow too long.
+  const unchecked = candidates.filter((name) => !chosen.has(name));
+  const pending = new Map<string, number>(unchecked.map((name) => [name, 0]));
+  for (const name of unchecked)
+    for (const callee of new Set(edges.get(name)))
+      if (pending.has(callee)) pending.set(callee, pending.get(callee)! + 1);
+  const run = new Map<string, number>();
+  const ready = unchecked.filter((name) => pending.get(name) === 0);
+  for (let i = 0; i < ready.length; i++) {
+    const name = ready[i]!;
+    let depth = (run.get(name) ?? 0) + (name === REENTRY ? 0 : 1);
+    if (depth > MAX_UNCHECKED_CYCLE_RUN && name !== REENTRY) {
+      chosen.add(name);
+      depth = 0;
+    }
+    for (const callee of new Set(edges.get(name))) {
+      if (!pending.has(callee)) continue;
+      run.set(callee, Math.max(run.get(callee) ?? 0, depth));
+      const left = pending.get(callee)! - 1;
+      pending.set(callee, left);
+      if (left === 0) ready.push(callee);
+    }
+  }
+  return chosen;
 }
 
 /** Functions on a cycle of the call graph (Tarjan's strongly connected

packages/compiler/src/contracts/check.ts

@@ -519,8 +519,10 @@ class ContractChecker {
           for (const fn of this.fnByLoc.get(key) ?? []) targets.push({ fn, note });
     if (targets.length === 0) return;
     // Target-independent: the native stack guard (Linux, macOS) counts, so a
-    // contract holds on every platform.
-    const mt = computeMayThrow(this.mod, { stackChecks: true, explain: true });
+    // contract holds on every platform. Every function on a call cycle counts
+    // as checking it, a superset of the functions the backend picks, so the
+    // answer does not depend on that choice.
+    const mt = computeMayThrow(this.mod, { stackChecks: "everyCycleMember", explain: true });
     const proven: string[] = [];
     for (const { fn, note } of targets) {
       if (fn.async === true || fn.generator !== undefined) {

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8370-stack-overflow-cycles/main.ts.json

@@ -0,0 +1,7 @@
+{
+ "order": [
+  "<repo>/tests/corpus/8370-stack-overflow-cycles/shapes.ts",
+  "<repo>/tests/corpus/8370-stack-overflow-cycles/main.ts"
+ ],
+ "diags": []
+}

tests/corpus/8370-stack-overflow-cycles/main.ts

@@ -0,0 +1,69 @@
+// Unbounded recursion through each kind of call cycle raises Node's
+// RangeError, and the program recovers and recurses again afterwards.
+import { Even, Odd, Tree } from "./shapes.ts";
+
+function probe(name: string, run: (n: number) => number): void {
+  try {
+    console.log(name, "returned", run(10_000_000));
+  } catch (e) {
+    console.log(name, e instanceof RangeError, (e as Error).message);
+  }
+  console.log(name, "after", run(100));
+}
+
+function down(n: number): number {
+  return n === 0 ? 0 : 1 + down(n - 1);
+}
+probe("direct", down);
+
+function ping(n: number): number {
+  return n === 0 ? 0 : 1 + pong(n - 1);
+}
+function pong(n: number): number {
+  return n === 0 ? 0 : 2 + ping(n - 1);
+}
+probe("mutual", ping);
+
+function first(n: number): number {
+  return n === 0 ? 0 : 1 + second(n - 1);
+}
+function second(n: number): number {
+  return n === 0 ? 0 : 1 + third(n - 1);
+}
+function third(n: number): number {
+  return n === 0 ? 0 : 1 + fourth(n - 1);
+}
+function fourth(n: number): number {
+  return n === 0 ? 0 : 1 + fifth(n - 1);
+}
+function fifth(n: number): number {
+  return n === 0 ? 0 : 1 + first(n - 1);
+}
+probe("ring", first);
+
+function viaValue(n: number, again: (m: number) => number): number {
+  return n === 0 ? 0 : 1 + again(n - 1);
+}
+const callback = (m: number): number => viaValue(m, callback);
+probe("closure", callback);
+
+function viaMap(n: number): number {
+  return n === 0 ? 0 : 1 + [n - 1].map(viaMap)[0]!;
+}
+probe("library callback", viaMap);
+
+const even = new Even();
+const odd = new Odd();
+even.next = odd;
+odd.next = even;
+probe("virtual", (n) => even.depth(n));
+
+probe("constructor", (n) => {
+  let tree: Tree | null = new Tree(n);
+  let count = 0;
+  while (tree !== null) {
+    count++;
+    tree = tree.child;
+  }
+  return count;
+});

tests/corpus/8370-stack-overflow-cycles/shapes.ts

@@ -0,0 +1,24 @@
+// Classes in their own module, so their names carry the module prefix.
+export abstract class Link {
+  next: Link | null = null;
+  abstract depth(n: number): number;
+}
+
+export class Even extends Link {
+  depth(n: number): number {
+    return n === 0 ? 0 : 1 + this.next!.depth(n - 1);
+  }
+}
+
+export class Odd extends Link {
+  depth(n: number): number {
+    return n === 0 ? 0 : 2 + this.next!.depth(n - 1);
+  }
+}
+
+export class Tree {
+  child: Tree | null;
+  constructor(n: number) {
+    this.child = n === 0 ? null : new Tree(n - 1);
+  }
+}

tests/harness/corpus-costs/8370-stack-overflow-cycles/main.ts.json

@@ -0,0 +1 @@
+{ "plain": 2780, "san": 53539 }