From fd15d2b0a16bf855afb5e2e44fc1def0b3e970a6 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Fri, 31 Jul 2026 17:13:53 -0700 Subject: [PATCH] Fix dangling CallStackFrame::bodyCtx for compiled function/module calls 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 --- pyproject.toml | 2 +- src/bytecode_vm.cpp | 31 +++++++++++++++++++++---- tests/test_debug_hooks.cpp | 47 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 6 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 7436864..11f86d9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.13.0" +version = "0.13.1" 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 19e9cc1..0893aae 100644 --- a/src/bytecode_vm.cpp +++ b/src/bytecode_vm.cpp @@ -234,8 +234,6 @@ void pushBracketedCallFrame(Evaluator& ev, const CompiledChunk& chunk, const osc bool usedChildCtx = false; EvalContext childCtx = ev.callCtxFor(declNode, callerCtx, fnScope, nullptr, nullptr, &usedChildCtx, capturedLet); const int upvalueParent = usedChildCtx ? callerFrameIdx : -1; - Evaluator::UserCallHandle handle = - ev.enterUserCall(name, declNode, &bodyExpr, childCtx, callPos, upvalueParent, /*skipDepthGuard=*/true); auto frame = ev.acquireVmFrame(); frame->chunk = &chunk; @@ -253,6 +251,23 @@ void pushBracketedCallFrame(Evaluator& ev, const CompiledChunk& chunk, const osc frame->hopEligible = true; // just pushed a fresh, correctly-named callStack_ entry bindBoundArgsIntoFrame(chunk, bound, *frame); 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 + // 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 + // calls, or an unrelated sibling's) that walks the WHOLE call stack + // to build per-frame debugger locals (Evaluator::buildDebugFrames) -- + // reading a dangling pointer there segfaults. Real bug, caught via a + // targeted ASan build reproducing BelfrySCAD's own "step over a call, + // but honor a breakpoint set inside it" debug scenario: pausing deep + // inside a native (interpreter-forced, due to the breakpoint) callee + // walks the WHOLE call stack, including this compiled caller's own + // now-stale frame. + Evaluator::UserCallHandle handle = ev.enterUserCall(name, declNode, &bodyExpr, frame->ctxChain.back(), callPos, + upvalueParent, /*skipDepthGuard=*/true); ev.vmCallStack_.push_back(std::move(frame)); ev.vmCallBrackets_.push_back(std::move(handle)); } @@ -277,9 +292,6 @@ void pushBracketedModuleFrame(Evaluator& ev, const CompiledChunk& chunk, const o if (ev.vmCallStack_.size() >= Evaluator::kMaxVmCallStackDepth) { ev.error("Recursion too deep while calling module '" + decl.name->name + "'", decl); } - Evaluator::UserCallHandle handle = ev.enterUserCall(decl.name->name, decl, /*bodyExpr=*/nullptr, childCtx, callPos, - /*upvalueParent=*/-1, /*skipDepthGuard=*/true, - CallStackFrame::Kind::Module); auto frame = ev.acquireVmFrame(); frame->chunk = &chunk; frame->code = &chunk.bodyCode; @@ -298,6 +310,15 @@ void pushBracketedModuleFrame(Evaluator& ev, const CompiledChunk& chunk, const o frame->tailHopGuard = 0; 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 + // `childCtx` -- see pushBracketedCallFrame's own doc comment, above, + // for the full dangling-pointer hazard this avoids (same fix, same + // root cause, module side). + Evaluator::UserCallHandle handle = ev.enterUserCall(decl.name->name, decl, /*bodyExpr=*/nullptr, + frame->ctxChain.back(), callPos, + /*upvalueParent=*/-1, /*skipDepthGuard=*/true, + CallStackFrame::Kind::Module); frame->moduleRandsBefore = randsBefore; frame->moduleSpliceCallNode = &callNode; ev.vmCallStack_.push_back(std::move(frame)); diff --git a/tests/test_debug_hooks.cpp b/tests/test_debug_hooks.cpp index 3998424..305376b 100644 --- a/tests/test_debug_hooks.cpp +++ b/tests/test_debug_hooks.cpp @@ -384,6 +384,53 @@ TEST(DebugHooks, ForcedBreakpointBuiltinAlwaysFiresEvenInHookSkippableMode) { EXPECT_EQ(normalCalls, 0); // cube/breakpoint()-as-statement/sphere: no breakpoint line matches, all skipped } +// Regression test for a real dangling-pointer bug (found via a targeted +// ASan build reproducing BelfrySCAD's own "step over a call, but honor a +// breakpoint set inside it" debug scenario): pushBracketedCallFrame/ +// pushBracketedModuleFrame (bytecode_vm.cpp) used to point +// CallStackFrame::bodyCtx at their own LOCAL `childCtx` variable, which is +// destroyed the instant they return -- but bodyCtx is read LATER, by any +// subsequent checkDebug() call that walks the WHOLE call stack to build +// per-frame debugger locals (getFrame(), below). A compiled caller +// (`stepped_over`, no breakpoint of its own, so it runs compiled) calling +// a callee forced into the interpreter by an active breakpoint on ITS OWN +// line (`heavy`) reproduces this exactly: pausing deep inside `heavy` and +// calling getFrame() used to segfault reading the compiled caller's own +// already-dangling frame. +TEST(DebugHooks, GetFrameDoesNotCrashWalkingACompiledCallerAboveAForcedInterpretedCallee) { + ScopedVm vm(true); + std::vector frameCountsAtLine1; + DebugHooks hooks; + hooks.debugHook = [&](int line, int depth, bool, bool exprLevel, const std::string&, + const std::vector& callStack, const DebugFramesFn& getFrame) { + if (line == 1 && depth == 2 && !exprLevel) { + std::vector frames = getFrame(); + frameCountsAtLine1.push_back(frames.size()); + EXPECT_EQ(callStack.size(), 2u); + } + return DebugAction{}; + }; + Evaluator ev(EchoFn{}, nullptr, nullptr, hooks); + // Mirrors BelfrySCAD's own _apply_fast_continue for an active step: + // a real breakpoint set (on heavy's own line 1, forcing IT specifically + // to stay interpreted via chunkEligibleNow) with hookSkippable=false. + ev.setFastContinueBreakpoints(std::unordered_map>{{"", {1}}}, + /*hookSkippable=*/false); + auto ast = parseSrc("function heavy(n) = [for (i=[0:1:n-1]) i*i];\n" + "function stepped_over() = let(r = heavy(3)) len(r);\n" + "b = stepped_over();\n"); + auto scope = oscad::buildScopes(ast); + EvalContext ctx = EvalContext::makeRoot(scope.get()); + ev.evaluate(ast, ctx); + // The actual point of this test is that getFrame() doesn't crash + // reading the compiled `stepped_over` frame's own (previously + // dangling) bodyCtx -- a consistent, non-empty frame list is enough + // evidence it read something real rather than garbage each time. + ASSERT_FALSE(frameCountsAtLine1.empty()); + for (size_t count : frameCountsAtLine1) EXPECT_EQ(count, frameCountsAtLine1.front()); + EXPECT_GT(frameCountsAtLine1.front(), 0u); +} + // --------------------------------------------------------------------------- // Full parity with the Python reference's _check_debug call sites. //