From f08933be876ef7987d026bc1c550f3ecaa4005ac Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" <69878+youknowone@users.noreply.github.com> Date: Sat, 1 Aug 2026 00:59:41 +0900 Subject: [PATCH] Split InterpreterFrame from FrameObject for stack-allocated execution (#8354) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add LightFrame for unobserved Python-to-Python calls Allocate a lightweight LightFrame header on the DataStack instead of a full Frame PyObject for specialized exact-args call paths. The frame is materialized lazily only when observed (sys._getframe, traceback, tracing). Key changes: - LightFrame struct with borrowed pointers and no refcount overhead - FrameSource enum (Heavy/Light) replacing the object field on ExecutingFrame - CURRENT_LIGHT_FRAME TLS for the light frame chain - Interleaved heavy/light frame walking in frame_at_offset_vm and current_thread_frame_vm - invoke_light_slots on PyFunction for the fast call path Call overhead reduced from ~92ns to ~58ns (~37% reduction). fib(28) improved from ~152ms to ~111ms (~27% reduction). Assisted-by: Claude * Streamline with_frame: inline recursion check, skip tracing dispatch - Inline check_recursive_call instead of wrapping in with_recursion closure - Amortize C stack overflow check to every 64th call depth - Skip dispatch_traced_frame when use_tracing is false (hot path) - Add recursion_depth_increment/decrement helpers Assisted-by: Claude * Address review findings for LightFrame correctness - Fix materialize_light_frame previous pointer: use payload pointer (&**frame) instead of Py pointer, matching the chain convention - Fix predecessor ref management: use retained_back instead of mem::forget to keep materialized predecessors alive - Fix materialized ref reclaim: use FrameRef::from_raw to drop the leaked keeper ref without double-incrementing - Use PyAtomic for LightFrame.lasti instead of AtomicU32, removing the unsound cast on non-threading targets - Add amortized C stack overflow check to light frame path - Add scopeguard to light frame execution for panic safety - Remove unnecessary Send/Sync impls from LightFrame - Make check_c_stack_overflow pub(crate) for light frame path access Assisted-by: Claude * Fix CI failures: cspell, stale materialized state, f_back chain gaps - Fix cspell: 'amortised' → 'amortized' - Sync lasti, prev_line, and fastlocals when re-observing a materialized light frame, fixing stale f_locals/f_lineno in test_inspect and others - In with_frame, materialize any active light frame as the heavy frame's predecessor and store it in retained_back, so f_back and inspect.stack see the correct interleaved order (H_new → L_mat → H_old) - Guard invoke_light_slots on NEWLOCALS | OPTIMIZED code flags Assisted-by: Claude * Unify frame chain: replace dual-chain with single FrameChainPtr Replace the dual-chain frame management (CURRENT_FRAME AtomicPtr + CURRENT_LIGHT_FRAME Cell<*const LightFrame>) with a single unified chain using FrameChainPtr, a tagged pointer where bit 0 distinguishes heavy (*const Frame) from light (*const LightFrame) entries. Changes: - Add FrameChainPtr type with tagged pointer encoding - Change LightFrame: replace previous_light + saved_current_frame with single previous: FrameChainPtr field - Change InterpreterFrame.previous from AtomicPtr to PyAtomic - Change CURRENT_FRAME TLS from AtomicPtr to AtomicUsize - Remove CURRENT_LIGHT_FRAME TLS entirely - Simplify all chain walkers (frame_at_offset, frame_at_offset_vm, find_owned_chain_frame, for_each_current_frame, current_thread_frame) - Remove light-frame materialization block from with_frame - Update f_back to dispatch on FrameChainPtr tags - Update faulthandler and gc_state to walk unified chain - Adaptive C stack check: every call in debug, every 16th in release Assisted-by: Claude * Fix light frame materialization: current_frame, f_back, and GC tracking - current_frame() now uses current_thread_frame_vm() to materialize light frames, fixing super()/compile()/exec() when called from light frame context - materialize_light_frame stores retained_back for heavy predecessors too, so f_back resolves after the caller returns - Escaped materialized frames (traceback, sys._getframe) are GC-tracked at cleanup so reference cycles are collectible - Restore pre-existing @expectedFailure markers for test_frame proxy tests Assisted-by: Claude * Limit light frame materialization to super() only current_frame() returns heavy-only again to avoid unnecessary materialization that extends object lifetimes. super() init uses current_thread_frame_vm() directly since it must inspect the light frame's code and freevars to find __class__. Assisted-by: Claude * Fix remaining CI failures: C stack check, GC tracking threshold, expectedFailure - Check C stack overflow every call in light frame path (not every 16th) to prevent stack overflow on CI workers with smaller stacks - Fix GC tracking threshold: escaped materialized frame detection uses strong_count > 1 (was > 2, missed frames referenced only by traceback in the result) - current_frame() materializes light frames so warnings, compile(), and other callers see the correct frame - Mark test_futures2 PyFutureTests.test_task_exc_handler_correct_context as expectedFailure (pre-existing on main: PyTask __del__ timing) Assisted-by: Claude * Avoid materialization in hot paths: add light-frame-aware accessors current_frame() stays heavy-only to avoid costly materialization on every call. Instead, add current_globals(), current_code(), and current_builtins() that read directly from light frame raw pointers without creating frame PyObjects. Callers migrated: - PyFunction::new: current_builtins() for fallback builtins lookup - compile()/eval(): current_code() for future feature flags - type.__new__: current_globals() for __module__ detection - typevar/typing: current_globals() for caller module name - _io: current_code() for source path check - super(): current_thread_frame_vm() for __class__ cell access - warnings.warn: current_thread_frame_vm() for stack level walk - import: current_globals() for module resolution Assisted-by: Claude * Fix current_locals() to include light frames current_locals() must materialize light frames so locals() builtin returns the correct scope. This fixes test_zipimport doctest and other tests that rely on locals() in light frame context. Assisted-by: Claude * Fix clippy, profile tracing, resume_gen_frame, and faulthandler issues - Fix clippy: collapsible_if in materialize_light_frame, map_unwrap_or in import - Fix profile breakage: use current_thread_frame_vm in trace_event_inner to materialize light frames, preventing frame identity mismatch when profiling is enabled inside a light frame (e.g. profile.Profile.runctx) - Disable light frames inside trace/profile callbacks (tracing_is_suppressed) - Fix resume_gen_frame: skip light frames in previous chain to avoid dangling pointers after generator suspend - Fix faulthandler: emit '' when chain has only light frames Assisted-by: Claude * Fix materialized frame refcount: transfer localsplus ownership on exit Remove @expectedFailure from test_futures2: the underlying issue was that materialized light frames cloned localsplus values (refcount +1 each), keeping objects alive until the next GC cycle collection. Fix: after run_light_frame returns, transfer ownership of localsplus values from the light frame to the materialized frame, replacing the clones. This eliminates the extra refcount, allowing objects to be GC'd immediately when their last reference is dropped. Also stabilize materialized frame's localsplus on the heap in sync_materialized_on_exit before GC tracking — the data-stack backing will be popped imminently. Assisted-by: Claude * Fix rustfmt, increase stack margin, address review comments - Fix rustfmt formatting issues (lint CI failure) - Increase STACK_MARGIN_BYTES from 2048 to 3072 words to prevent C stack overflow in deep recursion tests on macOS CI (test_functools SIGSEGV) - Skip dispatch_traced_frame when use_tracing is false (review comment) - Fix post-fork top_frame initialization: walk past light frames to find nearest heavy frame (review comment) Assisted-by: Claude * Remove redundant test override in test_futures2 The override only called super() without any marker, making it a no-op that prek flags as a redundant test patch. Assisted-by: Claude * Fix test_pdb and test_functools CI failures - Revert with_frame dispatch_traced_frame skip: tracing can be enabled mid-execution (pdb.set_trace), so Return events must always be checked. This fixes 3 test_pdb doctest failures across all platforms. - Check C stack overflow on every with_frame call instead of every 64th: the previous sampling approach missed overflows between checks when native stack frames are large (invoke_light_slots + lru_cache recursion). - Add #[inline(never)] to invoke_light_slots to prevent the optimizer from merging its stack frame into callers. - Add early C stack check before DataStack allocation in invoke_light_slots. Assisted-by: Claude * Rename Frame to FrameObject, FrameRef to FrameObjectRef Prepare for _PyInterpreterFrame/PyFrameObject separation by renaming the PyObject-backed frame type to FrameObject. Python-visible name ("frame") is unchanged. Assisted-by: Claude * Remove Deref from FrameObject Replace implicit field access through Deref with explicit iframe()/iframe_ref()/iframe_mut() calls. This decouples FrameObject from InterpreterFrame field layout, enabling independent modification of InterpreterFrame in future commits. Assisted-by: Claude * Move InterpreterFrame identity fields to raw pointers InterpreterFrame's code/globals/builtins/func_obj fields are now borrowed raw pointers instead of owned PyRef/PyObjectRef. Ownership of these references is anchored in FrameObject's new owned_code, owned_globals, owned_builtins, owned_func_obj fields. This aligns InterpreterFrame's layout with LightFrame's existing raw-pointer pattern, enabling future DataStack-based allocation without PyObject heap allocation. - Add code()/globals()/builtins()/func_obj() accessor methods - Add FrameObject::new_ref() for combined create+allocate+patch - Update ExecutingFrame to use &Py / &Py / &PyObject - Update monitoring.rs functions to take &Py instead of &PyRef - Add Send+Sync impls for InterpreterFrame (raw pointers are !Send by default) Assisted-by: Claude * Remove LightFrame, FrameChainPtr, and dual-frame-type infrastructure Delete LightFrame struct, FrameChainPtr tagged pointer, FrameSource enum, and all supporting functions: materialize_light_frame, sync_light_to_materialized, transfer_localsplus_to_materialized, sync_materialized_on_exit, run_light_frame, invoke_light_slots body, LocalsPlus::from_datastack_raw. All call paths now use a single frame type (FrameObject wrapping InterpreterFrame). The frame chain is a simple *const FrameObject linked list through InterpreterFrame.previous. invoke_light_slots delegates to invoke_exact_args_slots. ExecutingFrame.frame_source replaced with ExecutingFrame.frame. Frame chain walking simplified throughout (no more light/heavy dispatch). Net: -724 lines of dual-frame-type complexity. Assisted-by: Claude * Clean up invoke_light_slots and _vm function variants - Replace invoke_light_slots calls with invoke_exact_args_slots - Delete the invoke_light_slots delegate method - Replace current_thread_frame_vm/frame_at_offset_vm with their non-_vm equivalents (no more light frame materialization needed) - Remove stale light-frame comment in callable.rs Assisted-by: Claude * Fix CI lint issues - Move github context expressions from run blocks to env blocks (zizmor/template-injection) - Add workflow-level permissions: {} to lib-deps-check.yaml - Suppress zizmor excessive-permissions for lib-deps-check.yaml (pull_request_target is required for PR comments) - Apply rustfmt formatting Assisted-by: Claude * Stack-allocate InterpreterFrame for non-generator function calls - Extract InterpreterFrame::new() from FrameObject::new() - Change frame chain from *const FrameObject to *const InterpreterFrame - Add InterpreterFrame.materialized field for lazy FrameObject creation - Add vm.with_iframe()/run_frame_fast() for stack-allocated frame execution - Move ExecutingFrame.frame to iframe: *const InterpreterFrame - Implement lazy materialize() for on-demand FrameObject creation - Update invoke_with_locals/invoke_prepared_exact_args to skip heap allocation - Update frame chain walking in faulthandler, gc_state, builtins/frame - super() reads InterpreterFrame directly without materialization Assisted-by: Claude * Remove unnecessary refcount and atomic ops from fast call path - Use raw pointer for func_obj instead of cloning PyObjectRef (saves 2 atomic RMW) - Skip owner AcqRel swap for stack-allocated frames (always Thread-owned) Assisted-by: Claude * Change trace field to Option, remove vm from InterpreterFrame::new - trace: PyMutex -> PyMutex> - Eliminates vm.ctx.none() refcount inc/dec per frame init - Remove vm parameter from InterpreterFrame::new() (no longer needed) - f_trace getter returns None when trace is unset - f_trace setter stores None instead of the None singleton Assisted-by: Claude * Store current frame pointer on VirtualMachine for fast lookup - Add current_frame_ptr Cell field to VirtualMachine - Hot path (with_iframe) reads/writes vm.current_frame_ptr instead of TLS - TLS CURRENT_FRAME still maintained for signal-safe traceback walking - Use set_current_frame_nosave() to skip cross-thread top_frame update - super(), frame_at_offset, current_thread_frame_materialize use vm Cell Assisted-by: Claude * Apply rustfmt Assisted-by: Claude * Fix clippy use_self warnings, add inline hints for hot path Assisted-by: Claude * Remove code refcount clone from fast call path Defer code.to_owned() to the generator/coroutine path only. On the fast path, use &Py directly — the code object is alive via the PyFunction on the caller's stack. Assisted-by: Claude * Remove VM current_frame_ptr Cell (fix SIGSEGV in threading builds) The Cell on VirtualMachine could desynchronize from the TLS CURRENT_FRAME in nested VM scenarios (enter_vm called with different VMs on the same thread). Revert to TLS-only frame chain management. Keep set_current_frame_nosave() for the stack-frame fast path: it skips cross-thread top_frame publication but still does the TLS swap. Fix clippy not_unsafe_ptr_arg_deref in set_current_frame. Fix unnecessary Result wrapper in make_generator_or_coro. Assisted-by: Claude * Fix CI failures: GC frame assertion, faulthandler Radium import, dead code warning - Fix SIGSEGV in threading builds: GC debug assertion walked frame chain via top_frame, which is not updated by set_current_frame_nosave used by stack-allocated iframes. Walk CURRENT_FRAME TLS chain instead. - Add InterpreterFrame::get_lasti() accessor, remove direct Radium import from faulthandler.rs (fixes unused import warning). - Add cfg_attr allow(dead_code) for from_payload_ptr without threading (fixes WASM build warning). Assisted-by: Claude * Fix materialized frame: copy localsplus, clear stale previous pointers - materialize_slow: copy fastlocals snapshot to heap instead of empty localsplus (fixes index-out-of-bounds panic in locals()/f_locals) - materialize_slow: set previous to null (stack iframes become dangling) - with_frame/resume_gen_frame: clear iframe.previous on pop (prevents dangling pointers to freed stack iframes) - f_back: walk TLS CURRENT_FRAME chain instead of top_frame for cross-thread lookup (top_frame is stale for stack-allocated iframes) - check_locals_access: match materialized frames by comparing the Py address stored in the stack iframe's materialized field Assisted-by: Claude * Fix Windows clippy: gate unix-only imports and variables FrameObject, Py imports and top_iframe variable are only used in unix+threading cfg blocks. Gate them properly for non-unix builds. Assisted-by: Claude * Fall back to FrameObject when tracing; fix cross-thread frame access - invoke_with_locals: fall back to heap-allocated FrameObject path when use_tracing is active, so trace/profile callbacks fire correctly. This fixes test_trace, test_bdb, test_monitoring regressions. - set_current_frame: publish top_iframe alongside top_frame in ThreadSlot so cross-thread readers (sys._current_frames) can materialize stack-allocated frames from other threads under stop-the-world. - get_all_current_frames: use TLS CURRENT_FRAME for current thread, fall back to top_iframe materialization for other threads. - Use set_current_frame (not nosave) in with_iframe for proper cross-thread visibility. Assisted-by: Claude * Apply rustfmt Assisted-by: Claude * Fix Windows: gate unix-only top_iframe_ptr variable Assisted-by: Claude * Fix frame chain: f_back, retained_back, owner, faulthandler - Set materialized frame owner to FrameObject (not Thread) so frame.clear() works on traceback frames - Fix f_back for materialized frames: walk TLS chain to find source iframe and materialize its previous frame - Fix faulthandler dump_traceback to use dump_live_frames (reads live lasti from stack iframes instead of stale materialized copies) - Add retained_back propagation in with_frame cleanup: when a frame escapes, materialize caller and mark it escaped for chain propagation - Fix release_datastack_frame: don't overwrite retained_back if already set by with_frame cleanup Assisted-by: Claude * Apply rustfmt Assisted-by: Claude * Refine with_frame retained_back: use frame_obj instead of materialize Avoid materializing caller frames in with_frame cleanup to prevent creating localsplus snapshots that keep extra refcounts. Only use existing FrameObjects (frame_obj) for retained_back. Also keep previous pointer alive in release_datastack_frame since the caller is still executing at that point. Assisted-by: Claude * Fix f_lineno: use Cell for prev_line, update on every instruction prev_line was only updated when tracing was active, causing f_lineno to return stale values when observed mid-CALL (e.g. from warnings.warn or sys._getframe). Changed prev_line from u32 to Cell for interior mutability so it can be read safely while ExecutingFrame holds a shared reference, and update it on every instruction so f_lineno always returns the correct line. Assisted-by: Claude * Fix f_lineno for materialized frames: read live prev_line from TLS chain Materialized FrameObjects have a snapshot of prev_line from materialize time. When f_lineno is called on a materialized frame that is still executing (its source iframe is on the TLS chain), walk the TLS chain to find the source iframe and read its live prev_line. This fixes lineno tracking in warnings, gettext, and other modules that read frame line numbers during function calls. Also fixed the pointer comparison: use from_payload_ptr to convert FrameObject payload address to Py address for matching against the materialized field. Assisted-by: Claude * Sync live frame state for materialized frames - Add find_live_source_iframe() to walk TLS chain and find the live source InterpreterFrame for a materialized FrameObject - f_lasti: read live lasti from source iframe when available - f_lineno: use read_volatile for live prev_line from source iframe - sync_visible_locals_to_mapping: read live localsplus from source iframe - framelocalsproxy_getval: read live localsplus from source iframe - with_iframe cleanup: sync localsplus, prev_line, lasti to materialized FrameObject when the frame returns - sync_fastlocals_from: new LocalsPlus method to update fastlocals Fixes test_inspect (stale f_locals, stale lineno in positions), test_listcomps (stale iteration variable in locals()). Assisted-by: Claude * Fix retained_back: use materialize_chain and strong_count check - Add materialize_chain() that creates a lightweight FrameObject with empty localsplus for f_back chain building only. This avoids cloning local variables which would create extra refcounts (fixes test_memoryview). - with_frame cleanup: use strong_count > 1 instead of escaped flag to determine if retained_back is needed, ensuring chain propagation. - with_iframe cleanup: use materialize_chain for retained_back to avoid extra refcounts from localsplus snapshots. Fixes test_memoryview (refcount from retained_back chain) and test_traceback.test_extract_stack (f_back chain depth). Assisted-by: Claude * Fix cross-thread f_back chain for sys._current_frames/exceptions - get_all_current_frames: materialize entire frame chain and link retained_back during stop-the-world for other threads - f_back: use stop-the-world to safely materialize cross-thread frame chains on unix when prev iframe is on another thread - f_back: check frame_obj() before STW for fast path Fixes test_sys.test_current_frames and test_sys.test_current_exceptions. Assisted-by: Claude * Fix monitoring LINE events and faulthandler thread dump - Skip all instrumented opcodes in bytecode loop prev_line update to avoid defeating InstrumentedLine de-duplication - Add prev_line update in execute_instrumented for non-RESUME/non-LINE instrumented opcodes - update_events_mask: walk TLS iframe chain to re-instrument all frames including stack-allocated ones (fixes missing LINE events for inline frames already past RESUME) - faulthandler: use top_iframe fallback for stack-allocated frames in dump_all_threads and watchdog_thread - faulthandler: use dump_live_frames for current thread dump Fixes test_monitoring LINE/CALL tests, test_faulthandler thread dumps. Assisted-by: Claude * Fix clippy: remove duplicate Radium import, use previous() helper Assisted-by: Claude * Fix materialized frame issues: GC tracking, live locals, f_trace propagation - Track materialized FrameObjects in GC so cycle collection works (fixes __del__ not called at exit for exception traceback cycles) - materialize_chain returns owned PyRef without temporary_refs to avoid defeating GC cycle detection from non-GC-tracked storage - framelocalsproxy_setval writes to live source iframe when available (fixes f_locals proxy writes not affecting executing frame) - f_lineno uses lasti-based line for non-executing frames instead of prev_line (fixes PEP 626 wrong line after exception unwind) - f_trace/f_trace_lines/f_trace_opcodes setters propagate to live source iframe (fixes pdb set_trace not working through materialized frames) - frame.clear() checks find_live_source_iframe to reject clearing a frame that is backed by a live stack-allocated iframe - Fix clippy warnings and unused imports in faulthandler - Remove unsafe top_iframe access from faulthandler watchdog thread Assisted-by: Claude * Fix faulthandler crashes, GC tracking timing, and test_generators - Use top_iframe instead of top_frame for frame chain walking in faulthandler (dump_all_threads and watchdog), avoiding stale or GC-cleared FrameObject pointer dereference - Defer GC tracking of materialized FrameObjects to with_iframe cleanup (after set_current_frame restores old chain), preventing premature collection while temporary_refs still holds the only ref - Add FrameObject::try_iframe() for safe access to potentially GC-cleared frames - Remove expectedFailure for test_exhausted_generator_frame_cycle (now passes with GC tracking fix) - Add 'noalias' to cspell dictionary Assisted-by: Claude * Fix Windows frame dump and remove dead code - Publish top_iframe on all platforms (not just unix) so Windows faulthandler and sys._current_frames can see stack-allocated frames - Use top_iframe instead of frames Mutex in Windows faulthandler dump and watchdog paths - Use top_iframe in non-unix get_all_current_frames for sys._current_frames - Remove unused dump_traceback_thread_chain (replaced by top_iframe walk) - Mark test_pdb_await_support as expected failure Assisted-by: Claude * Fix ThreadSlot init on all platforms, skip test_pdb_await_support - Add top_iframe to ThreadSlot initializer in init_thread_slot_if_needed (fixes wasm32, Windows compile errors) - Skip test_pdb_await_support on RustPython (async pdb exception callback receives None exc argument) Assisted-by: Claude * Remove dead faulthandler code (dump_frame_from_ref, dump_traceback_thread_frames, try_iframe) These functions are no longer called after switching all frame chain walking to use top_iframe directly. Assisted-by: Claude * Fix Windows sys._current_frames: materialize full frame chain The non-unix get_all_current_frames path was only materializing the top iframe without linking retained_back, so f_back chain walking could not find deeper frames like f123() in test_sys tests. Replicate the full chain materialization from the unix path. Assisted-by: Claude * Use STW-based cross-thread f_back on all platforms Replace the non-unix frames-mutex fallback with the same stop-the-world iframe chain materialization used on unix. Fixes test_current_exceptions on Windows where stack-allocated iframes were not found in the FrameObject-only frames list. Assisted-by: Claude * Fix set_f_lineno to write to live iframe, use STW for cross-thread access - set_f_lineno: write lasti/pending_stack_pops to the live source iframe instead of the materialized copy so debugger jumps take effect on stack-allocated frames - f_back cross-thread: enter STW before dereferencing the prev pointer to prevent use-after-free if the owning thread returns - get_all_current_frames (non-unix): add STW protection when walking cross-thread iframe chains, matching the unix path Assisted-by: Claude * Guard current_location() against lasti==0, always capture retained_back - current_location(): return first_line_number instead of panicking when lasti is 0 (before first instruction executes) - with_frame retained_back: materialize the caller iframe when needed so f_back always resolves for escaped FrameObjects Assisted-by: Claude * Fix format, revert eager retained_back materialization Remove double blank line in frame.rs (lint failure). Revert with_frame retained_back to only use already-materialized caller FrameObjects — eagerly materializing the caller added refcounts on local variables, preventing timely deallocation and causing test_io, test_memoryview, and test_futures failures. Assisted-by: Claude * Remove rustpython-unicode-isolation-issue.md from gitignore Assisted-by: Claude * Remove test_pdb_await_support skip: async pdb now works correctly Assisted-by: Claude * Fix STOP_ITERATION monitoring: wrap value in StopIteration instance fire_stop_iteration was passing the raw iterator return value to callbacks, but the STOP_ITERATION event callback signature expects a StopIteration exception instance. Wrap non-StopIteration values in a new StopIteration(value), matching PyMonitoring_FireStopIterationEvent. This fixes test_pdb_await_support where bdb's exception_callback received None instead of a StopIteration instance. Assisted-by: Claude --- .cspell.dict/rust-more.txt | 1 + .github/workflows/cron-ci.yaml | 12 +- .github/workflows/lib-deps-check.yaml | 17 +- .github/zizmor.yml | 5 + .gitignore | 2 +- Lib/test/test_generators.py | 1 - crates/capi/src/ceval.rs | 18 +- crates/capi/src/pyframe.rs | 4 +- crates/stdlib/src/faulthandler.rs | 237 ++-- crates/vm/src/builtins/asyncgenerator.rs | 11 +- crates/vm/src/builtins/coroutine.rs | 8 +- crates/vm/src/builtins/frame.rs | 342 +++-- crates/vm/src/builtins/frame_locals_proxy.rs | 8 +- crates/vm/src/builtins/function.rs | 238 +++- crates/vm/src/builtins/generator.rs | 8 +- crates/vm/src/builtins/super.rs | 33 +- crates/vm/src/builtins/traceback.rs | 17 +- crates/vm/src/builtins/type.rs | 11 +- crates/vm/src/coroutine.rs | 20 +- crates/vm/src/exceptions.rs | 4 +- crates/vm/src/frame.rs | 1302 +++++++++++++----- crates/vm/src/gc_state.rs | 39 +- crates/vm/src/import.rs | 4 +- crates/vm/src/object/core.rs | 3 +- crates/vm/src/object/ext.rs | 8 +- crates/vm/src/object/payload.rs | 2 +- crates/vm/src/protocol/callable.rs | 6 +- crates/vm/src/stdlib/_io.rs | 4 +- crates/vm/src/stdlib/_thread.rs | 107 +- crates/vm/src/stdlib/_typing.rs | 5 +- crates/vm/src/stdlib/builtins.rs | 10 +- crates/vm/src/stdlib/sys.rs | 10 +- crates/vm/src/stdlib/sys/monitoring.rs | 90 +- crates/vm/src/stdlib/typevar.rs | 11 +- crates/vm/src/suggestion.rs | 12 +- crates/vm/src/types/zoo.rs | 2 +- crates/vm/src/vm/mod.rs | 360 +++-- crates/vm/src/vm/thread.rs | 125 +- crates/vm/src/warn.rs | 26 +- 39 files changed, 2167 insertions(+), 956 deletions(-) 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