Rewrite the bytecode VM onto an explicit heap frame stack - #59
Merged
Conversation
Native C++ recursion between compiled chunks (function calls, and now module calls too) is replaced with an explicit, heap-allocated call stack (Evaluator::vmCallStack_) serviced by one driver loop (driveVm), so a compiled-to-compiled recursive chain no longer grows the native stack at all -- bounded instead by a new kMaxVmCallStackDepth (1,000,000), not the native-stack-safety kMaxUserCallDepth (50) that used to cap every recursive call regardless of shape. - Functions: Op::CallFn/CallFnTail/CallDynamic/CallDynamicTail push/ replace explicit VmFrames instead of recursing natively or through the old runCompiledFunctionTrampoline (retired, along with TailCallRequest). - Modules: bodies compile too (Op::CallModule, plus NativeStatement/ NativeCondJumpIfFalse/NativeIterMaterialize/ForIterNext/ForIterEnd for control flow) -- no trampoline ever existed for modules, so a recursive tree/pattern-generator module used to segfault around depth ~5000 on an ordinary build. if/for get real Jump-based bytecode (so a recursive module call nested inside one doesn't hide behind a native call boundary); everything else (assignment, echo, assert, let-blocks, modifiers, intersection_for, a builtin/ unresolved module call) stays a native passthrough for one statement at a time -- those are leaf-shaped, never the recursion risk this targets. - Fixed an O(depth^2) perf cliff this surfaced: Evaluator::callCtxFor's closure/lexical-nesting detection used to scan the full callStack_ on every call. Replaced with activeDeclRefcount_, a refcounted set of DISTINCT active declarations (a deep call chain is overwhelmingly the same declaration repeated, not many different ones) -- ~50x faster at depth 40,000, and scaling is linear again. 695 tests green under both OSCAD_BYTECODE_VM on and off. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
FastContinueNotHookSkippableStillFiresEveryCheckpoint's own expected count (4, not 7) depends on the compiled path actually running (translate()'s own [1,0,0] argument becomes compile-eligible, collapsing 3 sub-expression stops) -- it never forced VM state, just relied on the process default, so it silently broke whenever CI (or anyone locally) ran the suite with OSCAD_BYTECODE_VM=0. Caught by PR #59's own CI (macOS/Ubuntu legs both failed on exactly this). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Factoring evalUserModule's own native-fallback body into runModuleBodyNative (shared with Op::CallModule's own native-fallback branch, so neither duplicates the enterUserCall/evalChildren/ exitUserCall bracket) added one sustained extra native stack frame to every level of INTERPRETED module recursion -- the one recursive shape this cap still protects (compiled module recursion runs through the explicit vmCallStack_ instead and never touches this guard at all). 50 was tight enough on Windows CI specifically that this alone segfaulted UserModule.DeepNonTailRecursionHitsAControlledError Interpreted instead of throwing cleanly at the guard (PR #59, both macOS/Ubuntu CI legs already passed; Windows didn't). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Stage 2 made deep recursive module chains resolve successfully, but that only bounds how many CALLS happen -- not how deep the CSGNode TREE those calls build ends up being. A module-call chain that wraps a single spliced child stays tree-depth 1 no matter how deep it recurses (evalModularCall's own splice logic collapses it away), but an ordinary "chain/spiral of shapes" pattern -- adding its own incremental geometry each level, e.g. `cube(0.1); recur(n-1);` -- makes each level's own splice see >1 child, so it gets wrapped in a synthetic union CSGNode instead, and the tree genuinely grows one level per call. Before this, such a chain resolved fine (confirmed directly: a standalone probe showed resolveTree() itself returning successfully) and then segfaulted the instant the resulting tree went out of scope -- std::unique_ptr<CSGNode>'s own default recursive destructor is an un-guardable native-stack risk once a tree that deep exists at all. A destructor can't throw a catchable error and a real stack overflow isn't a C++ exception, so the only fix is to stop the tree from ever getting that deep in the first place -- checked via a new CSGNode::treeDepth field, set and validated (Evaluator:: setTreeDepthOrThrow, kMaxCsgTreeDepth=2000) at every site that finalizes a node's own children (buildTreeNode, evalModularCall's non-splice tail, spliceModuleChildren's union-wrap branch), not by a generate-pass or destructor-side walk (an earlier version of this fix tried exactly that and didn't work -- the crash isn't even inside generateTreeImpl). 697 tests green under both OSCAD_BYTECODE_VM on and off. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A tail-recursive chain mutates one CallStackFrame in place per hop, so its trace was already small regardless of depth. A non-tail chain has no such collapsing -- every call is a real, distinct frame -- and Stage 1/2's explicit-stack VM means that chain can now legitimately run thousands deep. traceLines() printed one "TRACE:" pair per frame unconditionally, turning a single error into a multi-megabyte message (confirmed directly: the CSG-tree-depth guard's own error, PR #59). Now shows the innermost 10 frames (closest to the actual error) and outermost 10 (the original entry point), with one "... N more frames ..." marker in between once the stack exceeds 20 frames -- unchanged for every existing shallow trace. Deliberate departure from the Python reference (traceLines mirrors its own _trace_lines): Python's recursion limit makes a callStack this deep unreachable there. 698 tests green under both OSCAD_BYTECODE_VM on and off. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
5 tasks
revarbat
added a commit
that referenced
this pull request
Jul 31, 2026
) evalChildren() now tries compiling its whole statement list (not just its leading assignment run) before falling back to the native per-statement loop, via tryRunCompiledChildren/tryCompileChildrenList. This lets a resolvable module call inside a builtin's own children (e.g. `translate(v) recur(n-1);`) get real Op::CallModule bytecode instead of always falling through evalStatement/evalModularCall. That fix reintroduced a native C++ recursion path that PR #59's explicit frame-stack VM was supposed to have eliminated: a builtin statement still dispatches natively via Op::NativeStatement, and if its children re-enter the VM through the new shortcut, that's a genuine nested native call, not a cheap Op::CallModule hop. A recursive module wrapped in a builtin at every level segfaulted around depth 3000 with no guard at all, since the existing CSG tree-depth guard only fires after a full (already-crashed) descent unwinds. Added driveVmNativeDepth_/kMaxDriveVmNativeDepth in driveVm() to catch this the same way kMaxUserCallDepth already does for the interpreted path -- a clean, catchable error instead of a crash -- while leaving the common case (recursion is pure Op::CallModule, only occasional non-recursing leaves are builtin- wrapped) uncapped by anything but the existing heap-bounded kMaxVmCallStackDepth. Also fixed two pre-existing bugs surfaced by evalChildren's much broader compiled-path coverage: compileForLoop was missing Op::IterReset for inner cartesian dimensions (multi-variable for loops silently under-iterated after the first outer value), and assignBlockChunkCache_ was never actually cleared despite its own doc comment claiming it was. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
revarbat
added a commit
that referenced
this pull request
Aug 1, 2026
…ls (#62) Real crash, found via BelfrySCAD's own CI: pausing a debug session deep inside a native (interpreter-forced, e.g. by an active breakpoint on its own line) function call, with a COMPILED caller above it on the call stack, segfaulted the instant anything tried to read that caller's own frame locals (Evaluator::buildDebugFrames, walked by DebugFramesFn/getFrame() -- exactly what a debugger's "step over a call, but still honor a breakpoint set inside it" scenario needs). Root cause: pushBracketedCallFrame/pushBracketedModuleFrame (the explicit-stack VM's own compiled-call push helpers, bytecode_vm.cpp) called enterUserCall with a reference to their OWN local `childCtx` variable. enterUserCall stores that reference as CallStackFrame::bodyCtx for later use ("per-frame locals for the debugger") -- but the real, long-lived copy of that context ends up living in the pushed VmFrame's own ctxChain (heap-allocated, owned by vmCallStack_), not at the address bodyCtx pointed to. The instant either push helper returned, its own local childCtx was destroyed, leaving bodyCtx dangling for the ENTIRE lifetime of that call -- never noticed until something actually reads a compiled caller's own frame locals while a DEEPER (possibly native) call is still active on the stack. Confirmed present since the explicit-stack VM redesign itself (PR #59) -- bisected by rebuilding at each of PR #59/#60/#61's own commits and reinstalling into BelfrySCAD's venv, all three reproduce it. Root- caused via a targeted AddressSanitizer build of the Python bindings (a plain synchronous C++ probe didn't reproduce it -- needed the real cross-thread blocking a GUI debug session's own hook does) plus a threaded Python repro mirroring BelfrySCAD's debugger.py exactly. Fix: reorder both push helpers to build the VmFrame FIRST (so its own ctxChain owns the context), then call enterUserCall with a reference to frame->ctxChain.back() -- stable for the VmFrame's whole lifetime, since VmFrame lives behind a unique_ptr and never moves even if vmCallStack_ itself reallocates. New regression test (DebugHooks. GetFrameDoesNotCrashWalkingACompiledCallerAboveAForcedInterpretedCallee) reproduces the exact shape and crashes the test process without this fix (verified via git stash before landing this commit). Full 709-test suite green both OSCAD_BYTECODE_VM states. Verified against BelfrySCAD's own previously-crashing test (both directly and via its full 306-test suite) using an editable install of this fix. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Native C++ recursion between compiled chunks (function calls, and now module calls too) is replaced with an explicit, heap-allocated call stack (
Evaluator::vmCallStack_) serviced by one driver loop (driveVm), so a compiled-to-compiled recursive chain no longer grows the native stack at all — bounded instead by a newkMaxVmCallStackDepth(1,000,000), not the native-stack-safetykMaxUserCallDepth(50) that used to cap every recursive call regardless of shape.Op::CallFn/CallFnTail/CallDynamic/CallDynamicTailpush/replace explicitVmFrames instead of recursing natively or through the oldrunCompiledFunctionTrampoline(retired, along withTailCallRequest).Op::CallModule, plusNativeStatement/NativeCondJumpIfFalse/NativeIterMaterialize/ForIterNext/ForIterEndfor control flow) — no trampoline ever existed for modules, so a recursive tree/pattern-generator module used to segfault around depth ~5000 on an ordinary build.if/forget real Jump-based bytecode (so a recursive module call nested inside one doesn't hide behind a native call boundary); everything else (assignment, echo, assert, let-blocks, modifiers,intersection_for, a builtin/unresolved module call) stays a native passthrough for one statement at a time — those are leaf-shaped, never the recursion risk this targets.Evaluator::callCtxFor's closure/lexical-nesting detection used to scan the fullcallStack_on every call. Replaced withactiveDeclRefcount_, a refcounted set of DISTINCT active declarations (a deep call chain is overwhelmingly the same declaration repeated, not many different ones) — ~50x faster at depth 40,000, and scaling is linear again.Test plan
OSCAD_BYTECODE_VMon (default)DebugHooks.FastContinueNotHookSkippableStillFiresEveryCheckpointassumes VM-on and has noScopedVmguard)pyproject.toml)🤖 Generated with Claude Code