diff --git a/.cspell.dict/rust-more.txt b/.cspell.dict/rust-more.txt index c4457723c..b53639c3b 100644 --- a/.cspell.dict/rust-more.txt +++ b/.cspell.dict/rust-more.txt @@ -50,6 +50,7 @@ modpow msvc muldiv nanos +noalias nonoverlapping objclass peekable diff --git a/.github/workflows/cron-ci.yaml b/.github/workflows/cron-ci.yaml index 059604625..c6eebbdf8 100644 --- a/.github/workflows/cron-ci.yaml +++ b/.github/workflows/cron-ci.yaml @@ -85,7 +85,6 @@ jobs: if: ${{ github.event_name != 'pull_request' }} env: SSHKEY: ${{ secrets.ACTIONS_TESTS_DATA_DEPLOY_KEY }} - GITHUB_ACTOR: ${{ github.actor }} run: | echo "$SSHKEY" >~/github_key chmod 600 ~/github_key @@ -95,7 +94,7 @@ jobs: cd website cp ../extra_tests/cpython_tests_results.json ./_data/regrtests_results.json git add ./_data/regrtests_results.json - if git -c user.name="Github Actions" -c user.email="actions@github.com" commit -m "Update regression test results" --author="$GITHUB_ACTOR"; then + if git -c user.name="Github Actions" -c user.email="actions@github.com" commit -m "Update regression test results"; then git push fi @@ -127,7 +126,6 @@ jobs: if: ${{ github.event_name != 'pull_request' }} env: SSHKEY: ${{ secrets.ACTIONS_TESTS_DATA_DEPLOY_KEY }} - GITHUB_ACTOR: ${{ github.actor }} run: | echo "$SSHKEY" >~/github_key chmod 600 ~/github_key @@ -158,7 +156,7 @@ jobs: } EOF git add -A - if git -c user.name="Github Actions" -c user.email="actions@github.com" commit -m "Update what is left results" --author="$GITHUB_ACTOR"; then + if git -c user.name="Github Actions" -c user.email="actions@github.com" commit -m "Update what is left results"; then git push fi @@ -204,6 +202,8 @@ jobs: if: ${{ github.event_name != 'pull_request' }} env: SSHKEY: ${{ secrets.ACTIONS_TESTS_DATA_DEPLOY_KEY }} + COMMIT_SHA: ${{ github.sha }} + REF_NAME: ${{ github.ref_name }} run: | echo "$SSHKEY" >~/github_key chmod 600 ~/github_key @@ -215,8 +215,8 @@ jobs: cp -r ../target/criterion ./assets/criterion printf '{\n "generated_at": "%s",\n "rustpython_commit": "%s",\n "rustpython_ref": "%s"\n}\n' \ "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ - "${{ github.sha }}" \ - "${{ github.ref_name }}" > ./_data/criterion-metadata.json + "$COMMIT_SHA" \ + "$REF_NAME" > ./_data/criterion-metadata.json git add ./assets/criterion ./_data/criterion-metadata.json if git -c user.name="Github Actions" -c user.email="actions@github.com" commit -m "Update benchmark results"; then git push diff --git a/.github/workflows/lib-deps-check.yaml b/.github/workflows/lib-deps-check.yaml index a938c6ff0..6f147542e 100644 --- a/.github/workflows/lib-deps-check.yaml +++ b/.github/workflows/lib-deps-check.yaml @@ -6,6 +6,8 @@ on: paths: - "Lib/**" +permissions: {} + concurrency: group: ${{ github.workflow }}-${{ github.ref_name }}-${{ github.event.pull_request.number }} cancel-in-progress: true @@ -26,13 +28,17 @@ jobs: persist-credentials: false - name: Fetch PR head + env: + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | - git fetch origin ${{ github.event.pull_request.head.sha }} + git fetch origin "$PR_HEAD_SHA" - name: Checkout PR Lib files + env: + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | # Checkout only Lib/ directory from PR head for accurate comparison - git checkout ${{ github.event.pull_request.head.sha }} -- Lib/ + git checkout "$PR_HEAD_SHA" -- Lib/ - name: Get target CPython version id: cpython-version @@ -51,14 +57,17 @@ jobs: - name: Get changed Lib files id: all-changed-files + env: + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | # Get the list of changed files under Lib/ { echo 'changed<> "$GITHUB_OUTPUT" - name: Parse changed files diff --git a/.github/zizmor.yml b/.github/zizmor.yml index f22f76b70..33ac61c64 100644 --- a/.github/zizmor.yml +++ b/.github/zizmor.yml @@ -1,4 +1,9 @@ rules: + excessive-permissions: + ignore: + # pull_request_target is needed to post PR comments with pull-requests: write. + # Workflow-level permissions: {} restricts defaults; only the job has write access. + - lib-deps-check.yaml:3 unpinned-uses: config: policies: diff --git a/.gitignore b/.gitignore index 92fc399bf..cb8548877 100644 --- a/.gitignore +++ b/.gitignore @@ -29,5 +29,5 @@ Lib/site-packages/* Lib/test/data/* !Lib/test/data/README cpython/ -.claude/scheduled_tasks.lock +.claude/ docs/superpowers/ \ No newline at end of file diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index b3826f422..07c1decb4 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -134,7 +134,6 @@ class FinalizationTest(unittest.TestCase): self.assertEqual(len(resurrected), 1) self.assertIsInstance(resurrected[0].gi_code, types.CodeType) - @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: is not None def test_exhausted_generator_frame_cycle(self): def g(): yield diff --git a/crates/capi/src/ceval.rs b/crates/capi/src/ceval.rs index 366cb0071..7de5b3cf6 100644 --- a/crates/capi/src/ceval.rs +++ b/crates/capi/src/ceval.rs @@ -58,7 +58,7 @@ pub extern "C" fn PyEval_GetBuiltins() -> *mut PyObject { with_vm(|vm| { vm.current_frame().map_or_else( || vm.builtins.as_object().as_raw(), - |frame| frame.builtins.as_object().as_raw(), + |frame| frame.iframe().builtins().as_raw(), ) }) } @@ -78,7 +78,7 @@ pub extern "C" fn PyEval_GetFrameBuiltins() -> *mut PyObject { with_vm(|vm| { vm.current_frame().map_or_else( || vm.builtins.as_object().to_owned(), - |frame| frame.builtins.as_object().to_owned(), + |frame| frame.iframe().builtins().to_owned(), ) }) } @@ -87,7 +87,15 @@ pub extern "C" fn PyEval_GetFrameBuiltins() -> *mut PyObject { pub extern "C" fn PyEval_GetFrameGlobals() -> *mut PyObject { with_vm(|vm| { vm.current_frame() - .map(|frame| frame.globals.as_object().to_owned().into_raw().as_ptr()) + .map(|frame| { + frame + .iframe() + .globals() + .as_object() + .to_owned() + .into_raw() + .as_ptr() + }) .unwrap_or_default() }) } @@ -107,7 +115,7 @@ pub extern "C" fn PyEval_GetFrameLocals() -> *mut PyObject { pub extern "C" fn PyEval_GetGlobals() -> *mut PyObject { with_vm(|vm| { vm.current_frame() - .map(|frame| frame.globals.as_object().as_raw()) + .map(|frame| frame.iframe().globals().as_object().as_raw()) .unwrap_or_default() }) } @@ -119,7 +127,7 @@ pub extern "C" fn PyEval_GetLocals() -> *mut PyObject { return Ok(core::ptr::null_mut()); }; let _ = frame.locals(vm)?; - Ok(frame.locals.as_object(vm).as_raw().cast_mut()) + Ok(frame.iframe().locals.as_object(vm).as_raw().cast_mut()) }) } diff --git a/crates/capi/src/pyframe.rs b/crates/capi/src/pyframe.rs index 5c9ad3717..611cf79b0 100644 --- a/crates/capi/src/pyframe.rs +++ b/crates/capi/src/pyframe.rs @@ -2,9 +2,9 @@ use crate::pystate::with_vm; use core::ffi::c_int; use rustpython_vm::Py; use rustpython_vm::builtins::PyCode; -use rustpython_vm::frame::Frame; +use rustpython_vm::frame::FrameObject; -pub type PyFrameObject = Py; +pub type PyFrameObject = Py; pub type PyCodeObject = Py; #[unsafe(no_mangle)] diff --git a/crates/stdlib/src/faulthandler.rs b/crates/stdlib/src/faulthandler.rs index 900d66b76..6edd023f1 100644 --- a/crates/stdlib/src/faulthandler.rs +++ b/crates/stdlib/src/faulthandler.rs @@ -3,8 +3,6 @@ pub(crate) use decl::module_def; #[allow(static_mut_refs)] // TODO: group code only with static mut refs #[pymodule(name = "faulthandler")] mod decl { - #[cfg(any(unix, windows))] - use crate::vm::frame::Frame; use crate::vm::{ PyObjectRef, PyResult, VirtualMachine, function::{ArgIntoFloat, OptionalArg}, @@ -119,43 +117,45 @@ mod decl { } /// Dump the current thread's live frame chain to fd (signal-safe). - /// Walks the `Frame.previous` pointer chain starting from the - /// thread-local current frame pointer. + /// Walks the InterpreterFrame chain directly. #[cfg(any(unix, windows))] fn dump_live_frames(fd: i32) { const MAX_FRAME_DEPTH: usize = 100; - let mut frame_ptr = crate::vm::vm::thread::get_current_frame(); - if frame_ptr.is_null() { + let mut cur = crate::vm::vm::thread::get_current_frame(); + if cur.is_null() { puts(fd, " \n"); return; } let mut depth = 0; - while !frame_ptr.is_null() && depth < MAX_FRAME_DEPTH { - let frame = unsafe { &*frame_ptr }; - dump_frame_from_raw(fd, frame); - frame_ptr = frame.previous_frame(); + while !cur.is_null() && depth < MAX_FRAME_DEPTH { + let iframe = unsafe { &*cur }; + dump_iframe(fd, iframe); depth += 1; + cur = iframe.previous(); } - if depth >= MAX_FRAME_DEPTH && !frame_ptr.is_null() { + if depth == 0 { + puts(fd, " \n"); + } else if depth >= MAX_FRAME_DEPTH && !cur.is_null() { puts(fd, " ...\n"); } } - /// Dump a single frame's info to fd (signal-safe), reading live data. + /// Dump a single InterpreterFrame's info to fd (signal-safe). #[cfg(any(unix, windows))] - fn dump_frame_from_raw(fd: i32, frame: &Frame) { - let filename = frame.code.source_path().as_str(); - let funcname = frame.code.obj_name.as_str(); - let lasti = frame.lasti(); + fn dump_iframe(fd: i32, iframe: &rustpython_vm::frame::InterpreterFrame) { + let code = iframe.code(); + let filename = code.source_path().as_str(); + let funcname = code.obj_name.as_str(); + let lasti = iframe.get_lasti(); let lineno = if lasti == 0 { - frame.code.first_line_number.map_or(1, |n| n.get()) as u32 + code.first_line_number.map_or(1, |n| n.get()) as u32 } else { let idx = (lasti as usize).saturating_sub(1); - if idx < frame.code.locations.len() { - frame.code.locations[idx].0.line.get() as u32 + if idx < code.locations.len() { + code.locations[idx].0.line.get() as u32 } else { - frame.code.first_line_number.map_or(0, |n| n.get()) as u32 + code.first_line_number.map_or(0, |n| n.get()) as u32 } }; @@ -218,77 +218,6 @@ mod decl { } } - /// Write a frame's info to an fd using signal-safe I/O. - #[cfg(any(unix, windows))] - fn dump_frame_from_ref(fd: i32, frame: &crate::vm::Py) { - let funcname = frame.code.obj_name.as_str(); - let filename = frame.code.source_path().as_str(); - let lineno = if frame.lasti() == 0 { - frame.code.first_line_number.map_or(1, |n| n.get()) as u32 - } else { - frame.current_location().line.get() as u32 - }; - - puts(fd, " File \""); - dump_ascii(fd, filename); - puts(fd, "\", line "); - dump_decimal(fd, lineno as usize); - puts(fd, " in "); - dump_ascii(fd, funcname); - puts(fd, "\n"); - } - - /// Dump traceback for a thread given its frame stack (for cross-thread dumping). - /// # Safety - /// Each `FramePtr` must point to a live frame (caller holds the Mutex). - #[cfg(all(windows, feature = "threading"))] - fn dump_traceback_thread_frames( - fd: i32, - thread_id: u64, - is_current: bool, - frames: &[rustpython_vm::vm::FramePtr], - ) { - write_thread_id(fd, thread_id, is_current); - - if frames.is_empty() { - puts(fd, " \n"); - } else { - for fp in frames.iter().rev() { - // SAFETY: caller holds the Mutex, so the owning thread can't pop. - dump_frame_from_ref(fd, unsafe { fp.as_ref() }); - } - } - } - - /// Dump a thread's traceback by walking its published top frame down the - /// `previous` chain (most recent first). Signal-safe: only atomic pointer - /// loads, no locks. Callers guarantee frame liveness — under stop-the-world - /// for `faulthandler.dump_traceback`, or best-effort for the watchdog (like - /// `_Py_DumpTracebackThreads`, which walks lock-free while other threads - /// may still run). - #[cfg(all(unix, feature = "threading"))] - fn dump_traceback_thread_chain(fd: i32, thread_id: u64, is_current: bool, top: *const Frame) { - const MAX_FRAME_DEPTH: usize = 100; - write_thread_id(fd, thread_id, is_current); - - if top.is_null() { - puts(fd, " \n"); - return; - } - let mut frame_ptr = top; - let mut depth = 0; - while !frame_ptr.is_null() && depth < MAX_FRAME_DEPTH { - // SAFETY: the frame is alive per the caller's liveness guarantee. - let frame = unsafe { &*frame_ptr }; - dump_frame_from_raw(fd, frame); - frame_ptr = frame.previous_frame(); - depth += 1; - } - if depth >= MAX_FRAME_DEPTH && !frame_ptr.is_null() { - puts(fd, " ...\n"); - } - } - #[derive(FromArgs)] struct DumpTracebackArgs { #[pyarg(any, default)] @@ -307,9 +236,7 @@ mod decl { dump_all_threads(fd, vm); } else { puts(fd, "Stack (most recent call first):\n"); - crate::vm::frame::for_each_current_frame(|frame| { - dump_frame_from_ref(fd, frame); - }); + dump_live_frames(fd); } } @@ -344,21 +271,35 @@ mod decl { if tid == current_tid { continue; } - let top = slot.top_frame.load(Ordering::Relaxed) as *const Frame; - dump_traceback_thread_chain(fd, tid, false, top); + // Under STW, all other threads are suspended so their + // stack-allocated iframes are stable. Walk via top_iframe + // which covers both FrameObject and stack-allocated paths. + let iframe_ptr = slot.top_iframe.load(Ordering::Relaxed) + as *const rustpython_vm::frame::InterpreterFrame; + write_thread_id(fd, tid, false); + if iframe_ptr.is_null() { + puts(fd, " \n"); + } else { + let mut cur = iframe_ptr; + let mut depth = 0; + while !cur.is_null() && depth < 100 { + let iframe = unsafe { &*cur }; + dump_iframe(fd, iframe); + depth += 1; + cur = iframe.previous(); + } + if depth >= 100 && !cur.is_null() { + puts(fd, " ...\n"); + } + } puts(fd, "\n"); } } - // Now dump current thread from its live frame chain. + // Now dump current thread from its live frame chain + // (includes stack-allocated iframes without FrameObject). write_thread_id(fd, current_tid, true); - if crate::vm::vm::thread::get_current_frame().is_null() { - puts(fd, " \n"); - } else { - crate::vm::frame::for_each_current_frame(|frame| { - dump_frame_from_ref(fd, frame); - }); - } + dump_live_frames(fd); } #[cfg(all(not(unix), feature = "threading"))] @@ -366,7 +307,8 @@ mod decl { let current_tid = rustpython_vm::stdlib::_thread::get_ident(); let registry = vm.state.thread_frames.lock(); - // First dump non-current threads, then current thread last + // Dump non-current threads using top_iframe, which includes + // both FrameObject and stack-allocated frames. #[expect( clippy::iter_over_hash_type, reason = "Iteration order doesn't matter here" @@ -376,29 +318,38 @@ mod decl { continue; } - let frames_guard = slot.frames.lock(); - dump_traceback_thread_frames(fd, tid, false, &frames_guard); + let iframe_ptr = slot.top_iframe.load(core::sync::atomic::Ordering::Relaxed) + as *const rustpython_vm::frame::InterpreterFrame; + write_thread_id(fd, tid, false); + if iframe_ptr.is_null() { + puts(fd, " \n"); + } else { + let mut cur = iframe_ptr; + let mut depth = 0; + while !cur.is_null() && depth < 100 { + let iframe = unsafe { &*cur }; + dump_iframe(fd, iframe); + depth += 1; + cur = iframe.previous(); + } + if depth >= 100 && !cur.is_null() { + puts(fd, " ...\n"); + } + } puts(fd, "\n"); } - // Now dump current thread from its live frame chain. + // Now dump current thread from its live frame chain + // (includes stack-allocated iframes without FrameObject). write_thread_id(fd, current_tid, true); - if crate::vm::vm::thread::get_current_frame().is_null() { - puts(fd, " \n"); - } else { - crate::vm::frame::for_each_current_frame(|frame| { - dump_frame_from_ref(fd, frame); - }); - } + dump_live_frames(fd); } #[cfg(not(feature = "threading"))] { let _ = vm; write_thread_id(fd, current_thread_id(), true); - crate::vm::frame::for_each_current_frame(|frame| { - dump_frame_from_ref(fd, frame); - }); + dump_live_frames(fd); } } @@ -724,18 +675,56 @@ mod decl { // the VM, so it cannot stop-the-world. Walk each // published top frame lock-free and best-effort, like // the faulthandler watchdog thread. + // Walk via top_iframe for all threads. The watchdog + // cannot stop-the-world, so this is best-effort + // (like CPython's _Py_DumpTracebackThreads). Stack + // frames are still alive because the target thread + // is executing (inside a blocking call). for (tid, slot) in &thread_frame_slots { - let top = slot - .top_frame + let iframe_ptr = slot + .top_iframe .load(core::sync::atomic::Ordering::Relaxed) - as *const Frame; - dump_traceback_thread_chain(fd, *tid, false, top); + as *const rustpython_vm::frame::InterpreterFrame; + write_thread_id(fd, *tid, false); + if iframe_ptr.is_null() { + puts(fd, " \n"); + } else { + let mut cur = iframe_ptr; + let mut depth = 0; + while !cur.is_null() && depth < 100 { + let iframe = unsafe { &*cur }; + dump_iframe(fd, iframe); + depth += 1; + cur = iframe.previous(); + } + if depth >= 100 && !cur.is_null() { + puts(fd, " ...\n"); + } + } } } all(not(unix), feature = "threading") => { for (tid, slot) in &thread_frame_slots { - let frames = slot.frames.lock(); - dump_traceback_thread_frames(fd, *tid, false, &frames); + let iframe_ptr = slot + .top_iframe + .load(core::sync::atomic::Ordering::Relaxed) + as *const rustpython_vm::frame::InterpreterFrame; + write_thread_id(fd, *tid, false); + if iframe_ptr.is_null() { + puts(fd, " \n"); + } else { + let mut cur = iframe_ptr; + let mut depth = 0; + while !cur.is_null() && depth < 100 { + let iframe = unsafe { &*cur }; + dump_iframe(fd, iframe); + depth += 1; + cur = iframe.previous(); + } + if depth >= 100 && !cur.is_null() { + puts(fd, " ...\n"); + } + } } } _ => { diff --git a/crates/vm/src/builtins/asyncgenerator.rs b/crates/vm/src/builtins/asyncgenerator.rs index dea40062a..b53e59d58 100644 --- a/crates/vm/src/builtins/asyncgenerator.rs +++ b/crates/vm/src/builtins/asyncgenerator.rs @@ -5,7 +5,7 @@ use crate::{ class::PyClassImpl, common::lock::PyMutex, coroutine::{Coro, warn_deprecated_throw_signature}, - frame::FrameRef, + frame::FrameObjectRef, function::OptionalArg, object::{Traverse, TraverseFn}, protocol::PyIterReturn, @@ -50,7 +50,7 @@ impl PyAsyncGen { } #[must_use] - pub fn new(frame: FrameRef, name: PyStrRef, qualname: PyStrRef) -> Self { + pub fn new(frame: FrameObjectRef, name: PyStrRef, qualname: PyStrRef) -> Self { Self { inner: Coro::new(frame, name, qualname), running_async: AtomicCell::new(false), @@ -127,7 +127,7 @@ impl PyAsyncGen { self.inner.frame().yield_from_target() } #[pygetset] - fn ag_frame(&self, _vm: &VirtualMachine) -> Option { + fn ag_frame(&self, _vm: &VirtualMachine) -> Option { if self.inner.closed() { None } else { @@ -140,7 +140,7 @@ impl PyAsyncGen { } #[pygetset] fn ag_code(&self, _vm: &VirtualMachine) -> PyRef { - self.inner.frame().code.clone() + self.inner.frame().iframe().code().to_owned() } #[pyclassmethod] @@ -688,7 +688,8 @@ impl PyAnextAwaitable { && generator .as_coro() .frame() - .code + .iframe() + .code() .flags .contains(crate::bytecode::CodeFlags::ITERABLE_COROUTINE) { diff --git a/crates/vm/src/builtins/coroutine.rs b/crates/vm/src/builtins/coroutine.rs index 1780370b4..d472f1a0b 100644 --- a/crates/vm/src/builtins/coroutine.rs +++ b/crates/vm/src/builtins/coroutine.rs @@ -3,7 +3,7 @@ use crate::{ AsObject, Context, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, class::PyClassImpl, coroutine::{Coro, warn_deprecated_throw_signature}, - frame::FrameRef, + frame::FrameObjectRef, function::OptionalArg, object::{Traverse, TraverseFn}, protocol::PyIterReturn, @@ -41,7 +41,7 @@ impl PyCoroutine { } #[must_use] - pub fn new(frame: FrameRef, name: PyStrRef, qualname: PyStrRef) -> Self { + pub fn new(frame: FrameObjectRef, name: PyStrRef, qualname: PyStrRef) -> Self { Self { inner: Coro::new(frame, name, qualname), } @@ -80,7 +80,7 @@ impl PyCoroutine { self.inner.frame().yield_from_target() } #[pygetset] - fn cr_frame(&self, _vm: &VirtualMachine) -> Option { + fn cr_frame(&self, _vm: &VirtualMachine) -> Option { if self.inner.closed() { None } else { @@ -93,7 +93,7 @@ impl PyCoroutine { } #[pygetset] fn cr_code(&self, _vm: &VirtualMachine) -> PyRef { - self.inner.frame().code.clone() + self.inner.frame().iframe().code().to_owned() } // TODO: coroutine origin tracking: // https://docs.python.org/3/library/sys.html#sys.set_coroutine_origin_tracking_depth diff --git a/crates/vm/src/builtins/frame.rs b/crates/vm/src/builtins/frame.rs index 710d23b7c..70ff99979 100644 --- a/crates/vm/src/builtins/frame.rs +++ b/crates/vm/src/builtins/frame.rs @@ -6,11 +6,14 @@ use super::{PyCode, PyDictRef, PyIntRef, PyStrRef}; use crate::{ Context, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, class::PyClassImpl, - frame::{Frame, FrameOwner, FrameRef}, + frame::{FrameObject, FrameObjectRef, FrameOwner}, function::PySetterValue, types::Representable, }; +use core::sync::atomic::Ordering::Relaxed; use num_traits::Zero; +#[allow(unused_imports)] +use rustpython_common::atomic::Radium; use rustpython_compiler_core::bytecode::{self, Constant, Instruction, StackEffect}; use stack_analysis::*; @@ -426,10 +429,10 @@ pub(crate) mod stack_analysis { } pub(crate) fn init(context: &'static Context) { - Frame::extend_class(context, context.types.frame_type); + FrameObject::extend_class(context, context.types.frame_type); } -impl Representable for Frame { +impl Representable for FrameObject { #[inline] fn repr(_zelf: &Py, vm: &VirtualMachine) -> PyResult { const REPR: &str = ""; @@ -442,38 +445,86 @@ impl Representable for Frame { } } +impl FrameObject { + /// Find the live source InterpreterFrame on the TLS chain for a + /// materialized FrameObject. Returns the raw pointer if found, or null + /// if this FrameObject has no live source (already returned or not + /// currently executing on this thread). + pub(crate) fn find_live_source_iframe(&self) -> *const crate::frame::InterpreterFrame { + let self_py_ptr = unsafe { Py::::from_payload_ptr(self) } as usize; + let mut cur = crate::vm::thread::get_current_frame(); + while !cur.is_null() { + let materialized = unsafe { (*cur).materialized.load(Relaxed) }; + if materialized == self_py_ptr { + return cur; + } + cur = unsafe { &*cur }.previous(); + } + core::ptr::null() + } +} + #[pyclass(flags(DISALLOW_INSTANTIATION), with(Py))] -impl Frame { +impl FrameObject { #[pygetset] fn f_globals(&self) -> PyDictRef { - self.globals.clone() + self.iframe().globals().to_owned() } #[pygetset] fn f_builtins(&self) -> PyObjectRef { - self.builtins.clone() + self.iframe().builtins().to_owned() } #[pygetset] pub fn f_code(&self) -> PyRef { - self.code.clone() + self.iframe().code().to_owned() } #[pygetset] fn f_lasti(&self) -> u32 { - // Return byte offset (each instruction is 2 bytes) for compatibility - self.lasti() * 2 + // Return byte offset (each instruction is 2 bytes) for compatibility. + // For materialized frames, read live lasti from the source iframe on + // the TLS chain so f_lasti reflects the current execution position. + let live = self.find_live_source_iframe(); + let val = if !live.is_null() { + unsafe { (*live).lasti.load(Relaxed) } + } else { + self.lasti() + }; + val * 2 } #[pygetset] pub fn f_lineno(&self) -> usize { // If lasti is 0, execution hasn't started yet - use first line number - // Similar to PyCode_Addr2Line which returns co_firstlineno for addr_q < 0 if self.lasti() == 0 { - self.code.first_line_number.map_or(1, |n| n.get()) - } else { - self.current_location().line.get() + return self + .iframe() + .code() + .first_line_number + .map_or(1, |n| n.get()); } + // For executing frames (on the TLS chain), use prev_line which is + // updated at each bytecode instruction *before* the instruction + // runs. This gives the correct line even when observed mid-CALL + // (where lasti has already advanced past the CALL instruction). + let live = self.find_live_source_iframe(); + if !live.is_null() { + // Read live prev_line. Use read_volatile to bypass LLVM noalias + // on the &mut InterpreterFrame borrow in with_iframe. + let prev = unsafe { + let field_ptr = core::ptr::addr_of!((*live).prev_line); + core::ptr::read_volatile(field_ptr as *const u32) + }; + if prev > 0 { + return prev as usize; + } + } + // For returned frames, use lasti-based location lookup. This is + // correct for exception tracebacks where prev_line may have been + // updated by cleanup instructions after the exception. + self.current_location().line.get() } #[pygetset(setter)] @@ -492,7 +543,11 @@ impl Frame { } }; - let first_line = self.code.first_line_number.map_or(1, |n| n.get() as i32); + let first_line = self + .iframe() + .code() + .first_line_number + .map_or(1, |n| n.get() as i32); if l_new_lineno < first_line { return Err(vm.new_value_error(format!( @@ -500,7 +555,7 @@ impl Frame { ))); } - let py_code: &PyCode = &self.code; + let py_code: &PyCode = self.iframe().code(); let code = &py_code.code; let lines = mark_lines(code); @@ -513,13 +568,19 @@ impl Frame { } let stacks = mark_stacks(code); - let len = self.code.instructions.len(); + let len = self.iframe().code().instructions.len(); // lasti points past the current instruction (already incremented). // stacks[lasti - 1] gives the stack state before executing the // instruction that triggered this trace event, which is the current - // evaluation stack. - let current_lasti = self.lasti() as usize; + // evaluation stack. Read from the live iframe when available so the + // value reflects the actual execution position. + let live = self.find_live_source_iframe(); + let current_lasti = if !live.is_null() { + (unsafe { (*live).lasti.load(Relaxed) }) as usize + } else { + self.lasti() as usize + }; let start_idx = current_lasti.saturating_sub(1); let start_stack = if start_idx < stacks.len() { stacks[start_idx] @@ -567,36 +628,61 @@ impl Frame { } } - // Store the pending unwind for the execution loop to perform. - // We cannot pop stack entries here because the execution loop - // holds the state mutex, and trying to lock it again would deadlock. - self.set_pending_stack_pops(pop_count as u32); - self.set_pending_unwind_from_stack(start_stack); - - // Set lasti to best_addr. The executor will read lasti and execute - // the instruction at that index next. - self.set_lasti(best_addr as u32); + // Store the pending unwind and new lasti. When this frame is backed + // by a live stack-allocated iframe, write to the live iframe so the + // execution loop picks up the jump target. Reuse `live` from above. + let target = if !live.is_null() { + unsafe { &*live } + } else { + self.iframe() + }; + target.pending_stack_pops.store(pop_count as u32, Relaxed); + target.pending_unwind_from_stack.store(start_stack, Relaxed); + target.lasti.store(best_addr as u32, Relaxed); Ok(()) } #[pygetset] - fn f_trace(&self) -> PyObjectRef { - let boxed = self.trace.lock(); - boxed.clone() + fn f_trace(&self, vm: &VirtualMachine) -> PyObjectRef { + // Read from live source iframe if available. + let live = self.find_live_source_iframe(); + let trace = if !live.is_null() { + unsafe { &*live }.trace.lock().clone() + } else { + self.iframe().trace.lock().clone() + }; + trace.unwrap_or_else(|| vm.ctx.none()) } #[pygetset(setter)] fn set_f_trace(&self, value: PySetterValue, vm: &VirtualMachine) { - let mut storage = self.trace.lock(); - *storage = value.unwrap_or_none(vm); + let trace = match value { + PySetterValue::Assign(v) => { + if vm.is_none(&v) { + None + } else { + Some(v) + } + } + PySetterValue::Delete => None, + }; + // Set on the materialized FrameObject. + (*self.iframe().trace.lock()).clone_from(&trace); + // Also propagate to the live source iframe if this is a + // materialized copy of a stack-allocated frame, so pdb's + // f_trace assignment takes effect on the executing frame. + let live = self.find_live_source_iframe(); + if !live.is_null() { + *unsafe { &*live }.trace.lock() = trace; + } } #[expect(clippy::unnecessary_wraps, reason = "Needs to comply with a signature")] #[pymember(type = "bool")] fn f_trace_lines(vm: &VirtualMachine, zelf: PyObjectRef) -> PyResult { - let zelf: FrameRef = zelf.downcast().unwrap_or_else(|_| unreachable!()); + let zelf: FrameObjectRef = zelf.downcast().unwrap_or_else(|_| unreachable!()); - let boxed = zelf.trace_lines.lock(); + let boxed = zelf.iframe().trace_lines.lock(); Ok(vm.ctx.new_bool(*boxed).into()) } @@ -608,14 +694,19 @@ impl Frame { ) -> PyResult<()> { match value { PySetterValue::Assign(value) => { - let zelf: FrameRef = zelf.downcast().unwrap_or_else(|_| unreachable!()); + let zelf: FrameObjectRef = zelf.downcast().unwrap_or_else(|_| unreachable!()); let value: PyIntRef = value .downcast() .map_err(|_| vm.new_type_error("attribute value type must be bool"))?; - let mut trace_lines = zelf.trace_lines.lock(); - *trace_lines = !value.as_bigint().is_zero(); + let val = !value.as_bigint().is_zero(); + *zelf.iframe().trace_lines.lock() = val; + // Propagate to live source iframe. + let live = zelf.find_live_source_iframe(); + if !live.is_null() { + *unsafe { &*live }.trace_lines.lock() = val; + } Ok(()) } @@ -626,8 +717,8 @@ impl Frame { #[expect(clippy::unnecessary_wraps, reason = "Needs to comply with a signature")] #[pymember(type = "bool")] fn f_trace_opcodes(vm: &VirtualMachine, zelf: PyObjectRef) -> PyResult { - let zelf: FrameRef = zelf.downcast().unwrap_or_else(|_| unreachable!()); - let trace_opcodes = zelf.trace_opcodes.lock(); + let zelf: FrameObjectRef = zelf.downcast().unwrap_or_else(|_| unreachable!()); + let trace_opcodes = zelf.iframe().trace_opcodes.lock(); Ok(vm.ctx.new_bool(*trace_opcodes).into()) } @@ -639,14 +730,19 @@ impl Frame { ) -> PyResult<()> { match value { PySetterValue::Assign(value) => { - let zelf: FrameRef = zelf.downcast().unwrap_or_else(|_| unreachable!()); + let zelf: FrameObjectRef = zelf.downcast().unwrap_or_else(|_| unreachable!()); let value: PyIntRef = value .downcast() .map_err(|_| vm.new_type_error("attribute value type must be bool"))?; - let mut trace_opcodes = zelf.trace_opcodes.lock(); - *trace_opcodes = !value.as_bigint().is_zero(); + let val = !value.as_bigint().is_zero(); + *zelf.iframe().trace_opcodes.lock() = val; + // Propagate to live source iframe. + let live = zelf.find_live_source_iframe(); + if !live.is_null() { + *unsafe { &*live }.trace_opcodes.lock() = val; + } // TODO: Implement the equivalent of _PyEval_SetOpcodeTrace() @@ -658,11 +754,15 @@ impl Frame { } #[pyclass] -impl Py { +impl Py { #[pymethod] // = frame_clear_impl fn clear(&self, vm: &VirtualMachine) -> PyResult<()> { - let owner = FrameOwner::from_i8(self.owner.load(core::sync::atomic::Ordering::Acquire)); + let owner = FrameOwner::from_i8( + self.iframe() + .owner + .load(core::sync::atomic::Ordering::Acquire), + ); match owner { FrameOwner::Generator => { // Generator frame: check if suspended (lasti > 0 means @@ -677,12 +777,16 @@ impl Py { return Err(vm.new_runtime_error("cannot clear an executing frame")); } FrameOwner::FrameObject => { - // Detached frame: safe to clear. + // Check if this materialized frame is backed by a live + // stack-allocated iframe — if so, the frame is executing. + if !self.find_live_source_iframe().is_null() { + return Err(vm.new_runtime_error("cannot clear an executing frame")); + } } } // Clear fastlocals - // SAFETY: Frame is not executing (detached or stopped). + // SAFETY: FrameObject is not executing (detached or stopped). { let fastlocals = unsafe { self.fastlocals_mut() }; for slot in fastlocals.iter_mut() { @@ -694,10 +798,10 @@ impl Py { self.clear_stack_and_cells(); // Clear temporary refs - self.temporary_refs.lock().clear(); - self.f_locals_hidden_overlay.lock().take(); - self.f_extra_locals.lock().take(); - self.retained_back.lock().take(); + self.iframe().temporary_refs.lock().clear(); + self.iframe().f_locals_hidden_overlay.lock().take(); + self.iframe().f_extra_locals.lock().take(); + self.iframe().retained_back.lock().take(); Ok(()) } @@ -707,7 +811,12 @@ impl Py { // Optimized (function) frames expose a live write-through // FrameLocalsProxy; class/module/exec frames expose their namespace // mapping directly. - if self.code.flags.contains(bytecode::CodeFlags::OPTIMIZED) { + if self + .iframe() + .code() + .flags + .contains(bytecode::CodeFlags::OPTIMIZED) + { self.check_locals_access(vm)?; self.mark_escaped(); let proxy = crate::builtins::FrameLocalsProxy::new(self.to_owned()); @@ -719,85 +828,98 @@ impl Py { #[pygetset] fn f_generator(&self) -> Option { - self.generator.to_owned() + self.iframe().generator.to_owned() } #[pygetset] - pub fn f_back(&self, vm: &VirtualMachine) -> Option> { - #[cfg(not(feature = "threading"))] - let _ = vm; - let previous = self.previous_frame(); - if previous.is_null() { - return None; + pub fn f_back(&self, #[allow(unused)] vm: &VirtualMachine) -> Option> { + let mut prev = self.previous_iframe(); + + // For materialized frames (previous == 0), find the source iframe on + // the TLS chain and use its `previous` instead. + if prev.is_null() { + // materialized stores `*const Py` as usize. + // `self` is `&Py` — compare addresses directly. + let self_py_ptr = self as *const Self as usize; + let mut cur = crate::vm::thread::get_current_frame(); + while !cur.is_null() { + let materialized = unsafe { (*cur).materialized.load(Relaxed) }; + if materialized == self_py_ptr { + // Found the source iframe — use its previous + prev = unsafe { (*cur).previous() }; + break; + } + cur = unsafe { (*cur).previous() }; + } + if prev.is_null() { + // Check retained_back for frames whose callers have returned + let retained = self.iframe().retained_back.lock().clone(); + if let Some(frame) = retained { + frame.mark_escaped(); + return Some(frame); + } + return None; + } } - // Look for the caller on the current thread's signal-safe frame chain. - // Finding it there proves it is still live on this thread. - if let Some(frame) = crate::frame::find_owned_chain_frame(previous) { - frame.mark_escaped(); - return Some(frame); + // Walk the TLS chain to find the prev iframe and materialize it. + // This handles both heap-allocated FrameObjects and stack-allocated + // iframes that haven't been observed yet. + { + let mut cur = crate::vm::thread::get_current_frame(); + while !cur.is_null() { + if core::ptr::eq(cur, prev) { + let iframe_ref = unsafe { &*cur }; + let fo = iframe_ref.materialize(vm); + fo.mark_escaped(); + return Some(fo.to_owned()); + } + cur = unsafe { (*cur).previous() }; + } } - // The caller already returned and left the live chain, but this frame - // escaped and retained a strong reference to it at release time. - let retained = self.retained_back.lock().clone(); + // The caller already returned — check retained_back + let retained = self.iframe().retained_back.lock().clone(); if let Some(frame) = retained { frame.mark_escaped(); return Some(frame); } - // The caller lives on another thread. unix: park every thread under - // stop-the-world so their frame chains are quiescent and alive, then - // walk each published top frame down its `previous` chain looking for - // the caller. Request stop-the-world before the registry lock. - #[cfg(all(unix, feature = "threading"))] + // The caller lives on another thread. Use stop-the-world to + // safely materialize the cross-thread frame chain. + #[cfg(feature = "threading")] { - use core::sync::atomic::Ordering; + // Enter STW before dereferencing `prev` — the owning thread may + // return and free the stack-allocated iframe at any time. vm.state.stop_the_world.stop_the_world(vm); scopeguard::defer! { vm.state.stop_the_world.start_the_world(vm); } - let registry = vm.state.thread_frames.lock(); - #[expect( - clippy::iter_over_hash_type, - reason = "Iteration order doesn't matter here" - )] - for slot in registry.values() { - let mut cur = slot.top_frame.load(Ordering::Relaxed) as *const Frame; - while !cur.is_null() { - if core::ptr::eq(cur, previous) { - // SAFETY: world stopped -> this frame is alive on its - // owning thread's parked call stack. - let f = unsafe { &*Self::from_payload_ptr(cur) }; - f.mark_escaped(); - return Some(f.to_owned()); + let prev_ref = unsafe { &*prev }; + // Fast path: already materialized. + if let Some(fo) = prev_ref.frame_obj() { + fo.mark_escaped(); + return Some(fo.to_owned()); + } + // Slow path: materialize the entire chain and link retained_back. + let mut cur = prev; + let mut child_fo: Option> = None; + while !cur.is_null() { + let iframe = unsafe { &*cur }; + let fo = iframe.materialize(vm).to_owned(); + if let Some(child) = child_fo.take() { + let mut guard = child.iframe().retained_back.lock(); + if guard.is_none() { + *guard = Some(fo.clone()); } - // SAFETY: chain frames on a parked thread are alive. - cur = unsafe { (*cur).previous_frame() }; - } - } - } - - #[cfg(all(not(unix), feature = "threading"))] - { - let registry = vm.state.thread_frames.lock(); - #[expect( - clippy::iter_over_hash_type, - reason = "Iteration order doesn't matter here" - )] - for slot in registry.values() { - let frames = slot.frames.lock(); - // SAFETY: the owning thread can't pop while we hold the Mutex, - // so FramePtr is valid for the duration of the lock. - if let Some(frame) = frames.iter().find_map(|fp| { - let f = unsafe { fp.as_ref() }; - let ptr: *const Frame = &**f; - core::ptr::eq(ptr, previous).then(|| f.to_owned()) - }) { - frame.mark_escaped(); - return Some(frame); } + child_fo = Some(fo); + cur = iframe.previous(); } + let fo = prev_ref.materialize(vm); + fo.mark_escaped(); + return Some(fo.to_owned()); } + #[allow(unreachable_code)] None } } diff --git a/crates/vm/src/builtins/frame_locals_proxy.rs b/crates/vm/src/builtins/frame_locals_proxy.rs index fbbc7f5d9..eaba0bca6 100644 --- a/crates/vm/src/builtins/frame_locals_proxy.rs +++ b/crates/vm/src/builtins/frame_locals_proxy.rs @@ -7,7 +7,7 @@ use crate::{ AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, atomic_func, class::PyClassImpl, - frame::FrameRef, + frame::FrameObjectRef, function::{FuncArgs, OptionalArg, PyArithmeticValue, PyComparisonValue}, object::{Traverse, TraverseFn}, protocol::{PyMappingMethods, PyNumberMethods, PySequenceMethods}, @@ -23,7 +23,7 @@ use rustpython_common::wtf8::Wtf8Buf; #[pyclass(module = false, name = "FrameLocalsProxy", traverse = "manual")] #[derive(Debug)] pub struct FrameLocalsProxy { - frame: FrameRef, + frame: FrameObjectRef, } unsafe impl Traverse for FrameLocalsProxy { @@ -40,7 +40,7 @@ impl PyPayload for FrameLocalsProxy { } impl FrameLocalsProxy { - pub(crate) fn new(frame: FrameRef) -> Self { + pub(crate) fn new(frame: FrameObjectRef) -> Self { Self { frame } } @@ -67,7 +67,7 @@ impl Constructor for FrameLocalsProxy { args.len() ))); } - let frame: FrameRef = args + let frame: FrameObjectRef = args .pop() .unwrap() .downcast() diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index 43cc33a90..39195202a 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -14,7 +14,7 @@ use crate::{ bytecode, class::PyClassImpl, common::wtf8::{Wtf8Buf, wtf8_concat}, - frame::{Frame, FrameRef}, + frame::{FrameObject, FrameObjectRef}, function::{FuncArgs, OptionalArg, PyComparisonValue, PySetterValue}, scope::Scope, types::{ @@ -182,11 +182,7 @@ impl PyFunction { let module = vm.unwrap_or_none(globals.get_item_opt(identifier!(vm, __name__), vm)?); let builtins = globals.get_item("__builtins__", vm).unwrap_or_else(|_| { // If not in globals, inherit from current execution context - if let Some(frame) = vm.current_frame() { - frame.builtins.clone() - } else { - vm.builtins.dict().into() - } + crate::frame::current_builtins().unwrap_or_else(|| vm.builtins.dict().into()) }); // If builtins is a module, use its __dict__ instead let builtins = if let Some(module) = builtins.downcast_ref::() { @@ -228,7 +224,28 @@ impl PyFunction { fn fill_locals_from_args( &self, - frame: &Frame, + frame: &FrameObject, + func_args: FuncArgs, + vm: &VirtualMachine, + ) -> PyResult<()> { + // SAFETY: FrameObject was just created and not yet executing. + let fastlocals = unsafe { frame.fastlocals_mut() }; + self.fill_locals_from_args_inner(fastlocals, func_args, vm) + } + + fn fill_locals_from_args_iframe( + &self, + iframe: &mut crate::frame::InterpreterFrame, + func_args: FuncArgs, + vm: &VirtualMachine, + ) -> PyResult<()> { + let fastlocals = iframe.localsplus.fastlocals_mut(); + self.fill_locals_from_args_inner(fastlocals, func_args, vm) + } + + fn fill_locals_from_args_inner( + &self, + fastlocals: &mut [Option], func_args: FuncArgs, vm: &VirtualMachine, ) -> PyResult<()> { @@ -236,16 +253,6 @@ impl PyFunction { let nargs = func_args.args.len(); let n_expected_args = code.arg_count as usize; let total_args = code.arg_count as usize + code.kwonlyarg_count as usize; - // let arg_names = self.code.arg_names(); - - // This parses the arguments from args and kwargs into - // the proper variables keeping into account default values - // and star-args and kwargs. - // See also: PyEval_EvalCodeWithName in cpython: - // https://github.com/python/cpython/blob/main/Python/ceval.c#L3681 - - // SAFETY: Frame was just created and not yet executing. - let fastlocals = unsafe { frame.fastlocals_mut() }; let mut args_iter = func_args.args.into_iter(); @@ -564,69 +571,114 @@ impl Py { } } - let code: PyRef = (*self.code).to_owned(); - - let locals = if code.flags.contains(bytecode::CodeFlags::NEWLOCALS) { - None - } else if let Some(locals) = locals { - Some(locals) - } else { - Some(ArgMapping::from_dict_exact(self.globals.clone())) - }; + let code = &*self.code; let is_gen = code.flags.contains(bytecode::CodeFlags::GENERATOR); let is_coro = code.flags.contains(bytecode::CodeFlags::COROUTINE); let is_async_gen = code.flags.contains(bytecode::CodeFlags::ASYNC_GENERATOR); - let use_datastack = !(is_gen || is_coro || is_async_gen); - // Construct frame: - let frame = Frame::new( - code, - Scope::new(locals, self.globals.clone()), - self.builtins.clone(), - self.closure.as_ref().map_or(&[], |c| c.as_slice()), - Some(self.to_owned().into()), - use_datastack, - vm, - ) - .into_ref(&vm.ctx); + let needs_heap_frame = is_gen || is_coro || is_async_gen || vm.use_tracing.get(); - self.fill_locals_from_args(&frame, func_args, vm)?; - if use_datastack { - let result = vm.run_frame(frame.clone()); - // Release data stack memory after frame execution completes. - crate::frame::release_datastack_frame(&frame, vm); - result - } else { - let obj = if is_async_gen { - PyAsyncGen::new(frame.clone(), self.__name__(), self.__qualname__()) - .into_pyobject(vm) - } else if is_gen { - PyGenerator::new(frame.clone(), self.__name__(), self.__qualname__()) - .into_pyobject(vm) + if needs_heap_frame { + // Heap-allocate FrameObject for generators/coroutines (lifetime + // exceeds call stack) or when tracing is active (trace callbacks + // need a FrameObject). + let code_owned: PyRef = code.to_owned(); + let locals = if code.flags.contains(bytecode::CodeFlags::NEWLOCALS) { + None + } else if let Some(locals) = locals { + Some(locals) } else { - PyCoroutine::new(frame.clone(), self.__name__(), self.__qualname__()) - .into_pyobject(vm) + Some(ArgMapping::from_dict_exact(self.globals.clone())) }; - // Generator/coroutine frames outlive this call and can join a - // reference cycle through their owning generator, so they must - // participate in the GC. They were created untracked - // (NEW_REF_UNTRACKED); track them now, before the back-reference - // is installed. Their localsplus is heap-backed by construction - // (use_datastack == false), so a collector never reads data-stack - // storage when it traverses them. - debug_assert!( - !frame.localsplus_is_datastack_backed(), - "generator frame is data-stack-backed" + let use_datastack = !is_gen && !is_coro && !is_async_gen; + let frame = FrameObject::new_ref( + code_owned, + Scope::new(locals, self.globals.clone()), + self.builtins.clone(), + self.closure.as_ref().map_or(&[], |c| c.as_slice()), + Some(self.to_owned().into()), + use_datastack, + vm, ); - // SAFETY: the frame is alive (held by `frame`) and untracked. - unsafe { - crate::gc_state::gc_state() - .track_object(core::ptr::NonNull::from(frame.as_object())); + self.fill_locals_from_args(&frame, func_args, vm)?; + if is_gen || is_coro || is_async_gen { + return Ok(self.make_generator_or_coro(frame, vm)); } - frame.set_generator(&obj); - Ok(obj) + // Tracing active: use heap frame with full trace support. + let result = vm.run_frame(frame.clone()); + unsafe { + if let Some(base) = frame.iframe_mut().localsplus.release_datastack() { + vm.datastack_pop(base); + } + } + return result; } + + // Fast path: stack-allocated InterpreterFrame, no FrameObject. + // No refcount inc for code — it's alive via self.code for the call duration. + let nlocalsplus = code.localspluskinds.len(); + let max_stackdepth = code.max_stackdepth as usize; + let localsplus = + crate::frame::LocalsPlus::new_on_datastack(nlocalsplus, max_stackdepth, vm); + + let locals = if code.flags.contains(bytecode::CodeFlags::NEWLOCALS) { + crate::frame::FrameLocals::lazy() + } else if let Some(locals) = locals { + crate::frame::FrameLocals::with_locals(locals) + } else { + crate::frame::FrameLocals::with_locals(crate::function::ArgMapping::from_dict_exact( + self.globals.clone(), + )) + }; + + // Use self.as_object() as raw pointer — no refcount inc/dec. + // The function is alive on the caller's stack for the call duration. + let mut iframe = crate::frame::InterpreterFrame::new( + &self.code, + &self.globals, + &self.builtins, + Some(self.as_object()), + localsplus, + locals, + self.closure.as_ref().map_or(&[], |c| c.as_slice()), + crate::frame::FrameOwner::Thread, + ); + let result = self + .fill_locals_from_args_iframe(&mut iframe, func_args, vm) + .and_then(|()| vm.run_frame_fast(&mut iframe)); + // Release data stack memory — must happen on both success and error. + unsafe { + if let Some(base) = iframe.localsplus.release_datastack() { + vm.datastack_pop(base); + } + } + result + } + + /// Create generator, coroutine, or async generator from a FrameObject. + fn make_generator_or_coro(&self, frame: FrameObjectRef, vm: &VirtualMachine) -> PyObjectRef { + let code = frame.iframe().code(); + let is_async_gen = code.flags.contains(bytecode::CodeFlags::ASYNC_GENERATOR); + let is_gen = code.flags.contains(bytecode::CodeFlags::GENERATOR); + + let obj = if is_async_gen { + PyAsyncGen::new(frame.clone(), self.__name__(), self.__qualname__()).into_pyobject(vm) + } else if is_gen { + PyGenerator::new(frame.clone(), self.__name__(), self.__qualname__()).into_pyobject(vm) + } else { + PyCoroutine::new(frame.clone(), self.__name__(), self.__qualname__()).into_pyobject(vm) + }; + debug_assert!( + !frame.localsplus_is_datastack_backed(), + "generator frame is data-stack-backed" + ); + // SAFETY: the frame is alive (held by `frame`) and untracked. + unsafe { + crate::gc_state::gc_state().track_object(core::ptr::NonNull::from(frame.as_object())); + } + frame.set_generator(&obj); + obj } #[inline(always)] @@ -696,7 +748,7 @@ impl Py { &self, args: impl ExactSizeIterator, vm: &VirtualMachine, - ) -> FrameRef { + ) -> FrameObjectRef { let code: PyRef = (*self.code).to_owned(); debug_assert_eq!(args.len(), code.arg_count as usize); @@ -719,7 +771,7 @@ impl Py { Some(ArgMapping::from_dict_exact(self.globals.clone())) }; - let frame = Frame::new( + let frame = FrameObject::new_ref( code, Scope::new(locals, self.globals.clone()), self.builtins.clone(), @@ -727,8 +779,7 @@ impl Py { Some(self.to_owned().into()), true, // Exact-args fast path is only used for non-gen/coro functions. vm, - ) - .into_ref(&vm.ctx); + ); { let fastlocals = unsafe { frame.fastlocals_mut() }; @@ -745,10 +796,45 @@ impl Py { args: impl ExactSizeIterator, vm: &VirtualMachine, ) -> PyResult { - let frame = self.prepare_exact_args_frame(args, vm); + let code = &*self.code; + let nlocalsplus = code.localspluskinds.len(); + let max_stackdepth = code.max_stackdepth as usize; + let localsplus = + crate::frame::LocalsPlus::new_on_datastack(nlocalsplus, max_stackdepth, vm); - let result = vm.run_frame(frame.clone()); - crate::frame::release_datastack_frame(&frame, vm); + let locals = if code.flags.contains(bytecode::CodeFlags::NEWLOCALS) { + crate::frame::FrameLocals::lazy() + } else { + crate::frame::FrameLocals::with_locals(ArgMapping::from_dict_exact( + self.globals.clone(), + )) + }; + + let mut iframe = crate::frame::InterpreterFrame::new( + code, + &self.globals, + &self.builtins, + Some(self.as_object()), + localsplus, + locals, + self.closure.as_ref().map_or(&[], |c| c.as_slice()), + crate::frame::FrameOwner::Thread, + ); + + // Fill arguments directly into fastlocals + { + let fastlocals = iframe.localsplus.fastlocals_mut(); + for (slot, arg) in fastlocals.iter_mut().zip(args) { + *slot = Some(arg); + } + } + + let result = vm.run_frame_fast(&mut iframe); + unsafe { + if let Some(base) = iframe.localsplus.release_datastack() { + vm.datastack_pop(base); + } + } result } diff --git a/crates/vm/src/builtins/generator.rs b/crates/vm/src/builtins/generator.rs index 6b16917e3..52db3c952 100644 --- a/crates/vm/src/builtins/generator.rs +++ b/crates/vm/src/builtins/generator.rs @@ -7,7 +7,7 @@ use crate::{ AsObject, Context, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, class::PyClassImpl, coroutine::{Coro, warn_deprecated_throw_signature}, - frame::FrameRef, + frame::FrameObjectRef, function::OptionalArg, object::{Traverse, TraverseFn}, protocol::PyIterReturn, @@ -43,7 +43,7 @@ impl PyGenerator { } #[must_use] - pub fn new(frame: FrameRef, name: PyStrRef, qualname: PyStrRef) -> Self { + pub fn new(frame: FrameObjectRef, name: PyStrRef, qualname: PyStrRef) -> Self { Self { inner: Coro::new(frame, name, qualname), } @@ -70,7 +70,7 @@ impl PyGenerator { } #[pygetset] - fn gi_frame(&self, _vm: &VirtualMachine) -> Option { + fn gi_frame(&self, _vm: &VirtualMachine) -> Option { if self.inner.closed() { None } else { @@ -85,7 +85,7 @@ impl PyGenerator { #[pygetset] fn gi_code(&self, _vm: &VirtualMachine) -> PyRef { - self.inner.frame().code.clone() + self.inner.frame().iframe().code().to_owned() } #[pygetset] diff --git a/crates/vm/src/builtins/super.rs b/crates/vm/src/builtins/super.rs index 620363966..f71bf51a6 100644 --- a/crates/vm/src/builtins/super.rs +++ b/crates/vm/src/builtins/super.rs @@ -79,22 +79,27 @@ impl Initializer for PySuper { let (typ, obj) = if let OptionalArg::Present(ty) = py_type { (ty, py_obj.unwrap_or_none(vm)) } else { - let frame = vm - .current_frame() - .ok_or_else(|| vm.new_runtime_error("super(): no current frame"))?; + // Access the InterpreterFrame directly — no need to materialize + // a FrameObject just to read code/locals. + let iframe_ptr = crate::vm::thread::get_current_frame(); + if iframe_ptr.is_null() { + return Err(vm.new_runtime_error("super(): no current frame")); + } + let iframe = unsafe { &*iframe_ptr }; + let code = iframe.code(); - if frame.code.arg_count == 0 { + if code.arg_count == 0 { return Err(vm.new_runtime_error("super(): no arguments")); } - // SAFETY: Frame is current and not concurrently mutated. + // SAFETY: InterpreterFrame is current and not concurrently mutated. use rustpython_compiler_core::bytecode::CO_FAST_CELL; - let obj = unsafe { frame.fastlocals() }[0] + let fastlocals = iframe.localsplus.fastlocals(); + let obj = fastlocals[0] .clone() .and_then(|val| { // If slot 0 is a merged cell (LOCAL|CELL), extract value from cell - if frame - .code + if code .localspluskinds .first() .is_some_and(|&k| k & CO_FAST_CELL != 0) @@ -108,13 +113,15 @@ impl Initializer for PySuper { let mut typ = None; // Search for __class__ in freevars using localspluskinds - let nlocalsplus = frame.code.localspluskinds.len(); - let nfrees = frame.code.freevars.len(); + let nlocalsplus = code.localspluskinds.len(); + let nfrees = code.freevars.len(); let free_start = nlocalsplus - nfrees; - for (i, var) in frame.code.freevars.iter().enumerate() { + for (i, var) in code.freevars.iter().enumerate() { if var.as_bytes() == b"__class__" { - let class = frame - .get_cell_contents(free_start + i) + let class = fastlocals[free_start + i] + .as_ref() + .and_then(|v| v.downcast_ref::()) + .and_then(|c| c.get()) .ok_or_else(|| vm.new_runtime_error("super(): empty __class__ cell"))?; typ = Some(class.downcast().map_err(|o| { vm.new_type_error(format!( diff --git a/crates/vm/src/builtins/traceback.rs b/crates/vm/src/builtins/traceback.rs index b31de874b..0e93768dc 100644 --- a/crates/vm/src/builtins/traceback.rs +++ b/crates/vm/src/builtins/traceback.rs @@ -1,7 +1,7 @@ use super::{PyList, PyType}; use crate::{ AsObject, Context, Py, PyPayload, PyRef, PyResult, VirtualMachine, class::PyClassImpl, - frame::FrameRef, function::PySetterValue, types::Constructor, + frame::FrameObjectRef, function::PySetterValue, types::Constructor, }; use rustpython_common::lock::PyMutex; use rustpython_compiler_core::OneIndexed; @@ -10,7 +10,7 @@ use rustpython_compiler_core::OneIndexed; #[derive(Debug)] pub struct PyTraceback { pub next: PyMutex>, - pub frame: FrameRef, + pub frame: FrameObjectRef, #[pytraverse(skip)] pub lasti: u32, #[pytraverse(skip)] @@ -31,7 +31,7 @@ impl PyTraceback { #[must_use] pub const fn new( next: Option>, - frame: FrameRef, + frame: FrameObjectRef, lasti: u32, lineno: OneIndexed, ) -> Self { @@ -44,7 +44,7 @@ impl PyTraceback { } #[pygetset] - fn tb_frame(&self) -> FrameRef { + fn tb_frame(&self) -> FrameObjectRef { self.frame.clone() } @@ -104,7 +104,7 @@ impl PyTraceback { } impl Constructor for PyTraceback { - type Args = (Option>, FrameRef, u32, usize); + type Args = (Option>, FrameObjectRef, u32, usize); fn py_new(_cls: &Py, args: Self::Args, vm: &VirtualMachine) -> PyResult { let (next, frame, lasti, lineno) = args; @@ -130,9 +130,12 @@ impl serde::Serialize for PyTraceback { use serde::ser::SerializeStruct; let mut struc = s.serialize_struct("PyTraceback", 3)?; - struc.serialize_field("name", self.frame.code.obj_name.as_str())?; + struc.serialize_field("name", self.frame.iframe().code().obj_name.as_str())?; struc.serialize_field("lineno", &self.lineno.get())?; - struc.serialize_field("filename", self.frame.code.source_path().as_str())?; + struc.serialize_field( + "filename", + self.frame.iframe().code().source_path().as_str(), + )?; struc.end() } } diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index c27c6475f..0ccfe155d 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -1513,7 +1513,7 @@ impl PyType { // temporary refs so they never see a dangling pointer. let keep_alive = |type_ref: PyTypeRef, retired: &mut Vec| { if let Some(frame) = vm.current_frame() { - frame.temporary_refs.lock().push(type_ref.into()); + frame.iframe().temporary_refs.lock().push(type_ref.into()); } else { retired.push(type_ref.into()); } @@ -2183,14 +2183,11 @@ impl Constructor for PyType { *f = PyStaticMethod::from(f.clone()).into_pyobject(vm); } - if let Some(current_frame) = vm.current_frame() { + if let Some(globals) = crate::frame::current_globals() { let entry = attributes.entry(identifier!(vm, __module__)); if matches!(entry, Entry::Vacant(_)) { - let module_name = vm.unwrap_or_none( - current_frame - .globals - .get_item_opt(identifier!(vm, __name__), vm)?, - ); + let module_name = + vm.unwrap_or_none(globals.get_item_opt(identifier!(vm, __name__), vm)?); entry.or_insert(module_name); } } diff --git a/crates/vm/src/coroutine.rs b/crates/vm/src/coroutine.rs index 844887f85..c7252c66d 100644 --- a/crates/vm/src/coroutine.rs +++ b/crates/vm/src/coroutine.rs @@ -3,7 +3,7 @@ use crate::{ builtins::PyStrRef, common::lock::PyMutex, exceptions::types::PyBaseException, - frame::{ExecutionResult, Frame, FrameOwner, FrameRef}, + frame::{ExecutionResult, FrameObject, FrameObjectRef, FrameOwner}, function::OptionalArg, object::{PyAtomicRef, Traverse, TraverseFn}, protocol::PyIterReturn, @@ -29,7 +29,7 @@ impl ExecutionResult { #[derive(Debug)] pub struct Coro { - frame: FrameRef, + frame: FrameObjectRef, pub closed: AtomicCell, // TODO: https://github.com/RustPython/RustPython/pull/3183#discussion_r720560652 running: AtomicCell, // code @@ -62,7 +62,7 @@ fn gen_name(jen: &PyObject, vm: &VirtualMachine) -> &'static str { } impl Coro { - pub fn new(frame: FrameRef, name: PyStrRef, qualname: PyStrRef) -> Self { + pub fn new(frame: FrameObjectRef, name: PyStrRef, qualname: PyStrRef) -> Self { Self { frame, closed: AtomicCell::new(false), @@ -94,8 +94,8 @@ impl Coro { match res { Ok(ExecutionResult::Return(_)) | Err(_) => { self.closed.store(true); - // Frame is no longer suspended; allow frame.clear() to succeed. - self.frame.owner.store( + // FrameObject is no longer suspended; allow frame.clear() to succeed. + self.frame.iframe().owner.store( FrameOwner::FrameObject as i8, core::sync::atomic::Ordering::Release, ); @@ -114,7 +114,7 @@ impl Coro { func: F, ) -> (PyResult, bool) where - F: FnOnce(&Py) -> PyResult, + F: FnOnce(&Py) -> PyResult, { if self.running.compare_exchange(false, true).is_err() { return ( @@ -290,7 +290,7 @@ impl Coro { self.closed.load() } - pub fn frame(&self) -> FrameRef { + pub fn frame(&self) -> FrameObjectRef { self.frame.clone() } @@ -337,7 +337,8 @@ pub(crate) fn get_awaitable_iter(obj: PyObjectRef, vm: &VirtualMachine) -> PyRes || obj.downcast_ref::().is_some_and(|g| { g.as_coro() .frame() - .code + .iframe() + .code() .flags .contains(crate::bytecode::CodeFlags::ITERABLE_COROUTINE) }) @@ -352,7 +353,8 @@ pub(crate) fn get_awaitable_iter(obj: PyObjectRef, vm: &VirtualMachine) -> PyRes || result.downcast_ref::().is_some_and(|g| { g.as_coro() .frame() - .code + .iframe() + .code() .flags .contains(crate::bytecode::CodeFlags::ITERABLE_COROUTINE) }) diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index 7eaf8bafc..4facffee1 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -401,13 +401,13 @@ fn write_traceback_entry( output: &mut W, tb_entry: &Py, ) -> Result<(), W::Error> { - let filename = tb_entry.frame.code.source_path().as_str(); + let filename = tb_entry.frame.iframe().code().source_path().as_str(); writeln!( output, r##" File "{}", line {}, in {}"##, filename.trim_start_matches(r"\\?\"), tb_entry.lineno, - tb_entry.frame.code.obj_name + tb_entry.frame.iframe().code().obj_name )?; #[cfg(feature = "host_env")] diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 5df8b85e5..5c21ad29c 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -38,7 +38,6 @@ use bstr::ByteSlice; use core::cell::UnsafeCell; use core::ptr::NonNull; use core::sync::atomic; -use core::sync::atomic::AtomicPtr; use core::sync::atomic::Ordering::{Acquire, Relaxed}; use itertools::Itertools; use malachite_bigint::BigInt; @@ -48,72 +47,152 @@ use rustpython_common::{ lock::{OnceCell, PyMutex}, wtf8::{Wtf8, Wtf8Buf, wtf8_concat}, }; -use rustpython_compiler_core::SourceLocation; +use rustpython_compiler_core::{OneIndexed, SourceLocation}; -pub type FrameRef = PyRef; +pub type FrameObjectRef = PyRef; -/// Recover an owned reference to a live chain frame, or `None` for null. +// -- Frame chain utilities -- +// The frame chain is a singly-linked list of `*const InterpreterFrame` stored +// as `usize` values in `InterpreterFrame.previous` and the TLS +// `CURRENT_FRAME`. Null (0) marks the end. +// Each InterpreterFrame has a `materialized` pointer that is non-null when +// a FrameObject wraps it (always the case today; stack-allocated frames will +// leave it null until observed). + +/// Recover an owned reference to the FrameObject that wraps the given +/// InterpreterFrame, or `None` if null or not materialized. /// /// # Safety -/// A non-null `frame` must reference a frame that is live on the current -/// thread's execution chain, so the object outlives this call. -unsafe fn owned_chain_frame(frame: *const Frame) -> Option { - if frame.is_null() { +/// A non-null `iframe` must reference a live InterpreterFrame on the current +/// thread's execution chain. +unsafe fn owned_chain_frame(iframe: *const InterpreterFrame) -> Option { + if iframe.is_null() { return None; } - // SAFETY: caller guarantees the frame is live; from_payload_ptr recovers - // the enclosing object from the payload address. - let py = unsafe { &*Py::::from_payload_ptr(frame) }; - Some(py.to_owned()) + let iframe_ref = unsafe { &*iframe }; + let fo = iframe_ref.frame_obj()?; + Some(fo.to_owned()) } /// The current thread's topmost frame object, if any. #[must_use] -pub fn current_thread_frame() -> Option { - // SAFETY: the chain top executes on this thread, hence is alive. - unsafe { owned_chain_frame(crate::vm::thread::get_current_frame()) } +pub fn current_thread_frame() -> Option { + let ptr = crate::vm::thread::get_current_frame(); + unsafe { owned_chain_frame(ptr) } +} + +/// Get the current thread's topmost InterpreterFrame pointer. +#[must_use] +pub fn current_thread_iframe() -> *const InterpreterFrame { + crate::vm::thread::get_current_frame() +} + +/// The current thread's topmost frame object, materializing if necessary. +/// Unlike `current_thread_frame()`, this always returns `Some` if there is +/// an active frame, even if it's stack-allocated and hasn't been observed yet. +/// Uses `vm.current_frame` Cell for fast lookup (no TLS). +#[must_use] +pub fn current_thread_frame_materialize(vm: &VirtualMachine) -> Option { + let ptr = crate::vm::thread::get_current_frame(); + if ptr.is_null() { + return None; + } + let iframe = unsafe { &*ptr }; + Some(iframe.materialize(vm).to_owned()) +} + +/// Read the globals dict from the topmost frame on this thread's chain. +/// Returns `None` if the chain is empty. +#[must_use] +pub fn current_globals() -> Option { + let ptr = crate::vm::thread::get_current_frame(); + if ptr.is_null() { + return None; + } + Some(unsafe { (*ptr).globals().to_owned() }) +} + +/// Read the code object from the topmost frame on this thread's chain. +/// Returns `None` if the chain is empty. +#[must_use] +pub fn current_code() -> Option> { + let ptr = crate::vm::thread::get_current_frame(); + if ptr.is_null() { + return None; + } + Some(unsafe { (*ptr).code().to_owned() }) +} + +/// Read the builtins object from the topmost frame on this thread's chain. +#[must_use] +pub fn current_builtins() -> Option { + let ptr = crate::vm::thread::get_current_frame(); + if ptr.is_null() { + return None; + } + Some(unsafe { (*ptr).builtins().to_owned() }) } /// The frame `offset` positions below the current thread's top frame (offset 0 /// is the top), or `None` if the stack is not that deep. +/// Materializes the FrameObject on demand for stack-allocated frames. #[must_use] -pub fn frame_at_offset(offset: usize) -> Option { - let mut cur = crate::vm::thread::get_current_frame(); - for _ in 0..offset { - if cur.is_null() { - return None; - } - // SAFETY: chain frames are alive on the current thread's stack. - cur = unsafe { (*cur).previous_frame() }; - } - // SAFETY: same as above. - unsafe { owned_chain_frame(cur) } -} - -/// If `target` is a frame on the current thread's chain, return an owned -/// reference to it; otherwise `None`. Presence on the chain proves liveness. -#[must_use] -pub fn find_owned_chain_frame(target: *const Frame) -> Option { +pub fn frame_at_offset(offset: usize, vm: &VirtualMachine) -> Option { let mut cur = crate::vm::thread::get_current_frame(); + let mut remaining = offset; while !cur.is_null() { - if core::ptr::eq(cur, target) { - // SAFETY: a frame on the current thread's chain is alive. - return unsafe { owned_chain_frame(cur) }; + if remaining == 0 { + let iframe = unsafe { &*cur }; + return Some(iframe.materialize(vm).to_owned()); } - // SAFETY: chain frames are alive on the current thread's stack. - cur = unsafe { (*cur).previous_frame() }; + remaining -= 1; + cur = unsafe { (*cur).previous.load(Relaxed) as *const InterpreterFrame }; } None } -/// Invoke `f` for each frame on the current thread's chain, from the topmost -/// frame down to the bottom. -pub fn for_each_current_frame(mut f: impl FnMut(&Py)) { +/// If a FrameObject wrapping `target` InterpreterFrame is on the current +/// thread's chain, return an owned reference to it; otherwise `None`. +#[must_use] +pub fn find_owned_chain_frame_by_iframe(target: *const InterpreterFrame) -> Option { let mut cur = crate::vm::thread::get_current_frame(); while !cur.is_null() { - // SAFETY: chain frames are alive on the current thread's stack. - f(unsafe { &*Py::::from_payload_ptr(cur) }); - cur = unsafe { (*cur).previous_frame() }; + if core::ptr::eq(cur, target) { + return unsafe { owned_chain_frame(cur) }; + } + cur = unsafe { (*cur).previous.load(Relaxed) as *const InterpreterFrame }; + } + None +} + +/// If `target` FrameObject is on the current thread's chain, return an +/// owned reference to it; otherwise `None`. Presence on the chain proves liveness. +#[must_use] +pub fn find_owned_chain_frame(target: *const FrameObject) -> Option { + let mut cur = crate::vm::thread::get_current_frame(); + while !cur.is_null() { + let iframe_ref = unsafe { &*cur }; + if let Some(fo) = iframe_ref.frame_obj() { + let fo_payload: *const FrameObject = &**fo; + if core::ptr::eq(fo_payload, target) { + return Some(fo.to_owned()); + } + } + cur = iframe_ref.previous.load(Relaxed) as *const InterpreterFrame; + } + None +} + +/// Invoke `f` for each frame on the current thread's chain, from the +/// topmost frame down to the bottom. +pub fn for_each_current_frame(mut f: impl FnMut(&Py)) { + let mut cur = crate::vm::thread::get_current_frame(); + while !cur.is_null() { + let iframe_ref = unsafe { &*cur }; + if let Some(fo) = iframe_ref.frame_obj() { + f(fo); + } + cur = iframe_ref.previous.load(Relaxed) as *const InterpreterFrame; } } @@ -157,7 +236,7 @@ impl FrameOwner { /// Lock-free mutable storage for frame-internal data. /// /// # Safety -/// Frame execution is single-threaded: only one thread at a time executes +/// FrameObject execution is single-threaded: only one thread at a time executes /// a given frame (enforced by the owner field and generator running flag). /// External readers (e.g. `f_locals`) are on the same thread as execution /// (trace callback) or the frame is not executing. @@ -182,7 +261,7 @@ impl FrameUnsafeCell { } } -// SAFETY: Frame execution is single-threaded. See FrameUnsafeCell doc. +// SAFETY: FrameObject execution is single-threaded. See FrameUnsafeCell doc. #[cfg(feature = "threading")] unsafe impl Send for FrameUnsafeCell {} #[cfg(feature = "threading")] @@ -216,7 +295,7 @@ enum LocalsPlusData { } // SAFETY: DataStack variant points to thread-local DataStack memory. -// Frame execution is single-threaded (enforced by owner field). +// FrameObject execution is single-threaded (enforced by owner field). #[cfg(feature = "threading")] unsafe impl Send for LocalsPlusData {} #[cfg(feature = "threading")] @@ -247,7 +326,11 @@ impl LocalsPlus { /// When the frame finishes, the caller must migrate data to the heap with /// `materialize_localsplus()` (or drop it in place with /// `release_localsplus()`), then `datastack_pop()` to free the memory. - fn new_on_datastack(nlocalsplus: usize, stacksize: usize, vm: &VirtualMachine) -> Self { + pub(crate) fn new_on_datastack( + nlocalsplus: usize, + stacksize: usize, + vm: &VirtualMachine, + ) -> Self { let capacity = nlocalsplus .checked_add(stacksize) .expect("LocalsPlus capacity overflow"); @@ -288,7 +371,7 @@ impl LocalsPlus { /// /// Only valid when the values can never be observed again (the enclosing /// frame is uniquely referenced): the locals are gone afterwards. - fn release_datastack(&mut self) -> Option<*mut u8> { + pub(crate) fn release_datastack(&mut self) -> Option<*mut u8> { let LocalsPlusData::DataStack { ptr, .. } = &self.data else { return None; }; @@ -304,6 +387,50 @@ impl LocalsPlus { Some(base) } + /// Create a new heap-backed LocalsPlus that is a clone of the + /// fastlocals portion of this one. Stack slots are NOT copied + /// (stack_top = 0, stacksize = 0). + /// + /// # Safety + /// The caller must ensure that `self.fastlocals()` is a valid slice + /// (backing storage is alive, not concurrently mutated). + pub(crate) unsafe fn snapshot_to_heap(&self) -> Self { + let n = self.nlocalsplus as usize; + let src = self.fastlocals(); + let mut data = vec![0usize; n]; + // Clone each Option into the heap buffer. + for (i, slot) in src.iter().enumerate() { + if let Some(obj) = slot { + let cloned: Option = Some(obj.clone()); + // SAFETY: Option has the same layout as usize. + data[i] = unsafe { core::mem::transmute_copy(&cloned) }; + core::mem::forget(cloned); + } + } + Self { + data: LocalsPlusData::Heap(data.into_boxed_slice()), + nlocalsplus: self.nlocalsplus, + stack_top: 0, + } + } + + /// Update fastlocals in `self` from `src`. For each slot, drops the old + /// value and clones the new one. `self` must be heap-backed. + /// + /// # Safety + /// Both `self` and `src` must have valid backing storage, and the caller + /// must ensure no concurrent mutable access. + pub(crate) unsafe fn sync_fastlocals_from(&mut self, src: &Self) { + let n = core::cmp::min(self.nlocalsplus as usize, src.nlocalsplus as usize); + let dst = self.fastlocals_mut(); + let source = src.fastlocals(); + for i in 0..n { + let old = dst[i].take(); + dst[i].clone_from(&source[i]); + drop(old); + } + } + /// Drop all contained values without freeing the backing storage. fn drop_values(&mut self) { self.stack_clear(); @@ -360,7 +487,7 @@ impl LocalsPlus { /// Immutable access to fastlocals as `Option` slice. #[inline(always)] - fn fastlocals(&self) -> &[Option] { + pub(crate) fn fastlocals(&self) -> &[Option] { let data = self.data_as_slice(); let ptr = data.as_ptr() as *const Option; unsafe { core::slice::from_raw_parts(ptr, self.nlocalsplus as usize) } @@ -368,7 +495,7 @@ impl LocalsPlus { /// Mutable access to fastlocals as `Option` slice. #[inline(always)] - fn fastlocals_mut(&mut self) -> &mut [Option] { + pub(crate) fn fastlocals_mut(&mut self) -> &mut [Option] { let nlocalsplus = self.nlocalsplus as usize; let data = self.data_as_mut_slice(); let ptr = data.as_mut_ptr() as *mut Option; @@ -593,7 +720,7 @@ pub struct FrameLocals { impl FrameLocals { /// Create with an already-initialized locals mapping (non-NEWLOCALS frames). - fn with_locals(locals: ArgMapping) -> Self { + pub(crate) fn with_locals(locals: ArgMapping) -> Self { let cell = OnceCell::new(); let _ = cell.set(locals); Self { inner: cell } @@ -601,7 +728,7 @@ impl FrameLocals { /// Create an empty lazy locals (for NEWLOCALS frames). /// The dict will be created on first access. - fn lazy() -> Self { + pub(crate) fn lazy() -> Self { Self { inner: OnceCell::new(), } @@ -668,38 +795,43 @@ unsafe impl Traverse for FrameLocals { /// Lightweight execution frame. Not a PyObject. /// Analogous to CPython's `_PyInterpreterFrame`. /// -/// Currently always embedded inside a `Frame` PyObject via `FrameUnsafeCell`. -/// In future PRs this will be usable independently for normal function calls -/// (allocated on the Rust stack + DataStack), eliminating PyObject overhead. +/// The four "identity" fields (`code`, `globals`, `builtins`, `func_obj`) +/// are borrowed raw pointers — refcounts are maintained by the owner +/// (FrameObject's owned fields, or the PyFunction on the caller's stack +/// in the DataStack path). +#[repr(C)] pub struct InterpreterFrame { - pub code: PyRef, - pub func_obj: Option, + // Borrowed pointers — owned by FrameObject or by PyFunction on caller's stack. + pub(crate) code: *const Py, + pub(crate) func_obj: *const PyObject, // nullable + pub(crate) globals: *const Py, + pub(crate) builtins: *const PyObject, /// Unified storage for local variables and evaluation stack. pub(crate) localsplus: LocalsPlus, pub locals: FrameLocals, - pub globals: PyDictRef, - pub builtins: PyObjectRef, /// index of last instruction ran pub lasti: PyAtomic, - /// tracer function for this frame (usually is None) - pub trace: PyMutex, + /// Per-frame tracer function. `None` means no per-frame trace is set + /// (equivalent to `f_trace = None` in Python). This avoids a refcounted + /// None clone on every frame init. + pub trace: PyMutex>, /// Previous line number for LINE event suppression. - pub(crate) prev_line: u32, + pub(crate) prev_line: core::cell::Cell, // member pub trace_lines: PyMutex, pub trace_opcodes: PyMutex, pub temporary_refs: PyMutex>, /// Back-reference to owning generator/coroutine/async generator. - /// Borrowed reference (not ref-counted) to avoid Generator↔Frame cycle. + /// Borrowed reference (not ref-counted) to avoid Generator↔FrameObject cycle. /// Cleared by the generator's Drop impl. pub generator: PyAtomicBorrow, - /// Previous frame in the call chain for signal-safe traceback walking. - /// Mirrors `_PyInterpreterFrame.previous`. - pub(crate) previous: AtomicPtr, + /// Linked-list pointer to the previous frame in the call chain. + /// Stores a `*const FrameObject` as `usize`. + pub(crate) previous: PyAtomic, /// Who owns this frame. Mirrors `_PyInterpreterFrame.owner`. /// Used by `frame.clear()` to reject clearing an executing frame, /// even when called from a different thread. @@ -717,7 +849,7 @@ pub struct InterpreterFrame { /// Strong reference to the caller frame, captured when this frame escapes /// its execution so `f_back` still resolves after the caller returns and /// leaves the live frame chain. - pub(crate) retained_back: PyMutex>, + pub(crate) retained_back: PyMutex>, /// Number of stack entries to pop after set_f_lineno returns to the /// execution loop. set_f_lineno cannot pop directly because the /// execution loop holds the state mutex. @@ -726,19 +858,344 @@ pub struct InterpreterFrame { /// Used together with `pending_stack_pops` to identify Except entries /// that need special exception-state handling. pub(crate) pending_unwind_from_stack: PyAtomic, + /// Pointer to the owning `Py`, or null for stack-allocated + /// frames that have not been materialized yet. + /// Stored as `usize` for `PyAtomic` compatibility. + pub(crate) materialized: PyAtomic, +} + +// Raw pointers make InterpreterFrame !Send+!Sync by default. +// SAFETY: The pointers reference heap-resident PyObjects whose lifetimes +// are managed by the owning FrameObject (or PyFunction). Frame execution +// is single-threaded (enforced by the owner field). +#[cfg(feature = "threading")] +unsafe impl Send for InterpreterFrame {} +#[cfg(feature = "threading")] +unsafe impl Sync for InterpreterFrame {} + +impl InterpreterFrame { + /// Construct a new InterpreterFrame with raw pointers set from the given references. + /// + /// The caller must ensure that the pointed-to objects outlive this frame. + /// For FrameObject-owned frames, `init_iframe_ptrs` patches the pointers + /// after heap allocation; the pointers passed here are then overwritten. + /// For stack-allocated frames (future), the pointers remain valid for the + /// frame's lifetime on the native stack. + #[allow(clippy::too_many_arguments)] + pub(crate) fn new( + code: &Py, + globals: &Py, + builtins: &PyObject, + func_obj: Option<&PyObject>, + localsplus: LocalsPlus, + locals: FrameLocals, + closure: &[PyCellRef], + owner: FrameOwner, + ) -> Self { + let mut localsplus = localsplus; + let nlocalsplus = code.localspluskinds.len(); + + // Pre-copy closure cells into free var slots so that locals() works + // even before COPY_FREE_VARS runs (e.g. coroutine before first send). + // COPY_FREE_VARS will overwrite these on first execution. + { + let nfrees = code.freevars.len(); + if nfrees > 0 { + let freevar_start = nlocalsplus - nfrees; + let fastlocals = localsplus.fastlocals_mut(); + for (i, cell) in closure.iter().enumerate() { + fastlocals[freevar_start + i] = Some(cell.clone().into()); + } + } + } + + // For generators/coroutines, initialize prev_line to the def line + // so that preamble instructions (RETURN_GENERATOR, POP_TOP) don't + // fire spurious LINE events. + let prev_line = if code.flags.intersects( + bytecode::CodeFlags::GENERATOR + | bytecode::CodeFlags::COROUTINE + | bytecode::CodeFlags::ASYNC_GENERATOR, + ) { + code.first_line_number.map_or(0, |line| line.get() as u32) + } else { + 0 + }; + + Self { + code: code as *const Py, + func_obj: match func_obj { + Some(obj) => obj as *const PyObject, + None => core::ptr::null(), + }, + globals: globals as *const Py, + builtins: builtins as *const PyObject, + localsplus, + locals, + lasti: Radium::new(0), + prev_line: core::cell::Cell::new(prev_line), + trace: PyMutex::new(None), + trace_lines: PyMutex::new(true), + trace_opcodes: PyMutex::new(false), + temporary_refs: PyMutex::new(vec![]), + generator: PyAtomicBorrow::new(), + previous: Radium::new(0), + owner: atomic::AtomicI8::new(owner as i8), + f_locals_hidden_overlay: PyMutex::new(None), + f_extra_locals: PyMutex::new(None), + escaped: atomic::AtomicBool::new(false), + retained_back: PyMutex::new(None), + pending_stack_pops: Default::default(), + pending_unwind_from_stack: Default::default(), + materialized: Radium::new(0), + } + } + + /// Get the last instruction index. + #[inline(always)] + pub fn get_lasti(&self) -> u32 { + self.lasti.load(Relaxed) + } + + /// Get the previous InterpreterFrame in the chain, or null. + #[inline(always)] + pub fn previous(&self) -> *const Self { + self.previous.load(Relaxed) as *const Self + } + + /// Get the owning FrameObject, if this frame has been materialized. + #[inline(always)] + pub(crate) fn frame_obj(&self) -> Option<&Py> { + let ptr = self.materialized.load(Relaxed); + if ptr == 0 { + None + } else { + Some(unsafe { &*(ptr as *const Py) }) + } + } + + /// Materialize a FrameObject for this InterpreterFrame on demand. + /// If already materialized, returns the existing one. + /// The created FrameObject shares the raw pointers with this frame. + #[cold] + #[inline(never)] + pub(crate) fn materialize(&self, vm: &VirtualMachine) -> &Py { + if let Some(fo) = self.frame_obj() { + return fo; + } + self.materialize_slow(vm) + } + + /// Create a lightweight FrameObject with empty localsplus, suitable for + /// f_back chain building (retained_back). Unlike `materialize`, this does + /// NOT store into `temporary_refs` or set the `materialized` pointer, so + /// the returned FrameObject is only kept alive by the caller's `PyRef`. + /// This prevents non-GC-tracked `temporary_refs` on a stack-allocated + /// iframe from defeating cycle collection. + #[cold] + #[inline(never)] + pub(crate) fn materialize_chain(&self, vm: &VirtualMachine) -> FrameObjectRef { + if let Some(fo) = self.frame_obj() { + return fo.to_owned(); + } + self.materialize_slow_chain(vm) + } + + #[cold] + fn materialize_slow(&self, vm: &VirtualMachine) -> &Py { + // Create a full FrameObject with its own InterpreterFrame copy. + // The FrameObject owns references to the same objects (code, globals, etc). + let code: PyRef = self.code().to_owned(); + let globals: PyDictRef = self.globals().to_owned(); + let builtins: PyObjectRef = self.builtins().to_owned(); + let func_obj: Option = self.func_obj().map(|o| o.to_owned()); + + // Copy localsplus from the stack frame so materialized frames have + // usable fastlocals (for locals(), f_locals, tracebacks, etc). + let localsplus = unsafe { self.localsplus.snapshot_to_heap() }; + + // Copy the locals mapping if it exists. + let locals = match self.locals.get() { + Some(mapping) => FrameLocals::with_locals(mapping.clone()), + None => FrameLocals::lazy(), + }; + + // Build a fresh InterpreterFrame inside the FrameObject. + // Its raw pointers will be patched by init_iframe_ptrs. + let inner_iframe = Self { + code: core::ptr::null(), + func_obj: core::ptr::null(), + globals: core::ptr::null(), + builtins: core::ptr::null(), + localsplus, + locals, + lasti: Radium::new(self.lasti.load(Relaxed)), + prev_line: core::cell::Cell::new(self.prev_line.get()), + trace: PyMutex::new(None), + trace_lines: PyMutex::new(true), + trace_opcodes: PyMutex::new(false), + temporary_refs: PyMutex::new(vec![]), + generator: PyAtomicBorrow::new(), + // Do NOT copy previous — it may point to stack-allocated frames + // that become dangling after their call returns. The f_back chain + // is resolved through the TLS CURRENT_FRAME chain instead. + previous: Radium::new(0), + // Materialized frame is a detached snapshot — always FrameObject-owned. + // If we copied Thread from the source iframe, frame.clear() would + // reject the frame with "cannot clear an executing frame". + owner: atomic::AtomicI8::new(FrameOwner::FrameObject as i8), + f_locals_hidden_overlay: PyMutex::new(None), + f_extra_locals: PyMutex::new(None), + escaped: atomic::AtomicBool::new(true), + retained_back: PyMutex::new(None), + pending_stack_pops: Default::default(), + pending_unwind_from_stack: Default::default(), + materialized: Radium::new(0), + }; + + let frame_obj = FrameObject { + owned_code: Some(code), + owned_globals: Some(globals), + owned_builtins: Some(builtins), + owned_func_obj: func_obj, + iframe: FrameUnsafeCell::new(Some(inner_iframe)), + }; + let frame_ref = frame_obj.into_ref(&vm.ctx); + FrameObject::init_iframe_ptrs(&frame_ref); + // Set the inner iframe's materialized pointer to self + unsafe { + frame_ref + .iframe_mut() + .materialized + .store(&*frame_ref as *const Py as usize, Relaxed); + } + + // Store the materialized pointer on this stack frame. + let fo_ptr = &*frame_ref as *const Py as usize; + self.materialized.store(fo_ptr, Relaxed); + + // Keep the FrameObject alive by storing it in temporary_refs. + // GC tracking is deferred to with_iframe cleanup, where the frame + // is no longer executing and temporary_refs is cleared — at that + // point the FrameObject is self-sustaining and GC can safely + // traverse and collect it. + self.temporary_refs.lock().push(frame_ref.clone().into()); + + // SAFETY: the pointer we stored above remains valid because + // temporary_refs holds a strong reference. + unsafe { &*(fo_ptr as *const Py) } + } + + /// Like `materialize_slow` but with empty localsplus to avoid extra + /// refcounts on local variables. Only suitable for f_back chain building. + /// Returns an owned `PyRef` without storing into `temporary_refs` or + /// setting the `materialized` pointer, so GC can still detect cycles. + #[cold] + fn materialize_slow_chain(&self, vm: &VirtualMachine) -> FrameObjectRef { + let code: PyRef = self.code().to_owned(); + let globals: PyDictRef = self.globals().to_owned(); + let builtins: PyObjectRef = self.builtins().to_owned(); + let func_obj: Option = self.func_obj().map(|o| o.to_owned()); + + // Empty localsplus — reads go through find_live_source_iframe. + let nlocalsplus = code.localspluskinds.len() as u32; + let localsplus = LocalsPlus { + data: LocalsPlusData::Heap(vec![0usize; nlocalsplus as usize].into_boxed_slice()), + nlocalsplus, + stack_top: 0, + }; + + let locals = match self.locals.get() { + Some(mapping) => FrameLocals::with_locals(mapping.clone()), + None => FrameLocals::lazy(), + }; + + let inner_iframe = Self { + code: core::ptr::null(), + func_obj: core::ptr::null(), + globals: core::ptr::null(), + builtins: core::ptr::null(), + localsplus, + locals, + lasti: Radium::new(self.lasti.load(Relaxed)), + prev_line: core::cell::Cell::new(self.prev_line.get()), + trace: PyMutex::new(None), + trace_lines: PyMutex::new(true), + trace_opcodes: PyMutex::new(false), + temporary_refs: PyMutex::new(vec![]), + generator: PyAtomicBorrow::new(), + previous: Radium::new(0), + owner: atomic::AtomicI8::new(FrameOwner::FrameObject as i8), + f_locals_hidden_overlay: PyMutex::new(None), + f_extra_locals: PyMutex::new(None), + escaped: atomic::AtomicBool::new(true), + retained_back: PyMutex::new(None), + pending_stack_pops: Default::default(), + pending_unwind_from_stack: Default::default(), + materialized: Radium::new(0), + }; + + let frame_obj = FrameObject { + owned_code: Some(code), + owned_globals: Some(globals), + owned_builtins: Some(builtins), + owned_func_obj: func_obj, + iframe: FrameUnsafeCell::new(Some(inner_iframe)), + }; + let frame_ref = frame_obj.into_ref(&vm.ctx); + FrameObject::init_iframe_ptrs(&frame_ref); + + frame_ref + } + + /// Borrowed code object. + #[inline(always)] + pub fn code(&self) -> &Py { + unsafe { &*self.code } + } + + /// Borrowed globals dict. + #[inline(always)] + pub fn globals(&self) -> &Py { + unsafe { &*self.globals } + } + + /// Borrowed builtins object. + #[inline(always)] + pub fn builtins(&self) -> &PyObject { + unsafe { &*self.builtins } + } + + /// Borrowed function object, or None if not set. + #[inline(always)] + pub fn func_obj(&self) -> Option<&PyObject> { + if self.func_obj.is_null() { + None + } else { + Some(unsafe { &*self.func_obj }) + } + } } /// Python-visible frame object. Currently always wraps an `InterpreterFrame`. /// Analogous to CPython's `PyFrameObject`. #[pyclass(module = false, name = "frame", traverse = "manual")] -pub struct Frame { +pub struct FrameObject { + // Owned references — keep the pointed-to objects alive for InterpreterFrame's + // raw pointers. Wrapped in Option so Traverse::clear can release them, + // allowing GC cycle collection to reclaim referenced objects. + pub(crate) owned_code: Option>, + pub(crate) owned_globals: Option, + pub(crate) owned_builtins: Option, + pub(crate) owned_func_obj: Option, + /// Always `Some` while the frame is reachable from Python. Emptied only /// by `Traverse::clear` during deallocation, leaving a trivially-droppable /// husk that the freelist can cache. pub(crate) iframe: FrameUnsafeCell>, } -impl Frame { +impl FrameObject { /// Shared access to the embedded interpreter frame. /// /// # Safety @@ -746,7 +1203,7 @@ impl Frame { /// and that the frame has not been cleared (i.e. it is still reachable /// from Python; `Traverse::clear` only runs during deallocation). #[inline(always)] - unsafe fn iframe_ref(&self) -> &InterpreterFrame { + pub(crate) unsafe fn iframe_ref(&self) -> &InterpreterFrame { let opt = unsafe { &*self.iframe.get() }; #[cfg(debug_assertions)] if opt.is_none() { @@ -763,7 +1220,7 @@ impl Frame { /// the frame has not been cleared. #[inline(always)] #[allow(clippy::mut_from_ref)] - unsafe fn iframe_mut(&self) -> &mut InterpreterFrame { + pub(crate) unsafe fn iframe_mut(&self) -> &mut InterpreterFrame { let opt = unsafe { &mut *self.iframe.get() }; #[cfg(debug_assertions)] if opt.is_none() { @@ -772,6 +1229,18 @@ impl Frame { // SAFETY: iframe is always Some while the frame is reachable (see above). unsafe { opt.as_mut().unwrap_unchecked() } } + + /// Shared access to the embedded interpreter frame. Safe to call on any + /// reachable FrameObject: immutable fields and atomic/mutex fields are + /// always safe to access. + #[inline(always)] + pub fn iframe(&self) -> &InterpreterFrame { + // SAFETY: FrameObject is always reachable from Python when this is + // called. Immutable fields and atomic/mutex fields provide their own + // synchronization. Mutable fields (localsplus, prev_line) are only + // mutated during single-threaded execution via with_exec. + unsafe { self.iframe_ref() } + } } /// Out-of-line panic for the debug-only cleared-frame check, keeping the @@ -783,31 +1252,19 @@ fn cleared_frame_access() -> ! { panic!("frame accessed after clear"); } -impl core::ops::Deref for Frame { - type Target = InterpreterFrame; - /// Transparent access to InterpreterFrame fields. - /// - /// # Safety argument - /// Immutable fields (code, globals, builtins, func_obj, locals) are safe - /// to access at any time. Atomic/mutex fields (lasti, trace, owner, etc.) - /// provide their own synchronization. Mutable fields (localsplus, prev_line) - /// are only mutated during single-threaded execution via `with_exec`. - #[inline(always)] - fn deref(&self) -> &InterpreterFrame { - unsafe { self.iframe_ref() } - } -} +// NOTE: Deref removed to decouple FrameObject +// from InterpreterFrame field layout. Access through iframe_ref()/iframe_mut(). thread_local! { /// Free list of dead frame objects for reuse. Entries are cleared husks /// (`iframe == None`) whose child references were already released. - /// PyInner is fixed-size (localsplus storage is out-of-line), + /// PyInner is fixed-size (localsplus storage is out-of-line), /// so a single bucket suffices. - static FRAME_FREELIST: core::cell::Cell> = + static FRAME_FREELIST: core::cell::Cell> = const { core::cell::Cell::new(crate::object::FreeList::new()) }; } -impl PyPayload for Frame { +impl PyPayload for FrameObject { const MAX_FREELIST: usize = 200; const HAS_FREELIST: bool = true; // Ordinary call frames are created untracked and only enter the GC when @@ -851,37 +1308,21 @@ impl PyPayload for Frame { } } -unsafe impl Traverse for Frame { +unsafe impl Traverse for FrameObject { fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { - // SAFETY: this traversal reads the frame's live interpreter state - // (`localsplus`), which the owning thread mutates without - // synchronization while executing bytecode. It is only sound when no - // other thread is executing this frame: in threading builds the - // collector stops the world around the traversal phases, and in - // single-threaded builds there is no other thread. A cleared frame - // (iframe == None) has no children to visit. - // - // Invariant (load-bearing for the untracked-frame optimization): every - // reference *to* a frame is recorded as a graph edge by - // `PyRef::traverse` — no other type's `traverse` recurses into a - // frame's payload. So the collector reads a frame's `localsplus` only - // when the frame is itself a tracked candidate. Tracked datastack - // frames are tracked only at `release_datastack_frame`, after they stop - // executing and their localsplus is materialized onto the heap; tracked - // generator frames are heap-backed by construction and their execution - // is stopped-the-world. Hence a running, data-stack-resident frame is - // never traversed by a concurrent collector. If a future change makes - // some type's `traverse` recurse into a frame payload, this invariant - // (and the debug asserts at the track sites) breaks. + // Visit the owned reference anchors on FrameObject. + // After clear(), these are None. + self.owned_code.traverse(tracer_fn); + self.owned_func_obj.traverse(tracer_fn); + self.owned_globals.traverse(tracer_fn); + self.owned_builtins.traverse(tracer_fn); + + // Visit interior references in the InterpreterFrame. let Some(iframe) = (unsafe { &*self.iframe.get() }) else { return; }; - iframe.code.traverse(tracer_fn); - iframe.func_obj.traverse(tracer_fn); iframe.localsplus.traverse(tracer_fn); iframe.locals.traverse(tracer_fn); - iframe.globals.traverse(tracer_fn); - iframe.builtins.traverse(tracer_fn); iframe.trace.traverse(tracer_fn); iframe.temporary_refs.traverse(tracer_fn); iframe.f_locals_hidden_overlay.traverse(tracer_fn); @@ -890,14 +1331,14 @@ unsafe impl Traverse for Frame { } fn clear(&mut self, _out: &mut Vec) { - // Drop the interpreter frame in place instead of extracting children - // into `_out`: pushing ~10 refs per frame would grow the buffer, a - // heap allocation on the hot dealloc path. Direct drops release the - // same references under the same recursion protection (trashcan in - // dealloc, deferred-drop context in cycle collection) as the payload - // drop did before the freelist existed. The payload is left as a - // trivially-droppable husk for the freelist. + // Drop the interpreter frame and owned reference anchors so GC + // cycle collection can reclaim the referenced objects. The payload + // is left as a trivially-droppable husk for the freelist. drop(self.iframe.get_mut().take()); + self.owned_code.take(); + self.owned_globals.take(); + self.owned_builtins.take(); + self.owned_func_obj.take(); } } @@ -910,7 +1351,7 @@ pub enum ExecutionResult { /// A valid execution result, or an exception type FrameResult = PyResult>; -impl Frame { +impl FrameObject { pub(crate) fn new( code: PyRef, scope: Scope, @@ -922,73 +1363,76 @@ impl Frame { ) -> Self { let nlocalsplus = code.localspluskinds.len(); let max_stackdepth = code.max_stackdepth as usize; - let mut localsplus = if use_datastack { + let localsplus = if use_datastack { LocalsPlus::new_on_datastack(nlocalsplus, max_stackdepth, vm) } else { LocalsPlus::new(nlocalsplus, max_stackdepth) }; - // Pre-copy closure cells into free var slots so that locals() works - // even before COPY_FREE_VARS runs (e.g. coroutine before first send). - // COPY_FREE_VARS will overwrite these on first execution. - { - let nfrees = code.freevars.len(); - if nfrees > 0 { - let freevar_start = nlocalsplus - nfrees; - let fastlocals = localsplus.fastlocals_mut(); - for (i, cell) in closure.iter().enumerate() { - fastlocals[freevar_start + i] = Some(cell.clone().into()); - } - } - } - - // For generators/coroutines, initialize prev_line to the def line - // so that preamble instructions (RETURN_GENERATOR, POP_TOP) don't - // fire spurious LINE events. - let prev_line = if code.flags.intersects( - bytecode::CodeFlags::GENERATOR - | bytecode::CodeFlags::COROUTINE - | bytecode::CodeFlags::ASYNC_GENERATOR, - ) { - code.first_line_number.map_or(0, |line| line.get() as u32) - } else { - 0 + let locals = match scope.locals { + Some(locals) => FrameLocals::with_locals(locals), + None if code.flags.contains(bytecode::CodeFlags::NEWLOCALS) => FrameLocals::lazy(), + None => FrameLocals::with_locals(ArgMapping::from_dict_exact(scope.globals.clone())), }; - let iframe = InterpreterFrame { + // Build the InterpreterFrame using the constructor. + // Pointers are initially set from owned fields' references but will be + // dangling after the FrameObject moves into heap allocation — they get + // patched by `init_iframe_ptrs` after `into_ref`. + let iframe = InterpreterFrame::new( + &code, + &scope.globals, + &builtins, + func_obj.as_deref(), localsplus, - locals: match scope.locals { - Some(locals) => FrameLocals::with_locals(locals), - None if code.flags.contains(bytecode::CodeFlags::NEWLOCALS) => FrameLocals::lazy(), - None => { - FrameLocals::with_locals(ArgMapping::from_dict_exact(scope.globals.clone())) - } - }, - globals: scope.globals, - builtins, - code, - func_obj, - lasti: Radium::new(0), - prev_line, - trace: PyMutex::new(vm.ctx.none()), - trace_lines: PyMutex::new(true), - trace_opcodes: PyMutex::new(false), - temporary_refs: PyMutex::new(vec![]), - generator: PyAtomicBorrow::new(), - previous: AtomicPtr::new(core::ptr::null_mut()), - owner: atomic::AtomicI8::new(FrameOwner::FrameObject as i8), - f_locals_hidden_overlay: PyMutex::new(None), - f_extra_locals: PyMutex::new(None), - escaped: atomic::AtomicBool::new(false), - retained_back: PyMutex::new(None), - pending_stack_pops: Default::default(), - pending_unwind_from_stack: Default::default(), - }; + locals, + closure, + FrameOwner::FrameObject, + ); Self { + owned_code: Some(code), + owned_globals: Some(scope.globals), + owned_builtins: Some(builtins), + owned_func_obj: func_obj, iframe: FrameUnsafeCell::new(Some(iframe)), } } + /// Patch the InterpreterFrame's raw pointers to point at this + /// FrameObject's owned fields. Must be called once after the + /// FrameObject is allocated on the heap (i.e. after `into_ref`). + fn init_iframe_ptrs(self_: &Py) { + let iframe = unsafe { self_.iframe_mut() }; + iframe.code = &**self_.owned_code.as_ref().unwrap() as *const Py; + iframe.globals = &**self_.owned_globals.as_ref().unwrap() as *const Py; + iframe.builtins = &**self_.owned_builtins.as_ref().unwrap() as *const PyObject; + iframe.func_obj = match &self_.owned_func_obj { + Some(obj) => &**obj as *const PyObject, + None => core::ptr::null(), + }; + // Link the InterpreterFrame back to its owning FrameObject. + iframe + .materialized + .store(self_ as *const Py as usize, Relaxed); + } + + /// Create a new FrameObject, allocate it on the heap, and patch + /// the InterpreterFrame's raw pointers. Returns an owned reference. + pub(crate) fn new_ref( + code: PyRef, + scope: Scope, + builtins: PyObjectRef, + closure: &[PyCellRef], + func_obj: Option, + use_datastack: bool, + vm: &VirtualMachine, + ) -> FrameObjectRef { + let frame = Self::new(code, scope, builtins, closure, func_obj, use_datastack, vm) + .into_ref(&vm.ctx); + Self::init_iframe_ptrs(&frame); + frame + } + /// Access fastlocals immutably. /// /// # Safety @@ -1056,31 +1500,21 @@ impl Frame { /// Releases references held by the frame, matching _PyFrame_ClearLocals. pub(crate) fn clear_locals_and_stack(&self) { self.clear_stack_and_cells(); - // SAFETY: Frame is not executing (generator closed). + // SAFETY: FrameObject is not executing (generator closed). let fastlocals = unsafe { self.iframe_mut().localsplus.fastlocals_mut() }; for slot in fastlocals.iter_mut() { *slot = None; } - self.f_locals_hidden_overlay.lock().take(); - self.f_extra_locals.lock().take(); - } - - /// Get cell contents by localsplus index. - pub(crate) fn get_cell_contents(&self, localsplus_idx: usize) -> Option { - // SAFETY: Frame not executing; no concurrent mutation. - let fastlocals = unsafe { self.iframe_ref().localsplus.fastlocals() }; - fastlocals - .get(localsplus_idx) - .and_then(|slot| slot.as_ref()) - .and_then(|obj| obj.downcast_ref::()) - .and_then(|cell| cell.get()) + self.iframe().f_locals_hidden_overlay.lock().take(); + self.iframe().f_extra_locals.lock().take(); } /// Store a borrowed back-reference to the owning generator/coroutine. /// The caller must ensure the generator outlives the frame. pub fn set_generator(&self, generator: &PyObject) { - self.generator.store(generator); - self.owner + self.iframe().generator.store(generator); + self.iframe() + .owner .store(FrameOwner::Generator as i8, atomic::Ordering::Release); } @@ -1101,51 +1535,68 @@ impl Frame { } pub fn current_location(&self) -> SourceLocation { - self.code.locations[self.lasti() as usize - 1].0 + let lasti = self.lasti() as usize; + if lasti == 0 { + return SourceLocation { + line: self + .iframe() + .code() + .first_line_number + .unwrap_or(OneIndexed::MIN), + character_offset: OneIndexed::from_zero_indexed(0), + }; + } + self.iframe().code().locations[lasti - 1].0 } - /// Get the previous frame pointer for signal-safe traceback walking. + /// Get the previous InterpreterFrame in the chain. + /// Returns null if the frame has been cleared (GC deallocation). + pub fn previous_iframe(&self) -> *const InterpreterFrame { + // Use raw access instead of iframe() to avoid panicking on cleared frames. + let iframe_opt = unsafe { &*self.iframe.get() }; + match iframe_opt.as_ref() { + Some(iframe) => { + iframe.previous.load(atomic::Ordering::Relaxed) as *const InterpreterFrame + } + None => core::ptr::null(), + } + } + + /// Get the previous FrameObject in the chain, if any. + /// Walks through the chain to find the next materialized frame. pub fn previous_frame(&self) -> *const Self { - self.previous.load(atomic::Ordering::Relaxed) + let mut cur = self.previous_iframe(); + while !cur.is_null() { + let iframe = unsafe { &*cur }; + if let Some(fo) = iframe.frame_obj() { + return &**fo as *const Self; + } + cur = iframe.previous.load(atomic::Ordering::Relaxed) as *const InterpreterFrame; + } + core::ptr::null() } /// Record that a durable Python-level reference to this frame escaped. pub(crate) fn mark_escaped(&self) { - self.escaped.store(true, atomic::Ordering::Release); + self.iframe().escaped.store(true, atomic::Ordering::Release); } /// Whether a durable reference to this frame has escaped. pub(crate) fn has_escaped(&self) -> bool { - self.escaped.load(atomic::Ordering::Acquire) + self.iframe().escaped.load(atomic::Ordering::Acquire) } pub fn lasti(&self) -> u32 { - self.lasti.load(Relaxed) + self.iframe().lasti.load(Relaxed) } pub fn set_lasti(&self, val: u32) { - self.lasti.store(val, Relaxed); - } - - pub(crate) fn pending_stack_pops(&self) -> u32 { - self.pending_stack_pops.load(Relaxed) - } - - pub(crate) fn set_pending_stack_pops(&self, val: u32) { - self.pending_stack_pops.store(val, Relaxed); - } - - pub(crate) fn pending_unwind_from_stack(&self) -> i64 { - self.pending_unwind_from_stack.load(Relaxed) - } - - pub(crate) fn set_pending_unwind_from_stack(&self, val: i64) { - self.pending_unwind_from_stack.store(val, Relaxed); + self.iframe().lasti.store(val, Relaxed); } fn has_active_hidden_locals(&self) -> bool { use rustpython_compiler_core::bytecode::{CO_FAST_CELL, CO_FAST_FREE, CO_FAST_HIDDEN}; - let code = &**self.code; + let code = self.iframe().code(); let fastlocals = unsafe { self.iframe_ref().localsplus.fastlocals() }; let is_optimized = code.flags.contains(bytecode::CodeFlags::OPTIMIZED); !is_optimized @@ -1177,8 +1628,16 @@ impl Frame { }; // SAFETY: Either the frame is not executing (caller checked owner), // or we're in a trace callback on the same thread that's executing. - let code = &**self.code; - let fastlocals = unsafe { self.iframe_ref().localsplus.fastlocals() }; + let code = self.iframe().code(); + // If this FrameObject has a live source iframe on the TLS chain, read + // its localsplus for up-to-date values (the materialized copy is a + // stale snapshot from materialize time). + let live = self.find_live_source_iframe(); + let fastlocals = if !live.is_null() { + unsafe { (*live).localsplus.fastlocals() } + } else { + unsafe { self.iframe_ref().localsplus.fastlocals() } + }; // Iterate through all localsplus slots using localspluskinds let nlocalsplus = code.localspluskinds.len(); @@ -1281,16 +1740,26 @@ impl Frame { /// builtin, trace callbacks) is fine: the frame sits on the current /// thread's frame chain and is at a bytecode boundary. pub(crate) fn check_locals_access(&self, vm: &VirtualMachine) -> PyResult<()> { - let owner = FrameOwner::from_i8(self.owner.load(atomic::Ordering::Acquire)); + let owner = FrameOwner::from_i8(self.iframe().owner.load(atomic::Ordering::Acquire)); if owner != FrameOwner::Thread { return Ok(()); } + let self_iframe = self.iframe() as *const InterpreterFrame; + // Get the Py address from &FrameObject (payload). + // materialized stores a *const Py. + let self_py_ptr = unsafe { Py::::from_payload_ptr(self) } as usize; let mut cur = crate::vm::thread::get_current_frame(); while !cur.is_null() { - if core::ptr::eq(cur, self) { + if core::ptr::eq(cur, self_iframe) { return Ok(()); } - cur = unsafe { (*cur).previous_frame() }; + // Also match if this FrameObject is the materialized version + // of a stack-allocated frame in the chain. + let materialized = unsafe { (*cur).materialized.load(Relaxed) }; + if materialized == self_py_ptr { + return Ok(()); + } + cur = unsafe { (*cur).previous.load(Relaxed) as *const InterpreterFrame }; } Err(vm.new_runtime_error( "cannot access frame locals while the frame is executing in another thread", @@ -1300,12 +1769,12 @@ impl Frame { pub fn f_locals_mapping(&self, vm: &VirtualMachine) -> PyResult { self.check_locals_access(vm)?; if !self.has_active_hidden_locals() { - self.f_locals_hidden_overlay.lock().take(); + self.iframe().f_locals_hidden_overlay.lock().take(); return self.locals(vm); } let overlay_dict = { - let mut overlay = self.f_locals_hidden_overlay.lock(); + let mut overlay = self.iframe().f_locals_hidden_overlay.lock(); match overlay.as_ref() { Some(dict) => dict.clone(), None => { @@ -1329,8 +1798,8 @@ impl Frame { self.sync_visible_locals_to_mapping(overlay.mapping(), vm)?; overlay } else { - self.sync_visible_locals_to_mapping(self.locals.mapping(vm), vm)?; - self.locals.clone_mapping(vm) + self.sync_visible_locals_to_mapping(self.iframe().locals.mapping(vm), vm)?; + self.iframe().locals.clone_mapping(vm) }; self.fold_extra_locals(&mapping, vm)?; Ok(mapping) @@ -1339,7 +1808,7 @@ impl Frame { /// Copy the frame's extra-locals side storage (proxy keys that are not /// fast locals) into `mapping`. No-op when nothing was ever stored. fn fold_extra_locals(&self, mapping: &ArgMapping, vm: &VirtualMachine) -> PyResult<()> { - let extra = self.f_extra_locals.lock().clone(); + let extra = self.iframe().f_extra_locals.lock().clone(); if let Some(extra) = extra { for (key, value) in &extra { mapping.mapping().ass_subscript(&key, Some(value), vm)?; @@ -1354,9 +1823,21 @@ impl Frame { use rustpython_compiler_core::bytecode::{CO_FAST_CELL, CO_FAST_FREE}; // SAFETY: callers first pass through `check_locals_access`, so the // frame is not executing on another thread. - let fastlocals = unsafe { self.iframe_ref().localsplus.fastlocals() }; + // Use live source iframe if available for up-to-date values. + let live = self.find_live_source_iframe(); + let fastlocals = if !live.is_null() { + unsafe { (*live).localsplus.fastlocals() } + } else { + unsafe { self.iframe_ref().localsplus.fastlocals() } + }; let obj = fastlocals.get(i)?.as_ref()?; - let kind = self.code.localspluskinds.get(i).copied().unwrap_or(0); + let kind = self + .iframe() + .code() + .localspluskinds + .get(i) + .copied() + .unwrap_or(0); if kind & (CO_FAST_CELL | CO_FAST_FREE) != 0 { if let Some(cell) = obj.downcast_ref::() { cell.get() @@ -1372,9 +1853,22 @@ impl Frame { /// the slot holds one so closures keep sharing the same cell. fn framelocalsproxy_setval(&self, i: usize, value: PyObjectRef) { use rustpython_compiler_core::bytecode::{CO_FAST_CELL, CO_FAST_FREE}; - let kind = self.code.localspluskinds.get(i).copied().unwrap_or(0); + let kind = self + .iframe() + .code() + .localspluskinds + .get(i) + .copied() + .unwrap_or(0); // SAFETY: callers first pass through `check_locals_access`. - let fastlocals = unsafe { self.iframe_mut().localsplus.fastlocals_mut() }; + // Use live source iframe if available so writes reach the + // executing frame's actual local variables. + let live = self.find_live_source_iframe(); + let fastlocals = if !live.is_null() { + unsafe { &mut *live.cast_mut() }.localsplus.fastlocals_mut() + } else { + unsafe { self.iframe_mut().localsplus.fastlocals_mut() } + }; if kind & (CO_FAST_CELL | CO_FAST_FREE) != 0 && let Some(obj) = fastlocals[i].as_ref() && let Some(cell) = obj.downcast_ref::() @@ -1398,8 +1892,8 @@ impl Frame { use rustpython_compiler_core::bytecode::CO_FAST_HIDDEN; // The proxy hashes the key first; an unhashable key raises TypeError. key.hash(vm)?; - for (i, &kind) in self.code.localspluskinds.iter().enumerate() { - let name = localsplus_name(&self.code, i); + for (i, &kind) in self.iframe().code().localspluskinds.iter().enumerate() { + let name = localsplus_name(self.iframe().code(), i); if !name .as_object() .rich_compare_bool(key, PyComparisonOp::Eq, vm)? @@ -1440,7 +1934,7 @@ impl Frame { { return Ok(value); } - let extra = self.f_extra_locals.lock().clone(); + let extra = self.iframe().f_extra_locals.lock().clone(); if let Some(extra) = extra && let Some(value) = extra.get_item_opt(&*key, vm)? { @@ -1459,7 +1953,7 @@ impl Frame { if self.framelocalsproxy_getkeyindex(&key, true, vm)?.is_some() { return Ok(true); } - let extra = self.f_extra_locals.lock().clone(); + let extra = self.iframe().f_extra_locals.lock().clone(); if let Some(extra) = extra { return Ok(extra.get_item_opt(&*key, vm)?.is_some()); } @@ -1497,7 +1991,7 @@ impl Frame { { return Err(vm.new_value_error("cannot remove local variables from FrameLocalsProxy")); } - let extra = self.f_extra_locals.lock().clone(); + let extra = self.iframe().f_extra_locals.lock().clone(); if let Some(extra) = extra && extra.get_item_opt(&*key, vm)?.is_some() { @@ -1520,7 +2014,7 @@ impl Frame { { return Err(vm.new_value_error("cannot remove local variables from FrameLocalsProxy")); } - let extra = self.f_extra_locals.lock().clone(); + let extra = self.iframe().f_extra_locals.lock().clone(); if let Some(extra) = extra && let Some(value) = extra.pop_item(&*key, vm)? { @@ -1547,42 +2041,55 @@ impl Frame { } fn extra_locals_get_or_create(&self, vm: &VirtualMachine) -> PyDictRef { - let mut extra = self.f_extra_locals.lock(); + let mut extra = self.iframe().f_extra_locals.lock(); extra.get_or_insert_with(|| vm.ctx.new_dict()).clone() } } -impl Py { +impl Py { #[inline(always)] fn with_exec(&self, vm: &VirtualMachine, f: impl FnOnce(ExecutingFrame<'_>) -> R) -> R { - // SAFETY: Frame execution is single-threaded. Only one thread at a time + // SAFETY: FrameObject execution is single-threaded. Only one thread at a time // executes a given frame (enforced by the owner field and generator // running flag). Same safety argument as FastLocals (UnsafeCell). let iframe = unsafe { self.iframe_mut() }; + // Dereference the raw pointers before taking mutable borrows + // to localsplus/prev_line. The raw pointers point to FrameObject's + // owned fields, not into InterpreterFrame, so there is no aliasing. + let code: &Py = unsafe { &*iframe.code }; + let globals: &Py = unsafe { &*iframe.globals }; + let builtins: &PyObject = unsafe { &*iframe.builtins }; + let func_obj: Option<&PyObject> = if iframe.func_obj.is_null() { + None + } else { + Some(unsafe { &*iframe.func_obj }) + }; + let builtins_dict = if globals.class().is(vm.ctx.types.dict_type) { + builtins + .downcast_ref_if_exact::(vm) + // SAFETY: downcast_ref_if_exact already verified exact type + .map(|d| unsafe { PyExact::ref_unchecked(d) }) + } else { + None + }; + let iframe_ptr = iframe as *const InterpreterFrame; let exec = ExecutingFrame { - code: &iframe.code, + code, localsplus: &mut iframe.localsplus, locals: &iframe.locals, - globals: &iframe.globals, - builtins: &iframe.builtins, - builtins_dict: if iframe.globals.class().is(vm.ctx.types.dict_type) { - iframe - .builtins - .downcast_ref_if_exact::(vm) - // SAFETY: downcast_ref_if_exact already verified exact type - .map(|d| unsafe { PyExact::ref_unchecked(d) }) - } else { - None - }, + globals, + builtins, + builtins_dict, lasti: &iframe.lasti, - object: self, - prev_line: &mut iframe.prev_line, + iframe: iframe_ptr, + func_obj, + prev_line: &iframe.prev_line, monitoring_mask: 0, }; f(exec) } - // #[cfg_attr(feature = "flame-it", flame("Frame"))] + // #[cfg_attr(feature = "flame-it", flame("FrameObject"))] pub fn run(&self, vm: &VirtualMachine) -> PyResult { self.with_exec(vm, |mut exec| exec.run(vm)) } @@ -1613,22 +2120,32 @@ impl Py { pub fn yield_from_target(&self) -> Option { // If the frame is currently executing (owned by thread), it has no // yield-from target to report. - let owner = FrameOwner::from_i8(self.owner.load(atomic::Ordering::Acquire)); + let owner = FrameOwner::from_i8(self.iframe().owner.load(atomic::Ordering::Acquire)); if owner == FrameOwner::Thread { return None; } - // SAFETY: Frame is not executing, so UnsafeCell access is safe. + // SAFETY: FrameObject is not executing, so UnsafeCell access is safe. let iframe = unsafe { self.iframe_mut() }; + let code: &Py = unsafe { &*iframe.code }; + let globals: &Py = unsafe { &*iframe.globals }; + let builtins: &PyObject = unsafe { &*iframe.builtins }; + let func_obj: Option<&PyObject> = if iframe.func_obj.is_null() { + None + } else { + Some(unsafe { &*iframe.func_obj }) + }; + let iframe_ptr = iframe as *const InterpreterFrame; let exec = ExecutingFrame { - code: &iframe.code, + code, localsplus: &mut iframe.localsplus, locals: &iframe.locals, - globals: &iframe.globals, - builtins: &iframe.builtins, + globals, + builtins, builtins_dict: None, lasti: &iframe.lasti, - object: self, - prev_line: &mut iframe.prev_line, + iframe: iframe_ptr, + func_obj, + prev_line: &iframe.prev_line, monitoring_mask: 0, }; exec.yield_from_target().map(PyObject::to_owned) @@ -1641,7 +2158,7 @@ impl Py { filename.find(b"importlib").is_some() && filename.find(b"_bootstrap").is_some() } - pub fn next_external_frame(&self, vm: &VirtualMachine) -> Option { + pub fn next_external_frame(&self, vm: &VirtualMachine) -> Option { let mut frame = self.f_back(vm); while let Some(ref f) = frame { if !f.is_internal_frame() { @@ -1653,22 +2170,71 @@ impl Py { } } +/// Execute an InterpreterFrame's bytecode directly, without a FrameObject. +/// +/// # Safety +/// The InterpreterFrame's raw pointers (code, globals, builtins, func_obj) +/// must be valid for the duration of this call. +#[inline(always)] +pub(crate) fn run_iframe( + iframe: &mut InterpreterFrame, + vm: &VirtualMachine, +) -> PyResult { + let code: &Py = unsafe { &*iframe.code }; + let globals: &Py = unsafe { &*iframe.globals }; + let builtins: &PyObject = unsafe { &*iframe.builtins }; + let func_obj: Option<&PyObject> = if iframe.func_obj.is_null() { + None + } else { + Some(unsafe { &*iframe.func_obj }) + }; + let builtins_dict = if globals.class().is(vm.ctx.types.dict_type) { + builtins + .downcast_ref_if_exact::(vm) + .map(|d| unsafe { PyExact::ref_unchecked(d) }) + } else { + None + }; + let iframe_ptr = iframe as *const InterpreterFrame; + let mut exec = ExecutingFrame { + code, + localsplus: &mut iframe.localsplus, + locals: &iframe.locals, + globals, + builtins, + builtins_dict, + lasti: &iframe.lasti, + iframe: iframe_ptr, + func_obj, + prev_line: &mut iframe.prev_line, + monitoring_mask: 0, + }; + exec.run(vm) +} + /// An executing frame; borrows mutable frame-internal data for the duration /// of bytecode execution. -struct ExecutingFrame<'a> { - code: &'a PyRef, +pub(crate) struct ExecutingFrame<'a> { + code: &'a Py, localsplus: &'a mut LocalsPlus, locals: &'a FrameLocals, - globals: &'a PyDictRef, - builtins: &'a PyObjectRef, + globals: &'a Py, + builtins: &'a PyObject, /// Cached downcast of builtins to PyDict for fast LOAD_GLOBAL. /// Only set when both globals and builtins are exact dict types (not /// subclasses), so that `__missing__` / `__getitem__` overrides are /// not bypassed. builtins_dict: Option<&'a PyExact>, - object: &'a Py, + /// Raw pointer to the underlying InterpreterFrame. Used to access the + /// materialized FrameObject (via `frame_obj()`) and frame-level fields + /// like trace, pending_stack_pops, etc. Stored as a raw pointer because + /// mutable borrows to `localsplus` and `prev_line` are also held. + /// All accesses through this pointer use atomic/mutex operations. + iframe: *const InterpreterFrame, + /// Borrowed function object that created this frame (if any). + func_obj: Option<&'a PyObject>, lasti: &'a PyAtomic, - prev_line: &'a mut u32, + prev_line: &'a core::cell::Cell, /// Cached monitoring events mask. Reloaded at Resume instruction only, monitoring_mask: u32, } @@ -1757,7 +2323,7 @@ fn localsplus_name(code: &PyCode, idx: usize) -> &'static PyStrInterned { /// heap copy. Otherwise (the frame escaped through a traceback, /// `sys._getframe`, a trace callback, ...) the values are copied to the heap /// first so they stay readable through the escaped reference. -pub(crate) fn release_datastack_frame(frame: &Py, vm: &VirtualMachine) { +pub(crate) fn release_datastack_frame(frame: &Py, vm: &VirtualMachine) { let frame_obj = frame.as_object(); // Uniqueness argument: at this point the frame is already out of // the thread-frames registry and the current-frame chain @@ -1805,8 +2371,17 @@ pub(crate) fn release_datastack_frame(frame: &Py, vm: &VirtualMachine) { // the caller returns and leaves the live frame chain. The caller is still // executing here (this frame is unwinding back into it), so its payload // pointer is live. - // SAFETY: `previous` points at the live caller on this thread's stack. - *frame.retained_back.lock() = unsafe { owned_chain_frame(frame.previous_frame()) }; + { + let mut guard = frame.iframe().retained_back.lock(); + if guard.is_none() { + let prev = frame.previous_iframe(); + *guard = unsafe { owned_chain_frame(prev) }; + } + } + // Note: previous is NOT cleared here. retained_back captures the + // caller reference, and previous may be read again by f_back or + // frame chain walkers (the pointer is live as long as the caller + // is still executing, which it is at this point). // Invariant: a tracked frame must always have heap-backed localsplus // (proven here for escaped datastack frames and by construction for // generator frames, which are born heap-backed). A stop-the-world @@ -2017,7 +2592,54 @@ impl fmt::Debug for ExecutingFrame<'_> { } } +/// Run bytecode in a light frame context. +#[allow(clippy::too_many_arguments)] impl ExecutingFrame<'_> { + /// Get the underlying InterpreterFrame. + #[inline(always)] + fn iframe(&self) -> &InterpreterFrame { + // SAFETY: the iframe pointer is valid for the lifetime of the ExecutingFrame. + unsafe { &*self.iframe } + } + + /// Get the frame object. Materializes a FrameObject on demand if this + /// is a stack-allocated frame that hasn't been observed yet. + #[cold] + #[inline(never)] + fn frame_object(&self, vm: &VirtualMachine) -> FrameObjectRef { + self.iframe().materialize(vm).to_owned() + } + + /// Whether this frame has a per-frame trace function set. + #[inline] + fn trace_is_set(&self, _vm: &VirtualMachine) -> bool { + self.iframe().trace.lock().is_some() + } + + /// Access the frame's trace_opcodes lock. + #[inline] + fn trace_opcodes_is_set(&self) -> bool { + *self.iframe().trace_opcodes.lock() + } + + /// Get pending_stack_pops from the frame. + #[inline] + fn pending_stack_pops(&self) -> u32 { + self.iframe().pending_stack_pops.load(Relaxed) + } + + /// Get pending_unwind_from_stack from the frame. + #[inline] + fn pending_unwind_from_stack(&self) -> i64 { + self.iframe().pending_unwind_from_stack.load(Relaxed) + } + + /// Set pending_stack_pops on the frame. + #[inline] + fn set_pending_stack_pops(&self, val: u32) { + self.iframe().pending_stack_pops.store(val, Relaxed); + } + /// Run `__init__` for the tp_new specialization. `args` holds the /// `__init__` args with slot 0 left empty; it is filled with `new_obj` /// here. Enforces the `__init__() should return None` contract and @@ -2108,7 +2730,7 @@ impl ExecutingFrame<'_> { /// Matches `_PyEval_MonitorRaise` → `PY_MONITORING_EVENT_RAISE` → /// `sys_trace_exception_func` in legacy_tracing.c. fn fire_exception_trace(&self, exc: &PyBaseExceptionRef, vm: &VirtualMachine) -> PyResult<()> { - if vm.use_tracing.get() && !vm.is_none(&self.object.trace.lock()) { + if vm.use_tracing.get() && self.trace_is_set(vm) { let exc_type: PyObjectRef = exc.class().to_owned().into(); let exc_value: PyObjectRef = exc.clone().into(); let exc_tb: PyObjectRef = exc @@ -2122,7 +2744,7 @@ impl ExecutingFrame<'_> { fn run(&mut self, vm: &VirtualMachine) -> PyResult { flame_guard!(format!( - "Frame::run({obj_name})", + "FrameObject::run({obj_name})", obj_name = self.code.obj_name )); // Execute until return or exception: @@ -2140,26 +2762,26 @@ impl ExecutingFrame<'_> { // (frames entered before sys.settrace() have trace=None). // Skip RESUME – it should not generate user-visible line events. if vm.use_tracing.get() - && !vm.is_none(&self.object.trace.lock()) + && self.trace_is_set(vm) && !matches!( self.code.instructions.read_op(idx), Instruction::Resume { .. } | Instruction::InstrumentedResume ) && let Some((loc, _)) = self.code.locations.get(idx) - && loc.line.get() as u32 != *self.prev_line + && loc.line.get() as u32 != self.prev_line.get() { - *self.prev_line = loc.line.get() as u32; + self.prev_line.set(loc.line.get() as u32); vm.trace_event(crate::protocol::TraceEvent::Line, None)?; // Trace callback may have changed lasti via set_f_lineno. // Re-read and restart the loop from the new position. if self.lasti() != (idx as u32 + 1) { // set_f_lineno defers stack unwinding because we hold // the state mutex. Perform it now. - let pops = self.object.pending_stack_pops(); + let pops = self.pending_stack_pops(); if pops > 0 { - let from_stack = self.object.pending_unwind_from_stack(); + let from_stack = self.pending_unwind_from_stack(); self.unwind_stack_for_lineno(pops as usize, from_stack, vm); - self.object.set_pending_stack_pops(0); + self.set_pending_stack_pops(0); } arg_state.reset(); continue; @@ -2170,22 +2792,30 @@ impl ExecutingFrame<'_> { let mut do_extend_arg = false; let caches = op.cache_entries(); - // Update prev_line only when tracing or monitoring is active. - // When neither is enabled, prev_line is stale but unused. - if vm.use_tracing.get() { - if !matches!( - op.into(), - Opcode::Resume | Opcode::ExtendedArg | Opcode::InstrumentedLine - ) && let Some((loc, _)) = self.code.locations.get(idx) - { - *self.prev_line = loc.line.get() as u32; - } + // Always update prev_line so f_lineno returns the correct line + // even when the frame is observed mid-call (e.g. sys._getframe, + // warnings.warn). The lookup is a simple array index, so the + // cost is negligible. + // Update prev_line for f_lineno. Skip RESUME, ExtendedArg, + // and InstrumentedLine (it manages prev_line in its own handler; + // updating here first would defeat LINE de-duplication). + // Other instrumented opcodes update prev_line via + // execute_instrumented. + if !matches!( + op.into(), + Opcode::Resume | Opcode::ExtendedArg | Opcode::InstrumentedLine + ) && !op.is_instrumented() + && let Some((loc, _)) = self.code.locations.get(idx) + { + self.prev_line.set(loc.line.get() as u32); + } + if vm.use_tracing.get() { // Fire 'opcode' trace event for sys.settrace when f_trace_opcodes // is set. Skip RESUME and ExtendedArg // (_Py_call_instrumentation_instruction). - if !vm.is_none(&self.object.trace.lock()) - && *self.object.trace_opcodes.lock() + if self.trace_is_set(vm) + && self.trace_opcodes_is_set() && !matches!( op.into(), Opcode::Resume | Opcode::InstrumentedResume | Opcode::ExtendedArg @@ -2208,7 +2838,7 @@ impl ExecutingFrame<'_> { let next = exception.__traceback__(); let new_traceback = PyTraceback::new( next, - frame.object.to_owned(), + frame.frame_object(vm), idx as u32 * 2, loc.line, ); @@ -2277,7 +2907,7 @@ impl ExecutingFrame<'_> { let new_traceback = PyTraceback::new( next, - frame.object.to_owned(), + frame.frame_object(vm), idx as u32 * 2, loc.line, ); @@ -2388,7 +3018,9 @@ impl ExecutingFrame<'_> { // The traceback was created with the correct lasti when exception // was first raised, but frame.lasti may have changed during cleanup if let Some(tb) = exception.__traceback__() - && core::ptr::eq::>(&*tb.frame, self.object) + && self.iframe().frame_obj().is_some_and(|fo| { + core::ptr::eq::>(&*tb.frame, fo) + }) { // This traceback entry is for this frame - restore its lasti // tb.lasti is in bytes (idx * 2), convert back to instruction index @@ -2477,12 +3109,8 @@ impl ExecutingFrame<'_> { if idx < self.code.locations.len() { let (loc, _end_loc) = self.code.locations[idx]; let next = err.__traceback__(); - let new_traceback = PyTraceback::new( - next, - self.object.to_owned(), - idx as u32 * 2, - loc.line, - ); + let new_traceback = + PyTraceback::new(next, self.frame_object(vm), idx as u32 * 2, loc.line); err.set_traceback_typed(Some(new_traceback.into_ref(&vm.ctx))); } @@ -2490,7 +3118,7 @@ impl ExecutingFrame<'_> { vm.contextualize_exception(&err); return match self.unwind_blocks(vm, UnwindReason::Raising { exception: err }) { Ok(None) => { - *self.prev_line = 0; + self.prev_line.set(0); self.run(vm) } Ok(Some(result)) => Ok(result), @@ -2524,7 +3152,7 @@ impl ExecutingFrame<'_> { let next = err.__traceback__(); let new_traceback = PyTraceback::new( next, - self.object.to_owned(), + self.frame_object(vm), idx as u32 * 2, loc.line, ); @@ -2535,7 +3163,7 @@ impl ExecutingFrame<'_> { vm.contextualize_exception(&err); match self.unwind_blocks(vm, UnwindReason::Raising { exception: err }) { Ok(None) => { - *self.prev_line = 0; + self.prev_line.set(0); self.run(vm) } Ok(Some(result)) => Ok(result), @@ -2566,7 +3194,7 @@ impl ExecutingFrame<'_> { let (loc, _end_loc) = self.code.locations[idx]; let next = exception.__traceback__(); let new_traceback = - PyTraceback::new(next, self.object.to_owned(), idx as u32 * 2, loc.line); + PyTraceback::new(next, self.frame_object(vm), idx as u32 * 2, loc.line); exception.set_traceback_typed(Some(new_traceback.into_ref(&vm.ctx))); } @@ -2609,7 +3237,7 @@ impl ExecutingFrame<'_> { // Reset prev_line so that the first instruction in the handler // fires a LINE event. In CPython, gen_send_ex re-enters the // eval loop which reinitializes its local prev_instr tracker. - *self.prev_line = 0; + self.prev_line.set(0); self.run(vm) } Ok(Some(result)) => Ok(result), @@ -2673,7 +3301,7 @@ impl ExecutingFrame<'_> { vm: &VirtualMachine, ) -> FrameResult { flame_guard!(format!( - "Frame::execute_instruction({instruction:?} {arg:?})" + "FrameObject::execute_instruction({instruction:?} {arg:?})" )); #[cfg(feature = "vm-tracing-logging")] @@ -2930,9 +3558,7 @@ impl ExecutingFrame<'_> { let n = n.get(arg) as usize; if n > 0 { let closure = self - .object .func_obj - .as_ref() .and_then(|f| f.downcast_ref::()) .and_then(|f| f.closure.as_ref()); let nlocalsplus = self.code.localspluskinds.len(); @@ -3899,7 +4525,7 @@ impl ExecutingFrame<'_> { // Python preserves exception tracebacks even after the exception is no longer // the "current exception". This is important for code that catches an exception, // stores it, and later inspects its traceback. - // Reference cycles (Exception → Traceback → Frame → locals) are handled by + // Reference cycles (Exception → Traceback → FrameObject → locals) are handled by // Python's garbage collector which can detect and break cycles. Ok(None) @@ -4240,7 +4866,7 @@ impl ExecutingFrame<'_> { Ok(None) } PyIterReturn::StopIteration(value) => { - if vm.use_tracing.get() && !vm.is_none(&self.object.trace.lock()) { + if vm.use_tracing.get() && self.trace_is_set(vm) { let stop_exc = vm.new_stop_iteration(value.clone()); self.fire_exception_trace(&stop_exc, vm)?; } @@ -4277,7 +4903,7 @@ impl ExecutingFrame<'_> { return Ok(None); } PyIterReturn::StopIteration(value) => { - if vm.use_tracing.get() && !vm.is_none(&self.object.trace.lock()) { + if vm.use_tracing.get() && self.trace_is_set(vm) { let stop_exc = vm.new_stop_iteration(value.clone()); self.fire_exception_trace(&stop_exc, vm)?; } @@ -4295,7 +4921,7 @@ impl ExecutingFrame<'_> { Ok(None) } PyIterReturn::StopIteration(value) => { - if vm.use_tracing.get() && !vm.is_none(&self.object.trace.lock()) { + if vm.use_tracing.get() && self.trace_is_set(vm) { let stop_exc = vm.new_stop_iteration(value.clone()); self.fire_exception_trace(&stop_exc, vm)?; } @@ -6299,7 +6925,7 @@ impl ExecutingFrame<'_> { self.push_value(value); } Ok(PyIterReturn::StopIteration(value)) => { - if vm.use_tracing.get() && !vm.is_none(&self.object.trace.lock()) { + if vm.use_tracing.get() && self.trace_is_set(vm) { let stop_exc = vm.new_stop_iteration(value); self.fire_exception_trace(&stop_exc, vm)?; } @@ -6387,6 +7013,20 @@ impl ExecutingFrame<'_> { instruction.is_instrumented(), "execute_instrumented called with non-instrumented opcode {instruction:?}" ); + // Update prev_line for f_lineno. The main bytecode loop skips + // instrumented opcodes to avoid interfering with LINE event + // de-duplication in InstrumentedLine. Update here instead, except + // for RESUME (prev_line must stay 0 for the first LINE event) and + // InstrumentedLine (manages prev_line in its own handler). + if !matches!( + instruction, + Instruction::InstrumentedResume | Instruction::InstrumentedLine + ) { + let idx = self.lasti() as usize - 1; + if let Some((loc, _)) = self.code.locations.get(idx) { + self.prev_line.set(loc.line.get() as u32); + } + } self.monitoring_mask = vm.state.monitoring_events.load(); match instruction { Instruction::InstrumentedResume => { @@ -6689,8 +7329,8 @@ impl ExecutingFrame<'_> { // Fire LINE event only if line changed if let Some((loc, _)) = self.code.locations.get(idx) { let line = loc.line.get() as u32; - if line != *self.prev_line && line > 0 { - *self.prev_line = line; + if line != self.prev_line.get() && line > 0 { + self.prev_line.set(line); monitoring::fire_line(vm, self.code, offset, line)?; } } @@ -6700,6 +7340,12 @@ impl ExecutingFrame<'_> { monitoring::fire_instruction(vm, self.code, offset)?; } + // Update prev_line for f_lineno since the bytecode loop's + // update skips all instrumented opcodes. + if let Some((loc, _)) = self.code.locations.get(idx) { + self.prev_line.set(loc.line.get() as u32); + } + // Re-dispatch to the real original opcode let original_op = Instruction::try_from(real_op_byte) .expect("invalid opcode in side-table chain"); @@ -6781,7 +7427,7 @@ impl ExecutingFrame<'_> { if let Some(builtins_dict) = self.builtins_dict { // Fast path: both globals and builtins are exact dicts // SAFETY: builtins_dict is only set when globals is also exact dict - let globals_exact = unsafe { PyExact::ref_unchecked(self.globals.as_ref()) }; + let globals_exact = unsafe { PyExact::ref_unchecked(self.globals) }; globals_exact .get_chain_exact(builtins_dict, name, vm)? .ok_or_else(|| { @@ -6802,7 +7448,7 @@ impl ExecutingFrame<'_> { } } - #[cfg_attr(feature = "flame-it", flame("Frame"))] + #[cfg_attr(feature = "flame-it", flame("FrameObject"))] fn import(&mut self, vm: &VirtualMachine, module_name: Option<&Py>) -> PyResult<()> { let module_name = module_name.unwrap_or(vm.ctx.empty_str); let top = self.pop_value(); @@ -6818,7 +7464,7 @@ impl ExecutingFrame<'_> { Ok(()) } - #[cfg_attr(feature = "flame-it", flame("Frame"))] + #[cfg_attr(feature = "flame-it", flame("FrameObject"))] fn import_from(&mut self, vm: &VirtualMachine, idx: bytecode::NameIdx) -> PyResult { let module = self.top_value(); let name = self.code.names[idx as usize]; @@ -6920,7 +7566,7 @@ impl ExecutingFrame<'_> { Err(err) } - #[cfg_attr(feature = "flame-it", flame("Frame"))] + #[cfg_attr(feature = "flame-it", flame("FrameObject"))] fn import_star(&mut self, vm: &VirtualMachine) -> PyResult<()> { let module = self.pop_value(); @@ -6973,7 +7619,7 @@ impl ExecutingFrame<'_> { /// The reason for unwinding gives a hint on what to do when /// unwinding a block. /// Optionally returns an exception. - #[cfg_attr(feature = "flame-it", flame("Frame"))] + #[cfg_attr(feature = "flame-it", flame("FrameObject"))] fn unwind_blocks(&mut self, vm: &VirtualMachine, reason: UnwindReason) -> FrameResult { // use exception table for exception handling match reason { @@ -7580,7 +8226,7 @@ impl ExecutingFrame<'_> { self.push_value(vm.ctx.new_int(value).into()); return Ok(true); } - if vm.use_tracing.get() && !vm.is_none(&self.object.trace.lock()) { + if vm.use_tracing.get() && self.trace_is_set(vm) { let stop_exc = vm.new_stop_iteration(None); self.fire_exception_trace(&stop_exc, vm)?; } @@ -7599,7 +8245,7 @@ impl ExecutingFrame<'_> { Ok(PyIterReturn::StopIteration(value)) => { // Fire 'exception' trace event for StopIteration, matching // FOR_ITER's inline call to _PyEval_MonitorRaise. - if vm.use_tracing.get() && !vm.is_none(&self.object.trace.lock()) { + if vm.use_tracing.get() && self.trace_is_set(vm) { let stop_exc = vm.new_stop_iteration(value); self.fire_exception_trace(&stop_exc, vm)?; } @@ -7634,7 +8280,7 @@ impl ExecutingFrame<'_> { .expect("Stack value should be code object"); // Create function with minimal attributes - let func_obj = PyFunction::new(code_obj, self.globals.clone(), vm)?.into_pyobject(vm); + let func_obj = PyFunction::new(code_obj, self.globals.to_owned(), vm)?.into_pyobject(vm); self.push_value(func_obj); Ok(None) @@ -7669,7 +8315,7 @@ impl ExecutingFrame<'_> { Ok(None) } - #[cfg_attr(feature = "flame-it", flame("Frame"))] + #[cfg_attr(feature = "flame-it", flame("FrameObject"))] fn execute_bin_op(&mut self, vm: &VirtualMachine, op: bytecode::BinaryOperator) -> FrameResult { let b_ref = &self.pop_value(); let a_ref = &self.pop_value(); @@ -8023,7 +8669,7 @@ impl ExecutingFrame<'_> { Ok(!self._in(vm, needle, haystack)?) } - #[cfg_attr(feature = "flame-it", flame("Frame"))] + #[cfg_attr(feature = "flame-it", flame("FrameObject"))] fn execute_compare(&mut self, vm: &VirtualMachine, arg: bytecode::OpArg) -> FrameResult { let op = bytecode::ComparisonOperator::try_from(u32::from(arg)) .unwrap_or(bytecode::ComparisonOperator::Equal); @@ -10404,12 +11050,12 @@ impl ExecutingFrame<'_> { } } -impl fmt::Debug for Frame { +impl fmt::Debug for FrameObject { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { // SAFETY: Debug is best-effort; concurrent mutation is unlikely // and would only affect debug output. let Some(iframe) = (unsafe { &*self.iframe.get() }) else { - return f.write_str("Frame Object { cleared }"); + return f.write_str("FrameObject Object { cleared }"); }; let stack_str = iframe @@ -10433,9 +11079,9 @@ impl fmt::Debug for Frame { // TODO: fix this up write!( f, - "Frame Object {{ \n Stack:{}\n Locals initialized:{}\n}}", + "FrameObject Object {{ \n Stack:{}\n Locals initialized:{}\n}}", stack_str, - self.locals.get().is_some() + self.iframe().locals.get().is_some() ) } } diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs index ebae58f96..5843dfe24 100644 --- a/crates/vm/src/gc_state.rs +++ b/crates/vm/src/gc_state.rs @@ -633,33 +633,26 @@ impl GcState { // classified reachable. A running frame appearing in `unreachable` // would mean the reachability analysis observed its interpreter state // as garbage — the exact hazard the barrier exists to prevent. + // Verify no running frame is classified unreachable. + // Walk the TLS frame chain (CURRENT_FRAME) instead of top_frame, + // because stack-allocated frames update only CURRENT_FRAME (via + // set_current_frame_nosave), not top_frame. #[cfg(all(unix, feature = "threading", debug_assertions))] if stw.stopped { let unreachable_set: HashSet = unreachable.iter().copied().collect(); - crate::vm::thread::try_with_current_vm(|vm| { - let registry = vm.state.thread_frames.lock(); - #[expect( - clippy::iter_over_hash_type, - reason = "assertion over every registered thread slot" - )] - for slot in registry.values() { - let mut cur = slot.top_frame.load(core::sync::atomic::Ordering::Relaxed) - as *const crate::frame::Frame; - while !cur.is_null() { - // SAFETY: frames on a thread's active call stack are - // alive, and the world is stopped so none can be popped. - let obj = - unsafe { &*crate::Py::::from_payload_ptr(cur) } - .as_object(); - let ptr = GcPtr(NonNull::from(obj)); - debug_assert!( - !unreachable_set.contains(&ptr), - "running frame {obj:p} classified unreachable during GC" - ); - cur = unsafe { (*cur).previous_frame() }; - } + let mut cur = crate::vm::thread::get_current_frame(); + while !cur.is_null() { + let iframe = unsafe { &*cur }; + if let Some(fo) = iframe.frame_obj() { + let obj = fo.as_object(); + let ptr = GcPtr(NonNull::from(obj)); + debug_assert!( + !unreachable_set.contains(&ptr), + "running frame {obj:p} classified unreachable during GC" + ); } - }); + cur = iframe.previous(); + } } if debug.contains(GcDebugFlags::STATS) { diff --git a/crates/vm/src/import.rs b/crates/vm/src/import.rs index f7cc03d99..eb07c76a2 100644 --- a/crates/vm/src/import.rs +++ b/crates/vm/src/import.rs @@ -221,12 +221,12 @@ fn remove_importlib_frames_inner( return (None, false); }; - let file_name = traceback.frame.code.source_path().as_str(); + let file_name = traceback.frame.iframe().code().source_path().as_str(); let (inner_tb, mut now_in_importlib) = remove_importlib_frames_inner(vm, traceback.next.lock().clone(), always_trim); if file_name == "_frozen_importlib" || file_name == "_frozen_importlib_external" { - if traceback.frame.code.obj_name.as_str() == "_call_with_frames_removed" { + if traceback.frame.iframe().code().obj_name.as_str() == "_call_with_frames_removed" { now_in_importlib = true; } if always_trim || now_in_importlib { diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index 228fe1290..0ee7a062e 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -211,7 +211,7 @@ pub(super) unsafe fn default_dealloc(obj: *mut PyObject) { } // Extract child references to break circular refs (tp_clear), then drop - // them. Some payloads (e.g. Frame) drop children in place inside clear_fn + // them. Some payloads (e.g. FrameObject) drop children in place inside clear_fn // instead of extracting them, so user code (`__del__`) may run here. let mut edges = Vec::new(); if let Some(clear_fn) = vtable.clear { @@ -2191,6 +2191,7 @@ impl Py { /// obtained by dereferencing a `Py`), and the object must outlive the /// returned pointer's use. #[inline] + #[cfg_attr(not(feature = "threading"), allow(dead_code))] pub(crate) unsafe fn from_payload_ptr(payload: *const T) -> *const Self { let offset = core::mem::offset_of!(PyInner, payload); // `Py` is a newtype over `PyInner`, so their addresses coincide. diff --git a/crates/vm/src/object/ext.rs b/crates/vm/src/object/ext.rs index e576ac2c1..c4732d1cb 100644 --- a/crates/vm/src/object/ext.rs +++ b/crates/vm/src/object/ext.rs @@ -333,7 +333,7 @@ impl PyAtomicRef { pub fn swap_to_temporary_refs(&self, pyref: PyRef, vm: &VirtualMachine) { let old = unsafe { self.swap(pyref) }; if let Some(frame) = vm.current_frame() { - frame.temporary_refs.lock().push(old.into()); + frame.iframe().temporary_refs.lock().push(old.into()); } } } @@ -409,7 +409,7 @@ impl PyAtomicRef> { return; }; if let Some(frame) = vm.current_frame() { - frame.temporary_refs.lock().push(old.into()); + frame.iframe().temporary_refs.lock().push(old.into()); } } } @@ -452,7 +452,7 @@ impl PyAtomicRef { pub fn swap_to_temporary_refs(&self, obj: PyObjectRef, vm: &VirtualMachine) { let old = unsafe { self.swap(obj) }; if let Some(frame) = vm.current_frame() { - frame.temporary_refs.lock().push(old); + frame.iframe().temporary_refs.lock().push(old); } } } @@ -499,7 +499,7 @@ impl PyAtomicRef> { return; }; if let Some(frame) = vm.current_frame() { - frame.temporary_refs.lock().push(old); + frame.iframe().temporary_refs.lock().push(old); } } } diff --git a/crates/vm/src/object/payload.rs b/crates/vm/src/object/payload.rs index b6590239e..261b27821 100644 --- a/crates/vm/src/object/payload.rs +++ b/crates/vm/src/object/payload.rs @@ -51,7 +51,7 @@ pub trait PyPayload: MaybeTraverse + PyThreadingConstraint + Sized + 'static { /// Whether `PyRef::new_ref` skips auto-tracking this type in the GC even /// when it would otherwise qualify (has traverse, dict, or heap type). /// Such objects are created untracked and must be tracked explicitly if - /// and when they can become part of a reference cycle. Used by `Frame`, + /// and when they can become part of a reference cycle. Used by `FrameObject`, /// which is created untracked and tracked lazily only on escape. const NEW_REF_UNTRACKED: bool = false; diff --git a/crates/vm/src/protocol/callable.rs b/crates/vm/src/protocol/callable.rs index 70e0a54dc..9e80c5b2c 100644 --- a/crates/vm/src/protocol/callable.rs +++ b/crates/vm/src/protocol/callable.rs @@ -228,12 +228,12 @@ impl VirtualMachine { let is_profile_event = event.is_profile_event(); let is_opcode_event = event.is_opcode_event(); - let Some(frame_ref) = self.current_frame() else { + let Some(frame_ref) = crate::frame::current_thread_frame_materialize(self) else { return Ok(None); }; // Opcode events are only dispatched when f_trace_opcodes is set. - if is_opcode_event && !*frame_ref.trace_opcodes.lock() { + if is_opcode_event && !*frame_ref.iframe().trace_opcodes.lock() { return Ok(None); } @@ -261,7 +261,7 @@ impl VirtualMachine { // trace_trampoline behavior: clear per-frame f_trace // and propagate the error. if let Some(frame_ref) = self.current_frame() { - *frame_ref.trace.lock() = self.ctx.none(); + *frame_ref.iframe().trace.lock() = None; } return Err(e); } diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index ce41a9428..43a869fbe 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -5393,10 +5393,10 @@ mod _io { if vm.state.config.settings.warn_default_encoding { let mut stacklevel = stacklevel.unwrap_or(2); if stacklevel > 1 - && let Some(frame) = vm.current_frame() + && let Some(code) = crate::frame::current_code() && let Some(stdlib_dir) = vm.state.config.paths.stdlib_dir.as_deref() { - let path = frame.code.source_path().as_str(); + let path = code.source_path().as_str(); if !path.starts_with(stdlib_dir) { stacklevel = stacklevel.saturating_sub(1); } diff --git a/crates/vm/src/stdlib/_thread.rs b/crates/vm/src/stdlib/_thread.rs index a1942adc7..e9d75dc02 100644 --- a/crates/vm/src/stdlib/_thread.rs +++ b/crates/vm/src/stdlib/_thread.rs @@ -25,7 +25,7 @@ pub(crate) mod _thread { PyUtf8StrRef, }, common::{lock::PyMutex, wtf8::Wtf8Buf}, - frame::FrameRef, + frame::FrameObjectRef, function::{ArgCallable, FuncArgs, KwArgs, OptionalArg, PySetterValue, TimeoutSeconds}, object::{Traverse, TraverseFn}, types::{Constructor, GetAttr, Representable, SetAttr}, @@ -1161,44 +1161,117 @@ pub(crate) mod _thread { pub(crate) use crate::vm::thread::CurrentFrameSlot; /// Get all threads' current (top) frames. Used by sys._current_frames(). - pub(crate) fn get_all_current_frames(vm: &VirtualMachine) -> Vec<(u64, FrameRef)> { + pub(crate) fn get_all_current_frames(vm: &VirtualMachine) -> Vec<(u64, FrameObjectRef)> { // unix: read each thread's published top frame under stop-the-world so // the owning thread is parked at a safepoint and cannot pop or free the // frame while we take a strong reference. Request stop-the-world before // the registry lock to avoid deadlocking a thread parking mid-registry. + // + // For the current thread, use TLS CURRENT_FRAME directly because + // stack-allocated frames only update TLS (not top_frame). #[cfg(unix)] { use core::sync::atomic::Ordering; + let current_ident = get_ident(); vm.state.stop_the_world.stop_the_world(vm); scopeguard::defer! { vm.state.stop_the_world.start_the_world(vm); } let registry = vm.state.thread_frames.lock(); registry .iter() .filter_map(|(id, slot)| { - let top = slot.top_frame.load(Ordering::Relaxed); - core::ptr::NonNull::new(top).map(|p| { - // SAFETY: world stopped -> the owning thread is parked - // and cannot pop or free this frame; it is alive on - // that thread's call stack. - let py = - unsafe { &*Py::::from_payload_ptr(p.as_ptr()) }; - (*id, py.to_owned()) - }) + if *id == current_ident { + // Current thread: materialize from TLS chain + crate::frame::current_thread_frame_materialize(vm).map(|frame| (*id, frame)) + } else { + // Other threads: try top_frame first (FrameObject), + // fall back to top_iframe (may be a stack-allocated frame). + let top = slot.top_frame.load(Ordering::Relaxed); + if let Some(p) = core::ptr::NonNull::new(top) { + let py = unsafe { + &*Py::::from_payload_ptr(p.as_ptr()) + }; + Some((*id, py.to_owned())) + } else { + // Stack-allocated frame: materialize from top_iframe. + // SAFETY: world stopped -> owning thread is parked. + let iframe_ptr = slot.top_iframe.load(Ordering::Relaxed) + as *const crate::frame::InterpreterFrame; + if !iframe_ptr.is_null() { + // Materialize the entire frame chain and link + // retained_back so f_back works after STW ends. + let mut cur = iframe_ptr; + let mut child_fo: Option> = + None; + while !cur.is_null() { + let iframe = unsafe { &*cur }; + let fo = iframe.materialize(vm).to_owned(); + if let Some(child) = child_fo.take() { + let mut guard = child.iframe().retained_back.lock(); + if guard.is_none() { + *guard = Some(fo.clone()); + } + } + child_fo = Some(fo); + cur = iframe.previous(); + } + let iframe = unsafe { &*iframe_ptr }; + let fo = iframe.materialize(vm); + Some((*id, fo.to_owned())) + } else { + None + } + } + } }) .collect() } #[cfg(not(unix))] { + use core::sync::atomic::Ordering; + let current_ident = get_ident(); + vm.state.stop_the_world.stop_the_world(vm); + scopeguard::defer! { vm.state.stop_the_world.start_the_world(vm); } let registry = vm.state.thread_frames.lock(); registry .iter() .filter_map(|(id, slot)| { - let frames = slot.frames.lock(); - // SAFETY: the owning thread can't pop while we hold the Mutex, - // so the FramePtr is valid for the duration of the lock. - frames - .last() - .map(|fp| (*id, unsafe { fp.as_ref() }.to_owned())) + if *id == current_ident { + // Current thread: materialize from TLS chain + crate::frame::current_thread_frame_materialize(vm).map(|frame| (*id, frame)) + } else { + // Other threads: use top_iframe to include + // stack-allocated frames. Materialize the entire + // chain and link retained_back so f_back works. + // SAFETY: world stopped -> owning thread is parked. + let iframe_ptr = slot.top_iframe.load(Ordering::Relaxed) + as *const crate::frame::InterpreterFrame; + if !iframe_ptr.is_null() { + let mut cur = iframe_ptr; + let mut child_fo: Option> = + None; + while !cur.is_null() { + let iframe = unsafe { &*cur }; + let fo = iframe.materialize(vm).to_owned(); + if let Some(child) = child_fo.take() { + let mut guard = child.iframe().retained_back.lock(); + if guard.is_none() { + *guard = Some(fo.clone()); + } + } + child_fo = Some(fo); + cur = iframe.previous(); + } + let iframe = unsafe { &*iframe_ptr }; + let fo = iframe.materialize(vm); + Some((*id, fo.to_owned())) + } else { + // Fall back to frames stack for FrameObject-only path + let frames = slot.frames.lock(); + frames + .last() + .map(|fp| (*id, unsafe { fp.as_ref() }.to_owned())) + } + } }) .collect() } diff --git a/crates/vm/src/stdlib/_typing.rs b/crates/vm/src/stdlib/_typing.rs index 04b9208b3..0214c3cc5 100644 --- a/crates/vm/src/stdlib/_typing.rs +++ b/crates/vm/src/stdlib/_typing.rs @@ -417,9 +417,8 @@ pub(crate) mod decl { }; // Get caller's module name from frame globals, like typevar.rs caller() - let module = vm - .current_frame() - .and_then(|f| f.globals.get_item("__name__", vm).ok()); + let module = + crate::frame::current_globals().and_then(|g| g.get_item("__name__", vm).ok()); Ok(Self::new_eager(name, type_params, value, module)) } diff --git a/crates/vm/src/stdlib/builtins.rs b/crates/vm/src/stdlib/builtins.rs index e329763fa..7817d6ecb 100644 --- a/crates/vm/src/stdlib/builtins.rs +++ b/crates/vm/src/stdlib/builtins.rs @@ -126,12 +126,12 @@ mod builtins { fn merge_compile_future_features( flags: i32, dont_inherit: bool, - vm: &VirtualMachine, + _vm: &VirtualMachine, ) -> bytecode::CodeFlags { let mut future_features = compile_future_features_from_flags(flags); - if !dont_inherit && let Some(frame) = vm.current_frame() { + if !dont_inherit && let Some(code) = crate::frame::current_code() { future_features |= bytecode::CodeFlags::from_bits_truncate( - frame.code.flags.bits() & compile_future_feature_mask().bits(), + code.flags.bits() & compile_future_feature_mask().bits(), ); } future_features @@ -649,9 +649,9 @@ mod builtins { Either::A(string) => { let source = string.as_str(); let mut opts = vm.compile_opts(); - if let Some(frame) = vm.current_frame() { + if let Some(code) = crate::frame::current_code() { opts.future_features = bytecode::CodeFlags::from_bits_truncate( - frame.code.flags.bits() & compile_future_feature_mask().bits(), + code.flags.bits() & compile_future_feature_mask().bits(), ); } vm.compile_with_opts(source, mode, "", opts) diff --git a/crates/vm/src/stdlib/sys.rs b/crates/vm/src/stdlib/sys.rs index 65917865d..8f264d7a7 100644 --- a/crates/vm/src/stdlib/sys.rs +++ b/crates/vm/src/stdlib/sys.rs @@ -44,7 +44,7 @@ pub mod sys { hash::{PyHash, PyUHash}, }, convert::ToPyObject, - frame::FrameRef, + frame::FrameObjectRef, function::{FuncArgs, KwArgs, OptionalArg, PosArgs}, stdlib::{_warnings::warn, builtins}, types::PyStructSequence, @@ -968,9 +968,9 @@ pub mod sys { } #[pyfunction] - fn _getframe(offset: OptionalArg, vm: &VirtualMachine) -> PyResult { + fn _getframe(offset: OptionalArg, vm: &VirtualMachine) -> PyResult { let offset = offset.into_option().unwrap_or(0); - let frame_ref = crate::frame::frame_at_offset(offset) + let frame_ref = crate::frame::frame_at_offset(offset, vm) .ok_or_else(|| vm.new_value_error("call stack is not deep enough"))?; frame_ref.mark_escaped(); @@ -992,8 +992,8 @@ pub mod sys { } // Get the frame at the specified depth - let func_obj = match crate::frame::frame_at_offset(depth) { - Some(frame) => frame.func_obj.clone(), + let func_obj = match crate::frame::frame_at_offset(depth, vm) { + Some(frame) => frame.iframe().func_obj().map(|o| o.to_owned()), None => return Ok(vm.ctx.none()), }; diff --git a/crates/vm/src/stdlib/sys/monitoring.rs b/crates/vm/src/stdlib/sys/monitoring.rs index f468a86b0..7e47185bb 100644 --- a/crates/vm/src/stdlib/sys/monitoring.rs +++ b/crates/vm/src/stdlib/sys/monitoring.rs @@ -1,5 +1,5 @@ use crate::{ - AsObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, + AsObject, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, builtins::{PyCode, PyDictRef, PyNamespace, PyUtf8StrRef, code::CoMonitoringData}, function::FuncArgs, }; @@ -528,16 +528,23 @@ fn update_events_mask(vm: &VirtualMachine, state: &MonitoringState) { // Each code object gets only the events that apply to it (global + its // own local events), preventing e.g. INSTRUCTION from being applied to // unrelated code objects. - crate::frame::for_each_current_frame(|frame| { - let code = &frame.code; - let code_ver = code.instrumentation_version.load(Ordering::Acquire); - if code_ver != new_ver { - let code_events = state.events_for_code(code.get_id()); - instrument_code(code, code_events); - code.instrumentation_version - .store(new_ver, Ordering::Release); + // Re-instrument all frames on the current thread's stack, including + // stack-allocated iframes (with_iframe path) that have no FrameObject. + { + let mut cur = crate::vm::thread::get_current_frame(); + while !cur.is_null() { + let iframe_ref = unsafe { &*cur }; + let code = iframe_ref.code(); + let code_ver = code.instrumentation_version.load(Ordering::Acquire); + if code_ver != new_ver { + let code_events = state.events_for_code(code.get_id()); + instrument_code(code, code_events); + code.instrumentation_version + .store(new_ver, Ordering::Release); + } + cur = iframe_ref.previous(); } - }); + } } fn use_tool_id(tool_id: i32, name: &str, vm: &VirtualMachine) -> PyResult<()> { @@ -740,7 +747,7 @@ thread_local! { fn fire( vm: &VirtualMachine, event: u32, - code: &PyRef, + code: &Py, offset: u32, cb_extra: &[PyObjectRef], ) -> PyResult<()> { @@ -788,7 +795,7 @@ fn fire( } let mut args_vec = Vec::with_capacity(1 + cb_extra.len()); - args_vec.push(code.clone().into()); + args_vec.push(code.to_owned().into()); args_vec.extend_from_slice(cb_extra); let args = FuncArgs::from(args_vec); @@ -823,11 +830,7 @@ fn fire( // Public dispatch functions (called from frame.rs) -pub(crate) fn fire_py_start( - vm: &VirtualMachine, - code: &PyRef, - offset: u32, -) -> PyResult<()> { +pub(crate) fn fire_py_start(vm: &VirtualMachine, code: &Py, offset: u32) -> PyResult<()> { fire( vm, EVENT_PY_START, @@ -837,11 +840,7 @@ pub(crate) fn fire_py_start( ) } -pub(crate) fn fire_py_resume( - vm: &VirtualMachine, - code: &PyRef, - offset: u32, -) -> PyResult<()> { +pub(crate) fn fire_py_resume(vm: &VirtualMachine, code: &Py, offset: u32) -> PyResult<()> { fire( vm, EVENT_PY_RESUME, @@ -853,7 +852,7 @@ pub(crate) fn fire_py_resume( pub(crate) fn fire_py_return( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, retval: &PyObjectRef, ) -> PyResult<()> { @@ -868,7 +867,7 @@ pub(crate) fn fire_py_return( pub(crate) fn fire_py_yield( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, retval: &PyObjectRef, ) -> PyResult<()> { @@ -883,7 +882,7 @@ pub(crate) fn fire_py_yield( pub(crate) fn fire_call( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, callable: &PyObjectRef, arg0: PyObjectRef, @@ -899,7 +898,7 @@ pub(crate) fn fire_call( pub(crate) fn fire_c_return( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, callable: &PyObjectRef, arg0: PyObjectRef, @@ -915,7 +914,7 @@ pub(crate) fn fire_c_return( pub(crate) fn fire_c_raise( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, callable: &PyObjectRef, arg0: PyObjectRef, @@ -931,7 +930,7 @@ pub(crate) fn fire_c_raise( pub(crate) fn fire_line( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, line: u32, ) -> PyResult<()> { @@ -940,7 +939,7 @@ pub(crate) fn fire_line( pub(crate) fn fire_instruction( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, ) -> PyResult<()> { fire( @@ -954,7 +953,7 @@ pub(crate) fn fire_instruction( pub(crate) fn fire_raise( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, exception: &PyObjectRef, ) -> PyResult<()> { @@ -971,7 +970,7 @@ pub(crate) fn fire_raise( /// preventing duplicate events from chained cleanup handlers. pub(crate) fn fire_reraise( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, exception: &PyObjectRef, ) -> PyResult<()> { @@ -994,7 +993,7 @@ pub(crate) fn fire_reraise( pub(crate) fn fire_exception_handled( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, exception: &PyObjectRef, ) -> PyResult<()> { @@ -1010,7 +1009,7 @@ pub(crate) fn fire_exception_handled( pub(crate) fn fire_py_unwind( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, exception: &PyObjectRef, ) -> PyResult<()> { @@ -1026,7 +1025,7 @@ pub(crate) fn fire_py_unwind( pub(crate) fn fire_py_throw( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, exception: &PyObjectRef, ) -> PyResult<()> { @@ -1039,24 +1038,35 @@ pub(crate) fn fire_py_throw( ) } +/// If `value` is already a `StopIteration`, pass it directly; otherwise wrap +/// it in a new `StopIteration(value)` — matching `PyMonitoring_FireStopIterationEvent`. pub(crate) fn fire_stop_iteration( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, - exception: &PyObjectRef, + value: &PyObjectRef, ) -> PyResult<()> { + let exc: PyObjectRef = if value.fast_isinstance(vm.ctx.exceptions.stop_iteration) { + value.clone() + } else { + vm.ctx + .exceptions + .stop_iteration + .as_object() + .call(vec![value.clone()], vm)? + }; fire( vm, EVENT_STOP_ITERATION, code, offset, - &[vm.ctx.new_int(offset).into(), exception.clone()], + &[vm.ctx.new_int(offset).into(), exc], ) } pub(crate) fn fire_jump( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, destination: u32, ) -> PyResult<()> { @@ -1074,7 +1084,7 @@ pub(crate) fn fire_jump( pub(crate) fn fire_branch_left( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, destination: u32, ) -> PyResult<()> { @@ -1092,7 +1102,7 @@ pub(crate) fn fire_branch_left( pub(crate) fn fire_branch_right( vm: &VirtualMachine, - code: &PyRef, + code: &Py, offset: u32, destination: u32, ) -> PyResult<()> { diff --git a/crates/vm/src/stdlib/typevar.rs b/crates/vm/src/stdlib/typevar.rs index da018b552..3e2581406 100644 --- a/crates/vm/src/stdlib/typevar.rs +++ b/crates/vm/src/stdlib/typevar.rs @@ -47,20 +47,17 @@ pub(crate) mod typevar { /// /// Note: CPython's implementation (in typevarobject.c) gets the module from the /// frame's function object using PyFunction_GetModule(f->f_funcobj). However, - /// RustPython's Frame doesn't store a reference to the function object, so we + /// RustPython's FrameObject doesn't store a reference to the function object, so we /// get the module name from the frame's globals dictionary instead. fn caller(vm: &VirtualMachine) -> Option { - let frame = vm.current_frame()?; - - // In RustPython, we get the module name from frame's globals - // This is similar to CPython's sys._getframe().f_globals.get('__name__') - frame.globals.get_item("__name__", vm).ok() + let globals = crate::frame::current_globals()?; + globals.get_item("__name__", vm).ok() } /// Set __module__ attribute for an object based on the caller's module. /// This follows CPython's behavior for TypeVar and similar objects. fn set_module_from_caller(obj: &PyObject, vm: &VirtualMachine) -> PyResult<()> { - // Note: CPython gets module from frame->f_funcobj, but RustPython's Frame + // Note: CPython gets module from frame->f_funcobj, but RustPython's FrameObject // architecture is different - we use globals['__name__'] instead let module_value: PyObjectRef = if let Some(module_name) = caller(vm) { // Special handling for certain module names diff --git a/crates/vm/src/suggestion.rs b/crates/vm/src/suggestion.rs index 69ce8f5f6..57323ebea 100644 --- a/crates/vm/src/suggestion.rs +++ b/crates/vm/src/suggestion.rs @@ -71,17 +71,23 @@ pub fn offer_suggestions(exc: &Py, vm: &VirtualMachine) -> Opti let tb = exc.__traceback__()?; let tb = tb.iter().last().unwrap_or(tb); - let varnames = tb.frame.code.clone().co_varnames(vm); + let varnames = tb.frame.iframe().code().to_owned().co_varnames(vm); if let Some(suggestions) = calculate_suggestions(varnames.iter(), &name) { return Some(suggestions); }; - let globals: Vec<_> = tb.frame.globals.as_object().try_to_value(vm).ok()?; + let globals: Vec<_> = tb + .frame + .iframe() + .globals() + .as_object() + .try_to_value(vm) + .ok()?; if let Some(suggestions) = calculate_suggestions(globals.iter(), &name) { return Some(suggestions); }; - let builtins: Vec<_> = tb.frame.builtins.try_to_value(vm).ok()?; + let builtins: Vec<_> = tb.frame.iframe().builtins().try_to_value(vm).ok()?; calculate_suggestions(builtins.iter(), &name) } else if exc.class().fast_issubclass(vm.ctx.exceptions.import_error) { let mod_name = exc.as_object().get_attr("name", vm).ok()?; diff --git a/crates/vm/src/types/zoo.rs b/crates/vm/src/types/zoo.rs index 64807fc09..c8667050b 100644 --- a/crates/vm/src/types/zoo.rs +++ b/crates/vm/src/types/zoo.rs @@ -178,7 +178,7 @@ impl TypeZoo { dict_itemiterator_type: dict::PyDictItemIterator::init_builtin_type(), dict_reverseitemiterator_type: dict::PyDictReverseItemIterator::init_builtin_type(), ellipsis_type: slice::PyEllipsis::init_builtin_type(), - frame_type: crate::frame::Frame::init_builtin_type(), + frame_type: crate::frame::FrameObject::init_builtin_type(), frame_locals_proxy_type: frame_locals_proxy::FrameLocalsProxy::init_builtin_type(), function_type: function::PyFunction::init_builtin_type(), generator_type: generator::PyGenerator::init_builtin_type(), diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 801b1297b..a9091f895 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -33,7 +33,7 @@ use crate::{ common::{hash::HashSecret, lock::PyMutex, rc::PyRc}, convert::ToPyObject, exceptions::types::PyBaseException, - frame::{ExecutionResult, Frame, FrameRef}, + frame::{ExecutionResult, FrameObject, FrameObjectRef}, frozen::FrozenModule, function::{ArgMapping, FuncArgs, PySetterValue}, import, @@ -111,23 +111,23 @@ pub struct VirtualMachine { /// Non-owning frame pointer for the non-unix threading frames stack. /// The pointed-to frame is kept alive by the caller of with_frame/resume_gen_frame. /// Unix threading builds publish the top frame through `ThreadSlot::top_frame` -/// and walk the rest via `Frame::previous`, so they do not use this type. +/// and walk the rest via `FrameObject::previous`, so they do not use this type. #[cfg(all(not(unix), feature = "threading"))] #[derive(Copy, Clone)] -pub struct FramePtr(NonNull>); +pub struct FramePtr(NonNull>); #[cfg(all(not(unix), feature = "threading"))] impl FramePtr { /// # Safety /// The pointed-to frame must still be alive. #[must_use] - pub unsafe fn as_ref(&self) -> &Py { + pub unsafe fn as_ref(&self) -> &Py { unsafe { self.0.as_ref() } } } // SAFETY: FramePtr is only stored in a thread's shared frame stack -// (`ThreadSlot::frames`) while the corresponding FrameRef is alive on that +// (`ThreadSlot::frames`) while the corresponding FrameObjectRef is alive on that // thread's call stack; readers dereference it under the slot mutex. #[cfg(all(not(unix), feature = "threading"))] unsafe impl Send for FramePtr {} @@ -1332,7 +1332,16 @@ impl VirtualMachine { } #[inline(always)] - pub fn run_frame(&self, frame: FrameRef) -> PyResult { + /// Run a stack-allocated InterpreterFrame without heap allocation. + /// This is the fast path for regular (non-generator) function calls. + pub fn run_frame_fast(&self, iframe: &mut crate::frame::InterpreterFrame) -> PyResult { + match self.with_iframe(iframe, |iframe| crate::frame::run_iframe(iframe, self))? { + ExecutionResult::Return(value) => Ok(value), + _ => panic!("Got unexpected result from function"), + } + } + + pub fn run_frame(&self, frame: FrameObjectRef) -> PyResult { // Only ordinary (datastack) call frames reach `run_frame`; generator // and coroutine frames are resumed through `resume_gen_frame`. A // datastack frame is created untracked and is tracked lazily only when @@ -1580,8 +1589,10 @@ impl VirtualMachine { /// The margin is doubled for debug/sanitized builds because frame /// evaluation consumes more native stack in those configurations. #[cfg_attr(any(miri, target_env = "musl"), allow(dead_code))] + // 2× CPython's _PY_STACK_MARGIN_BYTES to account for both heavy and + // light frame native stack usage per recursion step. const STACK_MARGIN_BYTES: usize = - (if cfg!(debug_assertions) { 16384 } else { 2048 }) * core::mem::size_of::(); + (if cfg!(debug_assertions) { 16384 } else { 4096 }) * core::mem::size_of::(); /// Get the stack boundaries using platform-specific APIs. /// Returns (base, top) where base is the lowest address and top is the highest. @@ -1667,7 +1678,7 @@ impl VirtualMachine { /// single native frame can exceed the margin and step past it. #[cfg(all(not(miri), not(target_env = "musl")))] #[inline(always)] - fn check_c_stack_overflow(&self) -> bool { + pub(crate) fn check_c_stack_overflow(&self) -> bool { let current_sp = psm::stack_pointer() as usize; let soft_limit = self.c_stack_soft_limit.get(); current_sp < soft_limit @@ -1677,7 +1688,7 @@ impl VirtualMachine { /// the probe during stdlib bootstrap. #[cfg(any(miri, target_env = "musl"))] #[inline(always)] - fn check_c_stack_overflow(&self) -> bool { + pub(crate) fn check_c_stack_overflow(&self) -> bool { false } @@ -1697,68 +1708,230 @@ impl VirtualMachine { f() } - pub fn with_frame PyResult>( + pub fn with_frame PyResult>( &self, - frame: FrameRef, + frame: FrameObjectRef, f: F, ) -> PyResult { - self.with_recursion("", || { - // SAFETY: `frame` (FrameRef) stays alive for the entire closure scope, - // keeping the FramePtr valid. We pass a clone to `f` so that `f` - // consuming its FrameRef doesn't invalidate our pointer. - // Publish the frame for sys._current_frames() and faulthandler. - // On unix, set_current_frame below publishes the top frame into the - // thread slot; only non-unix builds maintain the mutex-guarded Vec. - #[cfg(all(not(unix), feature = "threading"))] - crate::vm::thread::push_thread_frame(FramePtr(NonNull::from(&*frame))); - // Link frame into the signal-safe frame chain (previous pointer). - // This chain is the single source for the current thread's frame - // stack (current_frame, sys._getframe, f_back, monitoring). - let old_frame = crate::vm::thread::set_current_frame((&**frame) as *const Frame); - frame.previous.store( - old_frame as *mut Frame, - core::sync::atomic::Ordering::Relaxed, - ); - // Normal frame calls share the caller's exc_info slot so that - // callees can see the caller's handled exception via sys.exc_info(). - // Save the current value to restore on exit — this prevents - // exc_info pollution from frames with unbalanced - // PUSH_EXC_INFO/POP_EXCEPT (e.g., exception escaping an except block - // whose cleanup entry is missing from the exception table). - // A callee whose bytecode never mutates the slot cannot pollute it, - // so the save/restore is skipped for it. - let save_exc = frame.code.has_exc_handling; - let saved_exc = if save_exc { - self.current_exception() - } else { - None - }; - let old_owner = frame.owner.swap( - crate::frame::FrameOwner::Thread as i8, - core::sync::atomic::Ordering::AcqRel, - ); + self.check_recursive_call("")?; - // Ensure cleanup on panic: restore owner, exc_info, and frame chain. - scopeguard::defer! { - frame.owner.store(old_owner, core::sync::atomic::Ordering::Release); - if save_exc { - self.restore_exception(saved_exc); + // Check the native C stack periodically. The sampling interval + // (every 8th call) balances overhead against the risk of missing + // an overflow between checks, especially when light and heavy + // frames alternate (each recursion step uses different native + // stack amounts). + let depth = self.recursion_depth.get(); + if depth & 7 == 0 && self.check_c_stack_overflow() { + return Err(self.new_recursion_error(String::new())); + } + + self.recursion_depth.update(|d| d + 1); + // Decrement on all exit paths (including panic between here and + // the explicit decrement at the bottom). + let _depth_guard = scopeguard::guard((), |()| { + self.recursion_depth.update(|d| d.saturating_sub(1)) + }); + + #[cfg(all(not(unix), feature = "threading"))] + crate::vm::thread::push_thread_frame(FramePtr(NonNull::from(&*frame))); + let iframe = frame.iframe() as *const crate::frame::InterpreterFrame; + let old_chain = crate::vm::thread::set_current_frame(iframe); + { + #[allow(unused_imports)] + use rustpython_common::atomic::Radium; + frame + .iframe() + .previous + .store(old_chain as usize, core::sync::atomic::Ordering::Relaxed); + } + let save_exc = frame.iframe().code().has_exc_handling; + let saved_exc = if save_exc { + self.current_exception() + } else { + None + }; + let old_owner = frame.iframe().owner.swap( + crate::frame::FrameOwner::Thread as i8, + core::sync::atomic::Ordering::AcqRel, + ); + + let result = self.dispatch_traced_frame(&frame, |frame| f(frame.to_owned())); + + // Capture f_back before clearing previous so code holding a + // reference to this FrameObject can walk the chain after return. + if !old_chain.is_null() { + let strong = frame.as_object().strong_count(); + // Only set retained_back if someone else holds a reference (escaped) + // AND the caller already has a FrameObject. Materializing the caller + // here would add refcounts on its local variables, preventing timely + // __del__ / ResourceWarning on dealloc. If the caller hasn't been + // materialized, f_back will resolve via the TLS chain while the + // caller is still executing, or return None after it returns. + if strong > 1 { + let mut guard = frame.iframe().retained_back.lock(); + if guard.is_none() { + let prev_iframe = unsafe { &*old_chain }; + if let Some(fo) = prev_iframe.frame_obj() { + *guard = Some(fo.to_owned()); + } } - crate::vm::thread::set_current_frame(old_frame); - #[cfg(all(not(unix), feature = "threading"))] - crate::vm::thread::pop_thread_frame(); } + } - self.dispatch_traced_frame(&frame, |frame| f(frame.to_owned())) - }) + frame + .iframe() + .owner + .store(old_owner, core::sync::atomic::Ordering::Release); + if save_exc { + self.restore_exception(saved_exc); + } + // Clear previous before popping — it may point to a stack-allocated + // iframe that will be freed when the caller's with_iframe exits. + { + #[allow(unused_imports)] + use rustpython_common::atomic::Radium; + frame + .iframe() + .previous + .store(0, core::sync::atomic::Ordering::Relaxed); + } + let _ = crate::vm::thread::set_current_frame(old_chain); + #[cfg(all(not(unix), feature = "threading"))] + crate::vm::thread::pop_thread_frame(); + // Disarm the panic guard — normal decrement. + scopeguard::ScopeGuard::into_inner(_depth_guard); + self.recursion_depth.update(|d| d - 1); + + result } - /// Frame execution for generator/coroutine resume. + /// Execute a stack-allocated InterpreterFrame without heap-allocating + /// a FrameObject. This is the fast path for regular function calls. + /// The frame is pushed onto the chain as a `*const InterpreterFrame`. + #[inline(always)] + pub fn with_iframe( + &self, + iframe: &mut crate::frame::InterpreterFrame, + f: impl FnOnce(&mut crate::frame::InterpreterFrame) -> PyResult, + ) -> PyResult { + self.check_recursive_call("")?; + + let depth = self.recursion_depth.get(); + if depth & 7 == 0 && self.check_c_stack_overflow() { + return Err(self.new_recursion_error(String::new())); + } + + self.recursion_depth.update(|d| d + 1); + + let iframe_ptr = iframe as *const crate::frame::InterpreterFrame; + let old_chain = crate::vm::thread::set_current_frame(iframe_ptr); + { + #[allow(unused_imports)] + use rustpython_common::atomic::Radium; + iframe + .previous + .store(old_chain as usize, core::sync::atomic::Ordering::Relaxed); + } + let save_exc = iframe.code().has_exc_handling; + let saved_exc = if save_exc { + self.current_exception() + } else { + None + }; + + let result = f(iframe); + + // If this iframe was materialized, capture f_back so that code + // holding a reference to the FrameObject (e.g. sys._getframe() + // return value, traceback frames) can walk the chain after return. + // + // Read materialized through the raw TLS pointer instead of the + // &mut iframe reference. During f(iframe), bytecode can + // materialize the frame via the TLS chain (a raw pointer alias); + // the &mut borrow lets LLVM assume no aliased writes, which can + // cause the store to be invisible through `iframe.materialized`. + { + // Use read_volatile through the original raw pointer to bypass + // LLVM's noalias assumptions on the &mut iframe borrow. + let mat_ptr = unsafe { + let field_ptr = core::ptr::addr_of!((*iframe_ptr).materialized); + core::ptr::read_volatile(field_ptr as *const usize) + }; + if mat_ptr != 0 { + let fo = unsafe { &*(mat_ptr as *const crate::Py) }; + // Sync localsplus, prev_line, lasti from the live iframe to + // the materialized FrameObject so f_locals, f_lineno, f_lasti + // reflect the final state after execution. + unsafe { + let live_iframe = &*iframe_ptr; + fo.iframe_mut() + .localsplus + .sync_fastlocals_from(&live_iframe.localsplus); + fo.iframe_mut().prev_line.set(live_iframe.prev_line.get()); + #[allow(unused_imports)] + use rustpython_common::atomic::Radium; + fo.iframe_mut().lasti.store( + live_iframe + .lasti + .load(core::sync::atomic::Ordering::Relaxed), + core::sync::atomic::Ordering::Relaxed, + ); + } + if !old_chain.is_null() { + let prev_iframe = unsafe { &*old_chain }; + // Use materialize_chain to avoid cloning localsplus, which + // would create extra refcounts on local variables. The + // lightweight frame has empty localsplus; live values are + // read through find_live_source_iframe when needed. + let back_fo = prev_iframe.materialize_chain(self); + *fo.iframe().retained_back.lock() = Some(back_fo); + } + // Set owner to FrameObject since this frame is no longer + // executing on a thread. + fo.iframe().owner.store( + crate::frame::FrameOwner::FrameObject as i8, + core::sync::atomic::Ordering::Release, + ); + } + } + + if save_exc { + self.restore_exception(saved_exc); + } + // Restore the frame chain BEFORE clearing temporary_refs, so + // top_frame no longer points at the materialized FrameObject + // when its last strong reference is released. + let _ = crate::vm::thread::set_current_frame(old_chain); + self.recursion_depth.update(|d| d - 1); + + // Now that the frame is off the chain, track the materialized + // FrameObject in the GC and release temporary_refs so cycle + // collection can detect and reclaim reference cycles. + { + let mat_ptr = unsafe { + let field_ptr = core::ptr::addr_of!((*iframe_ptr).materialized); + core::ptr::read_volatile(field_ptr as *const usize) + }; + if mat_ptr != 0 { + let fo = unsafe { &*(mat_ptr as *const crate::Py) }; + unsafe { + crate::gc_state::gc_state() + .track_object(core::ptr::NonNull::from(fo.as_object())); + let live_iframe = &*iframe_ptr; + live_iframe.temporary_refs.lock().clear(); + } + } + } + + result + } + + /// FrameObject execution for generator/coroutine resume. /// Pushes a new exc_info slot (gi_exc_state) onto the chain, /// linking the generator's saved handled-exception. - pub fn resume_gen_frame) -> PyResult>( + pub fn resume_gen_frame) -> PyResult>( &self, - frame: &FrameRef, + frame: &FrameObjectRef, exc: Option, f: F, ) -> PyResult { @@ -1768,19 +1941,22 @@ impl VirtualMachine { } self.recursion_depth.update(|d| d + 1); - // SAFETY: frame (&FrameRef) stays alive for the duration, so NonNull is valid until pop. + // SAFETY: frame (&FrameObjectRef) stays alive for the duration, so NonNull is valid until pop. #[cfg(all(not(unix), feature = "threading"))] crate::vm::thread::push_thread_frame(FramePtr(NonNull::from(&**frame))); - let old_frame = crate::vm::thread::set_current_frame((&***frame) as *const Frame); - frame.previous.store( - old_frame as *mut Frame, - core::sync::atomic::Ordering::Relaxed, - ); + let iframe = frame.iframe() as *const crate::frame::InterpreterFrame; + let old_chain = crate::vm::thread::set_current_frame(iframe); + { + #[allow(unused_imports)] + use rustpython_common::atomic::Radium; + frame + .iframe() + .previous + .store(old_chain as usize, core::sync::atomic::Ordering::Relaxed); + } // Push generator's exc_info slot onto the chain - // (gi_exc_state.previous_item = tstate->exc_info; - // tstate->exc_info = &gi_exc_state;) self.push_exception(exc); - let old_owner = frame.owner.swap( + let old_owner = frame.iframe().owner.swap( crate::frame::FrameOwner::Thread as i8, core::sync::atomic::Ordering::AcqRel, ); @@ -1788,9 +1964,16 @@ impl VirtualMachine { // Ensure cleanup on panic: restore owner, pop exc_info slot, frame chain, // frames Vec, and recursion depth. scopeguard::defer! { - frame.owner.store(old_owner, core::sync::atomic::Ordering::Release); + frame.iframe().owner.store(old_owner, core::sync::atomic::Ordering::Release); self.pop_exception(); - crate::vm::thread::set_current_frame(old_frame); + // Clear previous before popping — it may point to a stack-allocated + // iframe that will be freed when the caller's with_iframe exits. + { + #[allow(unused_imports)] + use rustpython_common::atomic::Radium; + frame.iframe().previous.store(0, core::sync::atomic::Ordering::Relaxed); + } + let _ = crate::vm::thread::set_current_frame(old_chain); #[cfg(all(not(unix), feature = "threading"))] crate::vm::thread::pop_thread_frame(); @@ -1809,9 +1992,9 @@ impl VirtualMachine { /// - Fire `TraceEvent::Return` on both normal return **and** exception /// unwind (`PY_UNWIND` → `PyTrace_RETURN` with `arg = None`). /// Propagate any trace-function error, replacing the original exception. - fn dispatch_traced_frame) -> PyResult>( + fn dispatch_traced_frame) -> PyResult>( &self, - frame: &Py, + frame: &Py, f: F, ) -> PyResult { use crate::protocol::TraceEvent; @@ -1819,7 +2002,7 @@ impl VirtualMachine { // Fire 'call' trace event. current_frame() now returns the callee. let trace_result = self.trace_event(TraceEvent::Call, None)?; if let Some(local_trace) = trace_result { - *frame.trace.lock() = local_trace; + *frame.iframe().trace.lock() = Some(local_trace); } let result = f(frame); @@ -1828,7 +2011,7 @@ impl VirtualMachine { // PY_UNWIND fires PyTrace_RETURN with arg=None — so we fire for // both Ok and Err, matching `call_trace_protected` behavior. if self.use_tracing.get() - && (!self.is_none(&frame.trace.lock()) || !self.is_none(&self.profile_func.borrow())) + && (frame.iframe().trace.lock().is_some() || !self.is_none(&self.profile_func.borrow())) { let ret_result = self.trace_event(TraceEvent::Return, None); // call_trace_protected: if trace function raises, its error @@ -1880,21 +2063,23 @@ impl VirtualMachine { } } - pub fn current_frame(&self) -> Option { - crate::frame::current_thread_frame() + pub fn current_frame(&self) -> Option { + crate::frame::current_thread_frame_materialize(self) } pub fn current_locals(&self) -> PyResult { - self.current_frame() + // Must include light frames so locals() returns the correct scope. + crate::frame::current_thread_frame_materialize(self) .expect("called current_locals but no frames on the stack") .locals(self) } pub fn current_globals(&self) -> PyDictRef { - self.current_frame() - .expect("called current_globals but no frames on the stack") - .globals - .clone() + let ptr = crate::vm::thread::get_current_frame(); + if !ptr.is_null() { + return unsafe { (*ptr).globals().to_owned() }; + } + crate::frame::current_globals().expect("called current_globals but no frames on the stack") } pub fn try_class(&self, module: &'static str, class: &'static str) -> PyResult { @@ -1953,11 +2138,14 @@ impl VirtualMachine { .get_attr(identifier!(self, __import__), self) .map_err(|_| self.new_import_error("__import__ not found", module.to_owned()))?; - let (locals, globals) = if let Some(frame) = self.current_frame() { - ( - Some(frame.locals.clone_mapping(self)), - Some(frame.globals.clone()), - ) + let (locals, globals) = if let Some(globals) = crate::frame::current_globals() { + // Locals fallback: use the heavy frame if available, otherwise + // use globals as locals (light frame locals are on the data stack). + let locals_mapping = self.current_frame().map_or_else( + || ArgMapping::from_dict_exact(globals.clone()), + |f| f.iframe().locals.clone_mapping(self), + ); + (Some(locals_mapping), Some(globals)) } else { (None, None) }; diff --git a/crates/vm/src/vm/thread.rs b/crates/vm/src/vm/thread.rs index 9948b936f..1520b2cd8 100644 --- a/crates/vm/src/vm/thread.rs +++ b/crates/vm/src/vm/thread.rs @@ -5,12 +5,21 @@ use crate::builtins::PyBaseExceptionRef; #[cfg(feature = "threading")] use alloc::sync::Arc; -use crate::frame::Frame; +#[cfg(all(unix, feature = "threading"))] +use crate::frame::FrameObject; +use crate::frame::InterpreterFrame; +#[cfg(all(unix, feature = "threading"))] +use crate::{AsObject, Py, PyObject, VirtualMachine}; +#[cfg(all(not(unix), feature = "threading"))] use crate::{AsObject, PyObject, VirtualMachine}; +#[cfg(not(feature = "threading"))] +use crate::{AsObject, PyObject, VirtualMachine}; +#[cfg(all(unix, feature = "threading"))] +use core::sync::atomic::AtomicPtr; use core::{ cell::{Cell, RefCell}, ptr::NonNull, - sync::atomic::{AtomicPtr, Ordering}, + sync::atomic::{AtomicUsize, Ordering}, }; use itertools::Itertools; use std::thread_local; @@ -38,9 +47,13 @@ pub struct ThreadSlot { /// thread at a safepoint and supplies the happens-before edge, so the /// pointer and the frames it reaches are quiescent and alive at read time. #[cfg(unix)] - pub top_frame: AtomicPtr, + pub top_frame: AtomicPtr, + /// Raw InterpreterFrame pointer, published alongside top_frame so + /// cross-thread readers (sys._current_frames) can materialize + /// stack-allocated frames that have no FrameObject. + pub top_iframe: AtomicUsize, /// Raw frame pointers, valid while the owning thread's call stack is active. - /// Readers must hold the Mutex and convert to FrameRef inside the lock. + /// Readers must hold the Mutex and convert to FrameObjectRef inside the lock. /// Used on non-unix threading builds, which have no stop-the-world. #[cfg(not(unix))] pub frames: parking_lot::Mutex>, @@ -82,12 +95,11 @@ thread_local! { static CURRENT_THREAD_SLOT: RefCell> = const { RefCell::new(None) }; /// Current top frame for signal-safe traceback walking. - /// Mirrors `PyThreadState.current_frame`. Read by faulthandler's signal - /// handler to dump tracebacks without accessing RefCell or locks. - /// Uses AtomicPtr for async-signal-safety (signal handlers may read this - /// while the owning thread is writing). - pub(crate) static CURRENT_FRAME: AtomicPtr = - const { AtomicPtr::new(core::ptr::null_mut()) }; + /// Stores a `*const InterpreterFrame` as `usize`. + /// Read by faulthandler's signal handler to dump tracebacks without + /// accessing RefCell or locks. Uses AtomicUsize for async-signal-safety. + pub(crate) static CURRENT_FRAME: AtomicUsize = + const { AtomicUsize::new(0) }; /// Cached pointer to this thread's `ThreadSlot::top_frame`, so the hot /// push/pop path can publish the top frame with a single relaxed store and @@ -95,7 +107,7 @@ thread_local! { /// initialized; the `Arc` in `CURRENT_THREAD_SLOT` keeps the /// pointee alive until `cleanup_current_thread_frames` clears this. #[cfg(all(unix, feature = "threading"))] - static CURRENT_TOP_FRAME_SLOT: Cell<*const AtomicPtr> = + static CURRENT_TOP_FRAME_SLOT: Cell<*const AtomicPtr> = const { Cell::new(core::ptr::null()) }; } @@ -339,6 +351,7 @@ fn init_thread_slot_if_needed(vm: &VirtualMachine) { let new_slot = Arc::new(ThreadSlot { #[cfg(unix)] top_frame: AtomicPtr::new(core::ptr::null_mut()), + top_iframe: AtomicUsize::new(0), #[cfg(not(unix))] frames: parking_lot::Mutex::new(Vec::new()), exception: crate::PyAtomicRef::from(None::), @@ -670,28 +683,56 @@ pub fn pop_thread_frame() { }); } -/// Set the current thread's top frame pointer for signal-safe traceback walking. -/// Returns the previous frame pointer so it can be restored on pop. -pub fn set_current_frame(frame: *const Frame) -> *const Frame { - // Publish the top frame for cross-thread readers. The relaxed store is - // ordered by stop-the-world at read time (see `ThreadSlot::top_frame`). - #[cfg(all(unix, feature = "threading"))] +/// Set the current thread's top InterpreterFrame pointer. +/// Returns the previous pointer so it can be restored on pop. +#[must_use] +#[allow(clippy::not_unsafe_ptr_arg_deref)] +pub fn set_current_frame(frame: *const InterpreterFrame) -> *const InterpreterFrame { + // Publish the top frame for cross-thread readers (faulthandler, + // sys._current_frames). + #[cfg(feature = "threading")] { - let slot_top = CURRENT_TOP_FRAME_SLOT.with(Cell::get); - if !slot_top.is_null() { - // SAFETY: points to this thread's `ThreadSlot::top_frame`, kept - // alive by the Arc in `CURRENT_THREAD_SLOT` for the thread's life. - unsafe { (*slot_top).store(frame as *mut Frame, Ordering::Relaxed) }; - } + CURRENT_THREAD_SLOT.with(|slot| { + if let Some(s) = slot.borrow().as_ref() { + if !frame.is_null() { + #[cfg(unix)] + { + let frame_obj = unsafe { (*frame).frame_obj() }; + let fo_ptr = match frame_obj { + Some(py) => { + py as *const Py as *const FrameObject + as *mut FrameObject + } + None => core::ptr::null_mut(), + }; + s.top_frame.store(fo_ptr, Ordering::Relaxed); + } + s.top_iframe.store(frame as usize, Ordering::Relaxed); + } else { + #[cfg(unix)] + s.top_frame.store(core::ptr::null_mut(), Ordering::Relaxed); + s.top_iframe.store(0, Ordering::Relaxed); + } + } + }); } - CURRENT_FRAME.with(|c| c.swap(frame as *mut Frame, Ordering::Relaxed) as *const Frame) + CURRENT_FRAME.with(|c| c.swap(frame as usize, Ordering::Relaxed)) as *const InterpreterFrame } -/// Get the current thread's top frame pointer. +/// Lightweight version that only writes to TLS CURRENT_FRAME, returning +/// the previous value. Does not update cross-thread top_frame (that's +/// updated by `set_current_frame` for FrameObject-based calls). +#[inline(always)] +#[must_use] +pub fn set_current_frame_nosave(frame: *const InterpreterFrame) -> *const InterpreterFrame { + CURRENT_FRAME.with(|c| c.swap(frame as usize, Ordering::Relaxed)) as *const InterpreterFrame +} + +/// Get the current thread's top InterpreterFrame pointer. /// Used by faulthandler's signal handler to start traceback walking. #[must_use] -pub fn get_current_frame() -> *const Frame { - CURRENT_FRAME.with(|c| c.load(Ordering::Relaxed) as *const Frame) +pub fn get_current_frame() -> *const InterpreterFrame { + CURRENT_FRAME.with(|c| c.load(Ordering::Relaxed)) as *const InterpreterFrame } /// Update the current thread's exception slot atomically (no locks). @@ -784,18 +825,36 @@ pub fn reinit_frame_slot_after_fork(vm: &VirtualMachine) { let mut cur = get_current_frame(); while !cur.is_null() { // SAFETY: the forking thread's chain frames are alive. - let py = unsafe { crate::Py::::from_payload_ptr(cur) }; - current_frames.push(FramePtr(unsafe { NonNull::new_unchecked(py as *mut _) })); - cur = unsafe { (*cur).previous_frame() }; + let iframe = unsafe { &*cur }; + if let Some(fo) = iframe.frame_obj() { + current_frames.push(FramePtr(unsafe { + NonNull::new_unchecked(fo as *const _ as *mut _) + })); + } + cur = iframe.previous.load(Ordering::Relaxed) as *const InterpreterFrame; } current_frames.reverse(); current_frames }; + #[cfg(unix)] + let top_fo_ptr = { + let top_iframe = get_current_frame(); + if top_iframe.is_null() { + core::ptr::null_mut() + } else { + match unsafe { (*top_iframe).frame_obj() } { + Some(fo) => fo as *const Py as *const FrameObject as *mut FrameObject, + None => core::ptr::null_mut(), + } + } + }; + let top_iframe_ptr = get_current_frame() as usize; let new_slot = Arc::new(ThreadSlot { - // The surviving child thread keeps executing its current frame chain, - // whose top is the signal-safe `get_current_frame()`. + // The surviving child thread keeps executing its current frame chain. + // Only publish heavy frames for signal safety. #[cfg(unix)] - top_frame: AtomicPtr::new(get_current_frame() as *mut Frame), + top_frame: AtomicPtr::new(top_fo_ptr), + top_iframe: AtomicUsize::new(top_iframe_ptr), #[cfg(not(unix))] frames: parking_lot::Mutex::new(current_frames), exception: crate::PyAtomicRef::from(vm.topmost_exception()), diff --git a/crates/vm/src/warn.rs b/crates/vm/src/warn.rs index 6500e8de0..d5729858d 100644 --- a/crates/vm/src/warn.rs +++ b/crates/vm/src/warn.rs @@ -513,7 +513,7 @@ fn show_warning( } /// Check if a frame's filename starts with any of the given prefixes. -fn is_filename_to_skip(frame: &crate::frame::Frame, prefixes: &PyTupleRef) -> bool { +fn is_filename_to_skip(frame: &crate::frame::FrameObject, prefixes: &PyTupleRef) -> bool { let filename = frame.f_code().co_filename(); let filename_bytes = filename.as_bytes(); prefixes.iter().any(|prefix| { @@ -523,15 +523,15 @@ fn is_filename_to_skip(frame: &crate::frame::Frame, prefixes: &PyTupleRef) -> bo }) } -/// Like Frame::next_external_frame but also skips frames matching prefixes. +/// Like FrameObject::next_external_frame but also skips frames matching prefixes. fn next_external_frame_with_skip( - frame: &crate::frame::FrameRef, + frame: &crate::frame::FrameObjectRef, skip_file_prefixes: Option<&PyTupleRef>, vm: &VirtualMachine, -) -> Option { +) -> Option { let mut f = frame.f_back(vm); loop { - let current: crate::frame::FrameRef = f.take()?; + let current: crate::frame::FrameObjectRef = f.take()?; if current.is_internal_frame() || skip_file_prefixes.is_some_and(|p| is_filename_to_skip(¤t, p)) { @@ -549,7 +549,9 @@ fn setup_context( skip_file_prefixes: Option<&PyTupleRef>, vm: &VirtualMachine, ) -> PyResult<(PyStrRef, usize, Option, PyObjectRef)> { - let mut f = vm.current_frame(); + // Materialize the topmost frame (including light frames) so stack + // level counting is correct across the full Python frame chain. + let mut f = crate::frame::current_thread_frame_materialize(vm); // Stack level comparisons to Python code is off by one as there is no // warnings-related stack level to avoid. @@ -576,10 +578,18 @@ fn setup_context( } let (globals, filename, lineno) = if let Some(f) = f { - (f.globals.clone(), f.code.source_path(), f.f_lineno()) + ( + f.iframe().globals().to_owned(), + f.iframe().code().source_path(), + f.f_lineno(), + ) } else if let Some(frame) = vm.current_frame() { // We have a frame but it wasn't found during stack walking - (frame.globals.clone(), vm.ctx.intern_str(""), 1) + ( + frame.iframe().globals().to_owned(), + vm.ctx.intern_str(""), + 1, + ) } else { // No frames on the stack - use sys.__dict__ (interp->sysdict) let globals = vm