Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion include/openscad_cpp_evaluator/bytecode_vm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <cstdint>
#include <memory>
#include <optional>
#include <deque>
#include <vector>

namespace oscadeval {
Expand Down Expand Up @@ -155,7 +156,16 @@ struct VmFrame {
std::vector<bool> bound;
std::vector<std::vector<Value>> accumStack;
std::vector<IterList> iterLists;
std::vector<EvalContext> 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<EvalContext> 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
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
9 changes: 6 additions & 3 deletions src/bytecode_vm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).
Expand Down
42 changes: 42 additions & 0 deletions tests/test_debug_hooks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -694,3 +694,45 @@ TEST(DebugHooksParity, BuiltinFunctionCallGetsNoCallSiteStop) {
TEST(DebugHooksParity, ModifierAndItsWrappedChildBothStop) {
EXPECT_EQ(recordStops("#cube(1);\n"), (std::vector<Stop>{{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<CallStackFrame>&, const DebugFramesFn& getFrame) {
if (exprLevel || line != 1) return DebugAction{};
std::vector<DebugFrame> 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<double>(it->second), 7.0);
}
return DebugAction{};
};
Evaluator ev(EchoFn{}, nullptr, nullptr, hooks);
ev.setFastContinueBreakpoints(std::unordered_map<std::string, std::set<int>>{{"<string>", {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";
}