From 6cf7c5369c7930dd60b2df6b4ff309cd570cab29 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Thu, 27 Aug 2026 18:49:30 -0700 Subject: [PATCH] Fix debugger segfault: VmFrame::ctxChain must not relocate its elements Setting a breakpoint inside a module and clicking Continue crashed instantly with SIGSEGV at address 0x10, in buildDebugFrame. Evaluator::enterUserCall stores CallStackFrame::bodyCtx as a pointer to frame->ctxChain.back() (pushBracketedCallFrame/pushBracketedModuleFrame). That pointer is read much later, by any checkDebug() that walks the whole call stack to build per-frame debugger locals. ctxChain was a std::vector, so the next push onto it reallocated and left bodyCtx pointing at the moved-from element -- whose let_ shared_ptr is null, hence the read at offset 0x10 from null. The push that does it is Op::PushBuiltinWrap: every transform inside a compiled body pushes a context. So the trigger is as ordinary as `module mid(a) { translate([a,0,0]) inner(a); }`. It only bites while debugging, because a chunk is compiled at all only when fastContinueBreakpoints_ is set (chunkEligibleNow) -- which is exactly the "breakpoint set, now continue" state. A breakpointed callee is forced to interpret while its caller stays compiled, and pausing in the callee walks the caller's now-stale frame. std::deque never invalidates references to existing elements on push_back/pop_back, so the fix is the container. lastCtx_ and childrenCallerCtx point into the same storage and are covered by it too. No measurable cost: 8 runs of eval_perf_benchmark (50,000 translate() calls, the same push path) give min 432ms vs 426ms for vector, medians 451ms vs 467ms -- within noise both ways. Regression test drives the user's exact scenario: a breakpoint that forces the callee to interpret while its caller runs compiled, then reads the caller's locals from the debug frames. It segfaults without the fix. Co-Authored-By: Claude Opus 5 (1M context) --- .../openscad_cpp_evaluator/bytecode_vm.hpp | 12 +++++- pyproject.toml | 2 +- src/bytecode_vm.cpp | 9 ++-- tests/test_debug_hooks.cpp | 42 +++++++++++++++++++ 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/include/openscad_cpp_evaluator/bytecode_vm.hpp b/include/openscad_cpp_evaluator/bytecode_vm.hpp index 07e6632..34904de 100644 --- a/include/openscad_cpp_evaluator/bytecode_vm.hpp +++ b/include/openscad_cpp_evaluator/bytecode_vm.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include namespace oscadeval { @@ -155,7 +156,16 @@ struct VmFrame { std::vector bound; std::vector> accumStack; std::vector iterLists; - std::vector ctxChain; + // std::deque, NOT vector: Evaluator::enterUserCall stores + // CallStackFrame::bodyCtx as a pointer to ctxChain.back() (see + // pushBracketedCallFrame/pushBracketedModuleFrame), and that pointer is + // read LATER, by any checkDebug() that walks the whole call stack to + // build per-frame debugger locals. A vector reallocates on the next + // push -- which every transform inside a compiled body does, via + // Op::PushBuiltinWrap -- leaving bodyCtx pointing at the moved-from + // element (null let_) and segfaulting buildDebugFrame. deque never + // invalidates references to existing elements on push_back/pop_back. + std::deque ctxChain; // Still-open Op::PushBuiltinWrap brackets, LIFO, scoped to THIS frame's // own instruction stream -- mirrors accumStack/iterLists' own role for // their respective bracket pairs. Normal execution always drains this diff --git a/pyproject.toml b/pyproject.toml index 7a78ef8..650749f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.51.2" +version = "0.51.3" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/bytecode_vm.cpp b/src/bytecode_vm.cpp index 223f4eb..26e5a4e 100644 --- a/src/bytecode_vm.cpp +++ b/src/bytecode_vm.cpp @@ -270,8 +270,11 @@ void pushBracketedCallFrame(Evaluator& ev, const CompiledChunk& chunk, const osc applyCompiledDefaultsToFrame(ev, chunk, *frame); // enterUserCall's own bodyCtx must point at frame->ctxChain's OWN - // storage (stable: this VmFrame lives behind a unique_ptr, so its - // address never moves even if vmCallStack_ itself reallocates), not + // storage (stable on two counts: this VmFrame lives behind a + // unique_ptr, so its address never moves even if vmCallStack_ itself + // reallocates, AND ctxChain is a std::deque, so a later push onto it + // never moves the element handed over here -- see its own doc comment, + // bytecode_vm.hpp, for the segfault a vector caused), not // a plain local -- a local `childCtx` here would be destroyed the // instant this function returns, but CallStackFrame::bodyCtx is read // LATER, by ANY subsequent checkDebug() call (this call's own nested @@ -328,7 +331,7 @@ void pushBracketedModuleFrame(Evaluator& ev, const CompiledChunk& chunk, const o frame->logicalName = decl.name->name; frame->ownsModuleSplice = true; // enterUserCall's own bodyCtx must point at frame->ctxChain's OWN - // storage, not the caller's own (about-to-be-destroyed) local + // (deque-stable) storage, not the caller's own (about-to-be-destroyed) local // `childCtx` -- see pushBracketedCallFrame's own doc comment, above, // for the full dangling-pointer hazard this avoids (same fix, same // root cause, module side). diff --git a/tests/test_debug_hooks.cpp b/tests/test_debug_hooks.cpp index 0b5a6a5..6b59d2d 100644 --- a/tests/test_debug_hooks.cpp +++ b/tests/test_debug_hooks.cpp @@ -694,3 +694,45 @@ TEST(DebugHooksParity, BuiltinFunctionCallGetsNoCallSiteStop) { TEST(DebugHooksParity, ModifierAndItsWrappedChildBothStop) { EXPECT_EQ(recordStops("#cube(1);\n"), (std::vector{{1, false, false}, {1, false, false}})); } + +// An enclosing COMPILED module frame's debugger locals must survive the +// frame growing its own ctxChain. `enterUserCall` stores +// CallStackFrame::bodyCtx as a pointer into VmFrame::ctxChain, and any +// later push onto that chain -- which every transform inside a compiled +// body does, via Op::PushBuiltinWrap -- used to reallocate the container +// and leave bodyCtx dangling. buildDebugFrames then read a moved-from +// EvalContext (null let_) and segfaulted: the crash a user hit by setting +// a breakpoint inside a module wrapped in translate() and continuing. +TEST(DebugHooksParity, EnclosingCompiledFrameLocalsSurviveCtxChainGrowth) { + ScopedVm vm(true); + // The breakpoint is what makes this mixed: `inner` (line 1) is forced + // to interpret, while `mid` -- called from inside compiled `top` -- + // keeps running compiled, so its callStack_ entry's bodyCtx comes from + // pushBracketedModuleFrame and points into VmFrame::ctxChain. The + // translate() in mid's body then pushes onto that same chain. + const std::string src = "module inner(b) { c = b; }\n" + "module mid(a) { translate([a,0,0]) inner(a); }\n" + "module top() { mid(7); }\n" + "top();\n"; + bool sawMid = false; + DebugHooks hooks; + hooks.debugHook = [&](int line, int, bool, bool exprLevel, const std::string&, + const std::vector&, const DebugFramesFn& getFrame) { + if (exprLevel || line != 1) return DebugAction{}; + std::vector frames = getFrame(); + for (const DebugFrame& f : frames) { + auto it = f.locals.find("a"); + if (it == f.locals.end()) continue; + sawMid = true; + EXPECT_EQ(std::get(it->second), 7.0); + } + return DebugAction{}; + }; + Evaluator ev(EchoFn{}, nullptr, nullptr, hooks); + ev.setFastContinueBreakpoints(std::unordered_map>{{"", {1}}}); + auto ast = parseSrc(src); + auto scope = oscad::buildScopes(ast); + EvalContext ctx = EvalContext::makeRoot(scope.get()); + ev.resolveTree(ast, ctx); + EXPECT_TRUE(sawMid) << "mid()'s own frame never appeared in the debugger frames"; +}