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. //