From fb4872d7da746acd0065a8da5efd998adef0c651 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 2 Jun 2026 12:18:44 +0200 Subject: [PATCH] =?UTF-8?q?fix(bin):=20run=20interpreter=20on=20a=20large-?= =?UTF-8?q?stack=20thread=20to=20avoid=20deep-nesting=20overflow=20?= =?UTF-8?q?=F0=9F=8C=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deeply nested input (((…))) / [[[…]]] / {{{…}}}, long ---… / not … / 2^2^… chains, and nested calls walk the AST recursively in the parser, analyser, compiler, and VM, one native frame per level. On the default main-thread stack this overflowed and aborted the process (SIGABRT, not a catchable panic) at a few hundred levels. Run the interpreter on a worker thread with a large explicit stack. The interpreter uses Rc and isn't Send, but the closure creates, uses, and drops it on the worker thread, so nothing crosses the boundary. Input far deeper than any realistic source now evaluates instead of crashing. Closes #175 Co-Authored-By: Claude Opus 4.8 (1M context) --- ndc_bin/src/main.rs | 26 ++++++++++++++++++++ ndc_bin/tests/deep_nesting.rs | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 ndc_bin/tests/deep_nesting.rs diff --git a/ndc_bin/src/main.rs b/ndc_bin/src/main.rs index f0b3edc1..ac4b9ef0 100644 --- a/ndc_bin/src/main.rs +++ b/ndc_bin/src/main.rs @@ -150,7 +150,33 @@ impl TryFrom for Action { } } +/// Stack size for the worker thread that runs the interpreter. +/// +/// The parser, analyser, compiler, and VM all walk the AST recursively, one +/// native frame per nesting level. The default main-thread stack is small +/// enough that deeply (but legitimately) nested programs overflow it and abort +/// the process. A generous stack keeps that from happening for any realistic +/// source. The reservation is virtual address space; only the pages actually +/// touched cost memory. +const INTERPRETER_STACK_SIZE: usize = 512 * 1024 * 1024; + fn main() -> anyhow::Result<()> { + // Run everything on a worker thread with a large explicit stack. The + // interpreter uses `Rc` and is not `Send`, but it is created, used, and + // dropped entirely inside this closure, so nothing crosses the boundary. + let worker = std::thread::Builder::new() + .name("ndc-interpreter".into()) + .stack_size(INTERPRETER_STACK_SIZE) + .spawn(run) + .expect("failed to spawn interpreter thread"); + + match worker.join() { + Ok(result) => result, + Err(panic) => std::panic::resume_unwind(panic), + } +} + +fn run() -> anyhow::Result<()> { let cli = Cli::parse(); let action: Action = cli.command.unwrap_or_default().try_into()?; diff --git a/ndc_bin/tests/deep_nesting.rs b/ndc_bin/tests/deep_nesting.rs new file mode 100644 index 00000000..1909aed6 --- /dev/null +++ b/ndc_bin/tests/deep_nesting.rs @@ -0,0 +1,46 @@ +//! Regression test: deeply nested input must not crash the interpreter. +//! +//! Each nesting level costs a native stack frame in the parser and in every +//! later phase that walks the AST. On the default main-thread stack this used +//! to overflow and abort the process (SIGABRT) at a few hundred levels. `ndc` +//! now runs the interpreter on a worker thread with a large explicit stack, so +//! input far deeper than that evaluates normally. See +//! . + +// This integration test links `ndc_bin`'s dependencies but drives the built +// binary instead of calling them, so silence `unused-crate-dependencies`. +#![allow(unused_crate_dependencies)] + +use std::fs; +use std::process::Command; + +#[test] +fn deeply_nested_input_does_not_overflow_the_stack() { + // Deep enough to overflow the default main-thread stack (which gave up + // around a few hundred levels), shallow enough to fit the worker thread's + // large stack with room to spare. + let depth = 2000; + let source = format!("print({}1{})", "(".repeat(depth), ")".repeat(depth)); + + let path = std::env::temp_dir().join(format!("ndc_deep_nesting_{}.ndc", std::process::id())); + fs::write(&path, &source).expect("write temp script"); + + let output = Command::new(env!("CARGO_BIN_EXE_ndc")) + .arg(&path) + .output() + .expect("run ndc"); + + let _ = fs::remove_file(&path); + + assert!( + output.status.success(), + "ndc crashed on {depth}-deep nesting (status {:?}); a stack overflow would show up here.\nstderr:\n{}", + output.status, + String::from_utf8_lossy(&output.stderr), + ); + assert_eq!( + String::from_utf8_lossy(&output.stdout).trim(), + "1", + "the nested expression should evaluate to 1", + ); +}